Repository navigation
feat: owner-verified session-lock release via ephpm_kv_del_if_eq - #4
Merged
Merged
Conversation
The opt-in session lock was released with a plain del, so a request that overran lockTtlSeconds deleted the lock a later request had since taken. Release it with the atomic compare-and-delete ephpm_kv_del_if_eq (ePHPm v0.11.x+) instead, feature-detected with function_exists(); older runtimes keep the best-effort del. The capability lives in a new optional CompareAndDeleteKvOpsInterface (implemented by SapiKvOps and InMemoryKvOps) rather than on KvOpsInterface, so custom KvOpsInterface backends keep working unchanged and fall back to the plain del.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
The opt-in session lock (
lockSessions: true) was released with a plainephpm_kv_del(). If a request overranlockTtlSeconds, its lock lapsed and a second request could take it. When the first request then reachedclose(), it deleted the second request's lock, which let a third request in at the same time. Now the release uses the atomic compare-and-deleteephpm_kv_del_if_eq($lockKey, $token), so a lock is deleted only while it still holds this request's token.CompareAndDeleteKvOpsInterfacewithsupportsDelIfEq()anddelIfEq(). BothSapiKvOpsandInMemoryKvOpsimplement it.KvSessionHandler::releaseLock()usesdelIfEq()when the backend supports it, and otherwise falls back to the plaindel().lockTtlSecondsabove the slowest request.What was considered and not changed (v0.13.0 functions)
ephpm_kv_set_if_eqfor lock extension: the handler has no extend or refresh step. The lock is taken inread()and released inclose(), and a session handler gets no callback in between, so there is nowhere to extend it.ephpm_kv_getex:updateTimestamp()is already a singleephpm_kv_expire()call (it has the payload in hand and falls back towrite()), andread()doesn't touch the TTL.getexwould not reduce round trips or close a race.Feature detection / backward compatibility
SapiKvOps::supportsDelIfEq()isfunction_exists('ephpm_kv_del_if_eq'). On ePHPm runtimes without it, the release is the same plaindel()as before.KvOpsInterfaceis unchanged. Third-partyKvOpsInterfaceimplementations keep working and keep the old release behaviour.Tests
close(), and our own lock is still freed.KvOpsInterface-only decorator (tests/Support/NoCompareAndDeleteKvOps.php): the unverified delete still happens.InMemoryKvOps::delIfEqsemantics.