Alternative to simple deletion in s_unlock_session()?

[Haravikk]

So I need to do some "locking" in memcached for another purpose, and decided to take a peek at how it's done for sessions in php-memcached.

Using a memcache add is all well and good, since it will fail if the lock already exists (is held), however I'm wondering at the correctness of using a straight up delete to then release it? It occurs to me that there is a possible (even if fairly unlikely) race condition in cases where the lock expires (or is flushed, memcached server holding it goes down etc.) where another process may acquire the non-existent session lock, only to have its lock deleted. In other words process A locks the key, key expires by any means, process B locks the same key, A deletes it under the mistaken assumption it still holds the lock.

With this in mind, my intended locking behaviour for memcached is instead as follows:

1. add a lock-key with the current time stored within it (probably using microtome(true)).
2. To unlock, gets the value of the lock key and see if it still matches the value that was stored (within margin of error). If it does, then it is still ours.
3. Attempt to replace using the returned CAS token. This will fail if the lock expired after getting its value, either because it is gone or another process has inserted a new one, otherwise it will confirm the lock is still ours while also resetting the expiration time.
4. Finally we do the delete, since we know for sure that our original lock is ours for a while yet.

This should eliminate any risk of the lock expiring and another process recreating it, only for the original process to blindly delete it. It also provides the opportunity to detect when another process has unexpectedly taken control of the session (if the replace failed due to an outdated CAS token), allowing the session handler to raise an error, and probably discard its data.

Of course, I realise that locking in memcached is something of a tenuous concept to begin with, there are other things that can go wrong, and the chances of a race condition are extremely slim, however I wanted to share my intended plan for unlocking to see if anyone thought it is something is worth implementing for PHP's memcached session handler?

[Haravikk]

Just to note; it seems that memcached's binary protocol now supports delete commands with a CAS token. This would make it possible to combine the final two steps when using the binary protocol.

[sodabrew]

You're right that this race condition is a problem. I have not reviewed the session code in depth since taking on a maintainer role to shepherd the 3.x releases, but will look for some time to do that.

Unfortunately the libmemcached library does not support CAS on all of the protocol commands that are CAS-enabled in current versions of the memcached server, so this extension is limited by the underlying library in that regard.