fix: fix LockManager deadlock detection not resolving cycles reliably#76
Draft
houssemexo26 wants to merge 2 commits into
Draft
fix: fix LockManager deadlock detection not resolving cycles reliably#76houssemexo26 wants to merge 2 commits into
houssemexo26 wants to merge 2 commits into
Conversation
All five TestLockManager tests failed consistently because isEmpty() only tracked threads waiting to acquire a lock, not threads currently holding one. Once a lock was acquired and unregister() was called, the lock became invisible to isEmpty(), causing assertTrue(manager.isEmpty()) to fire while locks were still held. Changes: - Add heldLocks map to track locks that are currently held (acquired but not yet released). isEmpty() now checks both maps. - Override unlock() in InternalReentrantLock to call unregisterHeld() when the hold count drops to zero (reentrant-safe). - Call registerHeld() after successful acquisition in lock(), lockInterruptibly(), and tryLock() variants. - Fix checkDeadLockOnce() returning false (inconclusive) when locks.get(owner) == null: a null entry means the owner is not waiting — this is a conclusive "no deadlock", not a race. Split the method into checkDeadLock() (spins only for the first hop) and walkDeadLockGraph() (definitive graph walk). - Fix InternalFutureTask.run() which decremented totalUncompletedTasks without ever incrementing it. Replaced with registerHeld/unregisterHeld so running tasks are visible to isEmpty().
houssemexo26
marked this pull request as draft
April 22, 2026 22:25
houssemexo26
changed the title
|
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
1 participant
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.
The
LockManagerdeadlock detector failed to identify actual deadlock cycles becausecheckDeadLockOnce()treated anullvalue fromlocks.get(currentOwner)as inconclusive in all cases. This caused the method to returnfalseprematurely whenever the current owner thread was not registered as waiting. After exhausting its retry attempts, the detection logic exited without throwing an exception, leaving both threads permanently blocked.The underlying issue was an incorrect interpretation of
null. Anullentry in the waiting map is only ambiguous at the initial step, where the direct lock owner might not yet have completed itsregister()andcheckDeadLock()sequence. Beyond that first hop, however,nullis definitive: it indicates the thread is not waiting on any lock and therefore cannot participate in a deadlock cycle.