#234 FindUnreleasedLock doesn't seem to ever work for me

closed-wont-fix
detectors (11)
5
2010-06-04
2010-06-03
Mike Schrag
No

It looks like a change in r11005 might have been the culprit, but if I process the example:

public class LockExample {
private ReentrantLock _lock = new ReentrantLock();

public void doSomething\(\) \{
    \_lock.lock\(\);
\}

}

no warnings are detected. It appears to be that because mightClose is false, it shortcircuits prescreen and decides not to run a check on the method. The following patch fixes this problem, though it's unclear to me what the original change was attempting to resolve, so this might cause a regression:

Index: src/java/edu/umd/cs/findbugs/detect/FindUnreleasedLock.java

--- src/java/edu/umd/cs/findbugs/detect/FindUnreleasedLock.java (revision 12115)
+++ src/java/edu/umd/cs/findbugs/detect/FindUnreleasedLock.java (working copy)
@@ -356,8 +356,8 @@

@Override
public boolean prescreen\(ClassContext classContext, Method method, boolean mightClose\) \{

- if (!mightClose)
- return false;
+// if (!mightClose)
+// return false;
BitSet bytecodeSet = classContext.getBytecodeSet(method);
if (bytecodeSet == null) return false;

Discussion

  • William Pugh

    William Pugh - 2010-06-03
    • assigned_to: nobody --> wpugh
    • status: open --> closed-wont-fix
     
  • William Pugh

    William Pugh - 2010-06-03

    The detector is designed to find cases where a lock is released on some paths but not others. If you have a method that obtains a lock but can't possibly release the lock, then it is probably the case that the method is designed to acquire the lock.

    Without more design information, it is really hard to determine what the developers intent is. Thus, we only signal a warning when we see a clear signal that the developers likely intent is violated.

     
  • Mike Schrag

    Mike Schrag - 2010-06-03

    Tough one ... I'm specifically writing a subclass of this one to detect people who just simply forget to unlock a particular resource.

    It seems to me that if your lock object doesn't escape a particular scope and it's never unlocked in that scope, then it's PROBABLY leaked. So your approach seems legitimate, but I think there's an extension to this of NO unlocks of the resource within its declaring scope -- assuming it doesn't escape as a public var or a return value.

    In my little example, the lock is private, which means you can guarantee that this is wrong. The actual case I'm dealing with is EOEditingContext objects in WebObjects EOF, where people lock, but just forget to put an unlock, which .... is really bad :)

     
  • Mike Schrag

    Mike Schrag - 2010-06-03
    • status: closed-wont-fix --> open-wont-fix
     
  • Keith Lea

    Keith Lea - 2010-06-04
    • status: open-wont-fix --> closed-wont-fix
     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks