diff --git a/java/ql/lib/change-notes/2026-01-27-unreleased-lock-pools.md b/java/ql/lib/change-notes/2026-01-27-unreleased-lock-pools.md new file mode 100644 index 000000000000..6ac8a19a7622 --- /dev/null +++ b/java/ql/lib/change-notes/2026-01-27-unreleased-lock-pools.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The query `java/unreleased-lock` no longer applies to lock types with names ending in "Pool", as these typically manage a collection of resources and the `lock` and `unlock` methods typically only lock one resource at a time. This may lead to a reduction in false positives. diff --git a/java/ql/lib/semmle/code/java/Concurrency.qll b/java/ql/lib/semmle/code/java/Concurrency.qll index da2783bc3080..b9dd7bfd99b0 100644 --- a/java/ql/lib/semmle/code/java/Concurrency.qll +++ b/java/ql/lib/semmle/code/java/Concurrency.qll @@ -6,12 +6,16 @@ import semmle.code.java.frameworks.Mockito /** * A Java type representing a lock. - * We identify a lock type as one that has both `lock` and `unlock` methods. + * + * We exclude types with a name ending in "Pool" as they typically manage a + * collection of resources and the `lock` and `unlock` methods typically only + * lock one resource at a time. */ class LockType extends RefType { LockType() { this.getAMethod().hasName("lock") and - this.getAMethod().hasName("unlock") + this.getAMethod().hasName("unlock") and + not this.getName().matches("%Pool") } /** Gets a method that is locking this lock type. */ diff --git a/java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.qhelp b/java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.qhelp index eb8dd211083c..faf03c338bdb 100644 --- a/java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.qhelp +++ b/java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.qhelp @@ -6,18 +6,23 @@

When a thread acquires a lock it must make sure to unlock it again; -failing to do so can lead to deadlocks. If a lock allows a thread to acquire +failing to do so can lead to deadlocks. If a lock allows a thread to acquire it multiple times, for example java.util.concurrent.locks.ReentrantLock, then the number of locks must match the number of unlocks in order to fully release the lock.

+

+Any class that has both lock and unlock methods is +considered a lock type. However, classes with names ending in "Pool" are excluded, +as they typically manage a collection of resources. +

It is recommended practice always to immediately follow a call to lock with a try block and place the call to unlock inside the -finally block. Beware of calls inside the finally block +finally block. Beware of calls inside the finally block that could cause exceptions, as this may result in skipping the call to unlock.

diff --git a/java/ql/test/query-tests/UnreleasedLock/UnreleasedLock.java b/java/ql/test/query-tests/UnreleasedLock/UnreleasedLock.java index eb8de3c496d6..2aadb5044be6 100644 --- a/java/ql/test/query-tests/UnreleasedLock/UnreleasedLock.java +++ b/java/ql/test/query-tests/UnreleasedLock/UnreleasedLock.java @@ -120,4 +120,16 @@ void bad10() { } } } + + static class TestPool { + void lock() {} + void unlock() {} + } + + void good11() { + TestPool pool = new TestPool(); + pool.lock(); // Should be excluded because of "Pool" suffix + f(); + pool.unlock(); + } }