diff --git a/CHANGELOG.md b/CHANGELOG.md index 08407e06ecb..6fd4d804cf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ This is the changelog for SpotBugs. This follows [Keep a Changelog v1.0.0](http: Currently the versioning policy of this project follows [Semantic Versioning v2.0.0](http://semver.org/spec/v2.0.0.html). ## Unreleased - 2023-??-?? +### Added +* New bug type `CWO_CLOSED_WITHOUT_OPENED` for locks that might be released without even being acquired. (See [SEI CERT rule LCK08-J](https://wiki.sei.cmu.edu/confluence/display/java/LCK08-J.+Ensure+actively+held+locks+are+released+on+exceptional+conditions))([#2055](https://github.com/spotbugs/spotbugs/pull/2055)) + ### Changed - Bump up Apache Commons BCEL to the version 6.6.1 ([#2223](https://github.com/spotbugs/spotbugs/pull/2223)) - Bump up slf4j-api to 2.0.3 ([#2220](https://github.com/spotbugs/spotbugs/pull/2220)) diff --git a/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/FindUnreleasedLockTest.java b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/FindUnreleasedLockTest.java new file mode 100644 index 00000000000..f2d4ecad79f --- /dev/null +++ b/spotbugs-tests/src/test/java/edu/umd/cs/findbugs/detect/FindUnreleasedLockTest.java @@ -0,0 +1,59 @@ +package edu.umd.cs.findbugs.detect; + +import edu.umd.cs.findbugs.AbstractIntegrationTest; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcher; +import edu.umd.cs.findbugs.test.matcher.BugInstanceMatcherBuilder; +import org.junit.Test; + +import static edu.umd.cs.findbugs.test.CountMatcher.containsExactly; +import static org.hamcrest.CoreMatchers.hasItem; +import static org.hamcrest.MatcherAssert.assertThat; + +public class FindUnreleasedLockTest extends AbstractIntegrationTest { + + @Test + public void doesNotFindSSDBug_ifLockIsClosedInAProperWay() { + performAnalysis("unreleasedLock/ProperlyClosedLock.class"); + + assertAnalyzedClassHasNoBugs(); + } + + @Test + public void findPossibleUnlockCall_withoutLockCall() { + performAnalysis("unreleasedLock/ThrowExceptionAndUnlockBeforeLock.class"); + + assertAnalyzedClassHasBug("CWO_CLOSED_WITHOUT_OPENED", 1); + assertExactPlaceOfBug("ThrowExceptionAndUnlockBeforeLock", "doSomething", "CWO_CLOSED_WITHOUT_OPENED"); + } + + @Test + public void findUnresolvedLock_onExceptionalCondition() { + performAnalysis("unreleasedLock/UnreleasedLock.class"); + + assertAnalyzedClassHasBug("UL_UNRELEASED_LOCK_EXCEPTION_PATH", 1); + assertExactPlaceOfBug("UnreleasedLock", "doSomething", "UL_UNRELEASED_LOCK_EXCEPTION_PATH"); + } + + private void assertAnalyzedClassHasNoBugs() { + final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder() + .build(); + assertThat(getBugCollection(), containsExactly(0, bugTypeMatcher)); + } + + private void assertAnalyzedClassHasBug(String bugType, int numberOfBugs) { + final BugInstanceMatcher bugTypeMatcher = new BugInstanceMatcherBuilder() + .bugType(bugType) + .build(); + assertThat(getBugCollection(), containsExactly(numberOfBugs, bugTypeMatcher)); + } + + private void assertExactPlaceOfBug(String className, String methodName, String bugType) { + final BugInstanceMatcher bugInstanceMatcher = new BugInstanceMatcherBuilder() + .bugType(bugType) + .inClass(className) + .inMethod(methodName) + .build(); + assertThat(getBugCollection(), hasItem(bugInstanceMatcher)); + } + +} diff --git a/spotbugs/etc/findbugs.xml b/spotbugs/etc/findbugs.xml index e9be54de7d9..7252d94e28e 100644 --- a/spotbugs/etc/findbugs.xml +++ b/spotbugs/etc/findbugs.xml @@ -497,7 +497,8 @@ + requirejre="1.5" + reports="UL_UNRELEASED_LOCK,UL_UNRELEASED_LOCK_EXCEPTION_PATH,CWO_CLOSED_WITHOUT_OPENED"/> + diff --git a/spotbugs/etc/messages.xml b/spotbugs/etc/messages.xml index 77c964f2ab9..27acc6c14ab 100644 --- a/spotbugs/etc/messages.xml +++ b/spotbugs/etc/messages.xml @@ -1054,13 +1054,14 @@ Returning a zero length array is generally preferred in this context to returnin
- This detector looks for JSR-166 (java.util.concurrent) -locks which are acquired, but not released on all paths out of the method.  -It is a moderately fast detector.  Note that in order to use this -detector, you need to have the java.util.concurrent package -in the auxiliary classpath (or be analyzing the package itself).

-]]> + This detector looks for JSR-166 (java.util.concurrent) + locks which are acquired, but not released on all paths out of the method.  + The detector also detects if a lock is released without even being opened in the first place. + It is a moderately fast detector.  Note that in order to use this + detector, you need to have the java.util.concurrent package + in the auxiliary classpath (or be analyzing the package itself).

+ ]]>
@@ -8785,6 +8786,18 @@ Using floating-point variables should not be used as loop counters, as they are

]]> + + Method releases lock without opening it + {1} releases lock without acquiring it in the first place +
+ This method releases a JSR-166 (java.util.concurrent) lock, + but it is possible that the lock is not even acquired at this point, + due to an exception thrower method before the locking. +

+ ]]> +
+