Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 6 additions & 2 deletions java/ql/lib/semmle/code/java/Concurrency.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
9 changes: 7 additions & 2 deletions java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.qhelp
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,23 @@
<overview>
<p>
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 <code>java.util.concurrent.locks.ReentrantLock</code>,
then the number of locks must match the number of unlocks in order to fully
release the lock.
</p>
<p>
Any class that has both <code>lock</code> and <code>unlock</code> methods is
considered a lock type. However, classes with names ending in "Pool" are excluded,
as they typically manage a collection of resources.
</p>
</overview>

<recommendation>
<p>
It is recommended practice always to immediately follow a call to <code>lock</code>
with a <code>try</code> block and place the call to <code>unlock</code> inside the
<code>finally</code> block. Beware of calls inside the <code>finally</code> block
<code>finally</code> block. Beware of calls inside the <code>finally</code> block
that could cause exceptions, as this may result in skipping the call to <code>unlock</code>.
</p>
</recommendation>
Expand Down
12 changes: 12 additions & 0 deletions java/ql/test/query-tests/UnreleasedLock/UnreleasedLock.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Loading