Skip to content

fix: resolve lock validity check, retry self-blocking, timer leak, an… - #369

Open
philipz wants to merge 2 commits into
mike-marcacci:mainfrom
philipz:main
Open

philipz wants to merge 2 commits into
mike-marcacci:mainfrom
philipz:main

Conversation

@philipz

@philipz philipz commented Sep 14, 2026

Copy link
Copy Markdown

Summary of Changes

This PR resolves several edge-case flaws and behavioral inconsistencies in Redlock and using():

  1. Lock Validity Check on Acquisition & Extension (F1, F5):

    • According to the official Redlock specification (steps 4 & 5), if the elapsed time exceeds the validity time, the lock must be considered failed and released.
    • Fixed acquire() and extend() to throw an ExecutionError and release acquired instances if Date.now() >= expiration, preventing returning expired locks.
  2. Prevent Self-Blocking during Retries (F2):

    • In ACQUIRE_SCRIPT, allow acquiring/refreshing if the key exists with the same value as the current request.
    • Prevents a client from blocking itself in subsequent retry attempts by its own unexpired key left from attempt 1.
  3. Timer Leak in using() (F6):

    • Added lifecycle tracking in using() to ensure that when routine completes, in-flight extension callbacks will not schedule subsequent timers.
    • Cleans up active timeouts so no unhandled timers linger after using() finishes.
  4. Error Handling & Abort Preservation in using() (F7):

    • If routine throws an error, that business error is now preserved rather than being masked by a failed lock.release().
    • If an abort occurred (signal.aborted), signal.error is prioritized over secondary cleanup errors.
  5. Settings Pass-Through:

    • Lock.prototype.extend now accepts optional settings, allowing per-call settings in using() (such as retryCount) to be honored during lock extensions.

Testing

  • Added regression test suite in src/regression.test.ts covering all 5 areas above.
  • All existing and new tests pass cleanly (npm run build && npm test).

In even-node configurations (e.g. N=2, quorumSize=2), a tie vote (1 for,
1 against) would result in all votes being collected without either
side reaching quorumSize. Previously, only done() was invoked, leaving
the outer attempt Promise permanently pending. This fix resolves the
attempt with vote 'against' once all votes are in without reaching quorum.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant