Skip to content

Commit a35e7b2

Browse files
authored
Merge pull request #21226 from owen-mc/java/update-qhelp-unrelease-lock
Java: Improve qhelp for `java/unreleased-lock` and add lock type exclusion
2 parents f4edff9 + a0c3551 commit a35e7b2

File tree

4 files changed

+29
-4
lines changed

4 files changed

+29
-4
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* 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.

java/ql/lib/semmle/code/java/Concurrency.qll

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@ import semmle.code.java.frameworks.Mockito
66

77
/**
88
* A Java type representing a lock.
9-
* We identify a lock type as one that has both `lock` and `unlock` methods.
9+
*
10+
* We exclude types with a name ending in "Pool" as they typically manage a
11+
* collection of resources and the `lock` and `unlock` methods typically only
12+
* lock one resource at a time.
1013
*/
1114
class LockType extends RefType {
1215
LockType() {
1316
this.getAMethod().hasName("lock") and
14-
this.getAMethod().hasName("unlock")
17+
this.getAMethod().hasName("unlock") and
18+
not this.getName().matches("%Pool")
1519
}
1620

1721
/** Gets a method that is locking this lock type. */

java/ql/src/Likely Bugs/Concurrency/UnreleasedLock.qhelp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,23 @@
66
<overview>
77
<p>
88
When a thread acquires a lock it must make sure to unlock it again;
9-
failing to do so can lead to deadlocks. If a lock allows a thread to acquire
9+
failing to do so can lead to deadlocks. If a lock allows a thread to acquire
1010
it multiple times, for example <code>java.util.concurrent.locks.ReentrantLock</code>,
1111
then the number of locks must match the number of unlocks in order to fully
1212
release the lock.
1313
</p>
14+
<p>
15+
Any class that has both <code>lock</code> and <code>unlock</code> methods is
16+
considered a lock type. However, classes with names ending in "Pool" are excluded,
17+
as they typically manage a collection of resources.
18+
</p>
1419
</overview>
1520

1621
<recommendation>
1722
<p>
1823
It is recommended practice always to immediately follow a call to <code>lock</code>
1924
with a <code>try</code> block and place the call to <code>unlock</code> inside the
20-
<code>finally</code> block. Beware of calls inside the <code>finally</code> block
25+
<code>finally</code> block. Beware of calls inside the <code>finally</code> block
2126
that could cause exceptions, as this may result in skipping the call to <code>unlock</code>.
2227
</p>
2328
</recommendation>

java/ql/test/query-tests/UnreleasedLock/UnreleasedLock.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,4 +120,16 @@ void bad10() {
120120
}
121121
}
122122
}
123+
124+
static class TestPool {
125+
void lock() {}
126+
void unlock() {}
127+
}
128+
129+
void good11() {
130+
TestPool pool = new TestPool();
131+
pool.lock(); // Should be excluded because of "Pool" suffix
132+
f();
133+
pool.unlock();
134+
}
123135
}

0 commit comments

Comments
 (0)