Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why does Cache::lock() return false in Laravel 7?

My framework is Laravel 7 and the Cache driver is Memcached. I want to perform atomic cache get/edit/put. For that I use Cache::lock() but it doesn't seem to work. The $lock->get() returns false (see below). How can I resolve this?

Fort testing, I reload Homestead, and run only the code below. And locking never happens. Is it possible Cache::has() break the lock mechanism?

if (Cache::store('memcached')->has('post_' . $post_id)) {
    $lock = Cache::lock('post_' . $post_id, 10);
    Log::info('checkpoint 1'); // comes here

    if ($lock->get()) {
        Log::info('checkpoint 2'); // but not here.
        $post_data = Cache::store('memcached')->get('post_' . $post_id);
        ... // updating $post_data..
        Cache::put('post_' . $post_id, $post_data, 5 * 60);
        $lock->release();
    }
} else {
        Cache::store('memcached')->put('post_' . $post_id, $initial, 5 * 60);
}
like image 752
horse Avatar asked Apr 25 '20 18:04

horse


2 Answers

So first of all a bit of background.

A mutual exclusion (mutex) lock as you correctly mentioned is meant to prevent race conditions by ensuring only one thread or process ever enters a critical section.

But first of all what is a critical section?

Consider this code:

public function withdrawMoney(User $user, $amount) {
    if ($user->bankAccount->money >= $amount) {
        $user->bankAccount->money = $user->bankAccount->money - $amount;
        $user->bankAccount->save();
        return true; 
    }
    return false;

}

The problem here is if two processes run this function concurrently, they will both enter the if check at around the same time, and both succeed in withdrawing, however this might lead the user having negative balance or money being double-withdrawn without the balance being updated (depending on how out of phase the processes are).

The problem is the operation takes multiple steps and can be interrupted at any given step. In other words the operation is NOT atomic.

This is the sort of critical section problem that a mutual exclusion lock solves. You can modify the above to make it safer:

public function withdrawMoney(User $user, $amount) {
    try {
        if (acquireLockForUser($user)) {
            if ($user->bankAccount->money >= $amount) {
                $user->bankAccount->money = $user->bankAccount->money - $amount;
                $user->bankAccount->save();
                return true; 
            }
            return false;
         }
    } finally {
       releaseLockForUser($user);
    }

}

The interesting things to point out are:

  1. Atomic (or thread-safe) operations don't require such protection
  2. The code we put between the lock acquire and release, can be considered to have been "converted" to an atomic operation.
  3. Acquiring the lock itself needs to be a thread-safe or atomic operation.

At the operating system level, mutex locks are typically implemented using atomic processor instructions built for this specific purpose such as an atomic test-and-set operation. This would check if a value if set, and if it is not set, set it. This works as a mutex if you just say the lock itself is the existence of the value. If it exists, the lock is taken and if it's not then you acquire the lock by setting the value.

Laravel implements the locks in a similar manner. It takes advantage of the atomic nature of the "set if not already set" operations that certain cache drivers provide which is why locks only work when those specific cache drivers are there.

However here's the thing that's most important:

In the test-and-set lock, the lock itself is the cache key being tested for existence. If the key is set, then the lock is taken and cannot generally be re-acquired. Typically locks are implemented with a "bypass" in which if the same process tries to acquire the same lock multiple times it succeeds. This is called a reentrant mutex and allows to use the same lock object throughout your critical section without worrying about locking yourself out. This is useful when the critical section becomes complicated and spans multiple functions.

Now here's where you have two flaws with your logic:

  1. Using the same key for both the lock and the value is what is breaking your lock. In the lock analogy you're trying to store your valuables in a safe which itself is part of your valuables. That's impossible.
  2. You have if (Cache::store('memcached')->has('post_' . $post_id)) { outside your critical section but it should itself be part of the critical section.

To fix this issue you need to use a different key for the lock than you use for the cached entries and move your has check in the critical section:


$lock = Cache::lock('post_' . $post_id. '_lock', 10);
try {
    if ($lock->get()) { 
        //Critical section starts
        Log::info('checkpoint 1'); // if it comes here  

        if (Cache::store('memcached')->has('post_' . $post_id)) {          
            Log::info('checkpoint 2'); // it should also come here.
            $post_data = Cache::store('memcached')->get('post_' . $post_id);
            ... // updating $post_data..
            Cache::put('post_' . $post_id, $post_data, 5 * 60);
                    
        } else {
            Cache::store('memcached')->put('post_' . $post_id, $initial, 5 * 60);
        }
     }
     // Critical section ends
} finally {
   $lock->release();
}

The reason for having the $lock->release() in the finally part is because in case there's an exception you still want the lock being released rather than staying "stuck".

Another thing to note is that due to the nature of PHP you also need to set a duration that the lock will be held before it is automatically released. This is because under certain circumstances (when PHP runs out of memory for example) the process terminates abruptly and therefore is unable to run any cleanup code. The duration of the lock ensures the lock is released even in those situations and the duration should be set as the absolute maximum time the lock would reasonably be held.

like image 123
apokryfos Avatar answered Oct 16 '22 11:10

apokryfos


Cache::lock('post_' . $post_id, 10)->get() return false, because the 'post_' . $post_id is locked, the lock has not been released.

So you need to release the lock first:

Cache::lock('post_' . $post_id)->release()
// or release a lock without respecting its current owner
Cache::lock('post_' . $post_id)->forceRelease(); 

then try again, it will return true.

And recommend to use try catch or block to set a specified time limit, Laravel will wait for this time limit. An Illuminate\Contracts\Cache\LockTimeoutException will be thrown, the lock can be released.

use Illuminate\Contracts\Cache\LockTimeoutException;

$lock = Cache::lock('post_' . $post_id, 10);

try {
    $lock->block(5);
    ...
    Cache::put('post_' . $post_id, $post_data, 5 * 60);
    $lock->release();
    // Lock acquired after waiting maximum of 5 seconds...
} catch (LockTimeoutException $e) {
    // Unable to acquire lock...
} finally {
    optional($lock)->release();
}
Cache::lock('post_' . $post_id, 10)->block(5, function () use ($post_id, $post_data) {
    // Lock acquired after waiting maximum of 5 seconds...
    ...
    Cache::put('post_' . $post_id, $post_data, 5 * 60);
    $lock->release();
});
like image 22
TsaiKoga Avatar answered Oct 16 '22 12:10

TsaiKoga