Skip to content

Commit

Permalink
Fix potential double acquisition in SingleLock
Browse files Browse the repository at this point in the history
`SingleLock` uses a `ForkJoinPool.ManagedBlocker` for efficient blocking
in a fork-join pool. In the `isReleasable()` method it calls `tryLock()`
without remembering the outcome. The `block` method also unconditionally
called `lockInterruptibly()` even if the lock was already acquired.
The problem that can arise is that the lock is acquired multiple times,
but only released once.

`CompositeLockManagedBlocker` is also updated, so that `block()` checks
for `acquired` before locking.
  • Loading branch information
leonard84 authored and marcphilipp committed Sep 16, 2024
1 parent bc87b07 commit efdb8cf
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ JUnit repository on GitHub.
the store is queried via one of the `get(...)` or `getOrComputeIfAbsent(...)` methods;
however, if a `getOrComputeIfAbsent(...)` invocation results in the computation of a new
value, an exception will still be thrown.
* Fix potential locking issue with `ExclusiveResource` in the `HierarchicalTestExecutorService`.
This could lead to deadlocks in certain scenarios.

[[release-notes-5.11.1-junit-platform-deprecations-and-breaking-changes]]
==== Deprecations and Breaking Changes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ private class CompositeLockManagedBlocker implements ForkJoinPool.ManagedBlocker

@Override
public boolean block() throws InterruptedException {
acquireAllLocks();
acquired = true;
if (!acquired) {
acquireAllLocks();
acquired = true;
}
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,16 @@ private class SingleLockManagedBlocker implements ForkJoinPool.ManagedBlocker {

@Override
public boolean block() throws InterruptedException {
lock.lockInterruptibly();
acquired = true;
if (!acquired) {
lock.lockInterruptibly();
acquired = true;
}
return true;
}

@Override
public boolean isReleasable() {
return acquired || lock.tryLock();
return acquired || (acquired = lock.tryLock());
}

}
Expand Down

0 comments on commit efdb8cf

Please sign in to comment.