diff --git a/exo.kernel.container.mt/src/main/java/org/exoplatform/container/LockManager.java b/exo.kernel.container.mt/src/main/java/org/exoplatform/container/LockManager.java index c602351b9..6419b5251 100644 --- a/exo.kernel.container.mt/src/main/java/org/exoplatform/container/LockManager.java +++ b/exo.kernel.container.mt/src/main/java/org/exoplatform/container/LockManager.java @@ -148,11 +148,10 @@ private void unregister(Lockable l) * registered when A runs its check – so A sees an empty entry for B's * thread and returns "no deadlock", then both threads block forever. *
- * To close this window we retry the walk a few times with a brief yield - * between attempts. If the deadlock graph materialises within the retry - * budget we detect and break it; if it never materialises the lock owner - * really does not hold anything and we let the underlying primitive block - * normally. + * To close this window we spin-wait briefly for the direct owner of the + * contested lock to appear in the waiting map before concluding there is no + * deadlock. Subsequent hops in the wait-for graph are already registered + * (they were blocked earlier), so a null there is conclusive. */ private void checkDeadLock(Lockable l) throws InterruptedException { @@ -168,30 +167,23 @@ private void checkDeadLock(Lockable l) throws InterruptedException + "thread so we cannot have a deadlock"); return; } - // Retry loop: give concurrent threads a chance to complete their own - // register() call before we conclude there is no deadlock. - final int MAX_RETRIES = 10; - for (int attempt = 0; attempt < MAX_RETRIES; attempt++) + // Spin briefly waiting for the direct owner to register its own wait-for + // entry. It may be between register() and checkDeadLock() itself. + final int maxSpins = 50; + for (int spin = 0; spin < maxSpins && !locks.containsKey(owner); spin++) { - boolean conclusive = checkDeadLockOnce(l, owner); - if (conclusive) - return; // confirmed no deadlock - // Inconclusive: other thread hasn't registered yet. Yield and retry. Thread.yield(); } - LOG.trace("No deadlock detected after retries – treating as no deadlock"); + // Now walk the full wait-for graph. + checkDeadLockOnce(l, owner); } /** - * Single deadlock-graph walk. Returns {@code true} when the walk concludes - * "no deadlock" with certainty (e.g. the lock became free, or owner chain - * does not loop back); returns {@code false} when the result is inconclusive - * because a concurrent thread has not yet completed its register() call - * (i.e. {@code locks.get(currentOwner)} returned null while the owner is - * actively running); throws {@link InterruptedException} when a deadlock is - * confirmed. + * Walks the wait-for graph to detect a deadlock cycle involving the current + * thread. Throws {@link InterruptedException} and interrupts the owner if a + * cycle is confirmed; returns silently when no cycle is found. */ - private boolean checkDeadLockOnce(Lockable l, Thread owner) throws InterruptedException + private void checkDeadLockOnce(Lockable l, Thread owner) throws InterruptedException { Thread currentOwner = owner; while (true) @@ -199,19 +191,15 @@ private boolean checkDeadLockOnce(Lockable l, Thread owner) throws InterruptedEx Lockable lock = locks.get(currentOwner); if (lock == null) { - // The owner thread is not waiting on anything right now. - // This could mean it truly holds no other lock (no deadlock), - // OR it has not finished its register() call yet (inconclusive). - // We return false (inconclusive) so the caller retries. - LOG.trace("Owner has no registered lockable resource yet – result inconclusive, will retry"); - return false; + // currentOwner is not waiting on anything – no cycle through it. + LOG.trace("Owner has no registered lockable resource – no deadlock"); + return; } - // We first check the locks Thread lockToAcquireOwner = lock.getOwner(); if (lockToAcquireOwner == null) { LOG.trace("The lockable resource has no owner anymore so we cannot have a deadlock"); - return true; + return; } else if (lockToAcquireOwner == Thread.currentThread()) { @@ -219,15 +207,13 @@ else if (lockToAcquireOwner == Thread.currentThread()) if (owner == l.getOwner() && l.isLocked()) { LOG.debug("A deadlock has been detected, both threads will be interrupted"); - // The owner did not change so we have a deadlock, so - // we will interrupt both threads owner.interrupt(); throw new InterruptedException(); } else { LOG.trace("The owner has changed or the resource is no more locked so we cannot have a deadlock"); - return true; + return; } } currentOwner = lockToAcquireOwner; @@ -443,4 +429,4 @@ private static interface Lockable */ boolean isLocked(); } -} +} \ No newline at end of file