faramir faramir - 3 months ago 10
Java Question

Why here NetBeans display warning about null pointer dereference?

I've following code:

import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

public class NullDereference {

private static final ConcurrentMap<Integer, Object> MAP = new ConcurrentHashMap<>();

public static void main(String[] args) {
Object object = getObject(1);

if (object == null) {
Lock lock = new ReentrantLock();
lock.lock();
try {
lock.newCondition().await(1, TimeUnit.SECONDS);

object = new Object();
object = addObject(object); // [3]
} catch (InterruptedException ex) {
throw new RuntimeException(ex);
} finally { // [1]
lock.unlock(); // [1]
}
}

System.out.println("class: " + object.getClass()); // [2]
}

private static Object getObject(int hashCode) {
return MAP.get(hashCode);
}

private static Object addObject(Object newObject) {
Object oldObject = MAP.putIfAbsent(newObject.hashCode(), newObject);
if (oldObject != null) {
return oldObject;
}

return newObject;
}
}


NetBeans display warning about "Dereferencing possible null pointer" in the line [2]. I'm not sure why. I thought that is because of line [3], but when I comment out line [3] the warning is still here. The warning disappear when I do explicit check for null value before line [2] or when I comment out whole
finally
statement (lines annotated by [1]).

I analyzed code and think that this one is false positive. Am I correct?

I don't want to do additional check for null pointer. What can be wrong with this code? Can I change something to have the code without warning?

Answer

I can reproduce this. Short: you're right, it looks like NetBeans bug. Eclipse and IDEA show no warning here.

Long: Issuing "possible null dereference" warning is not very trivial static analysis as it requires careful traversing all possible control-flow paths (I'm actually writing the similar analyzer, so I know how it's hard). Having finally makes things even more difficult as finally section is executed after every code path, then returns the control to the original code. Proper control-flow graph must make several duplicates of finally block, it's not enough to add several incoming and outgoing edges. I can speculate that NetBeans does this part incorrectly.

Here's incorrect control-flow graph sketch:

[ try { lock.newCondition().await(...) ...} ]
         /          |          \
        /           |           \
       /            |            \
Successful  InterruptedException  other exception
Execution           |               /
     \              |              /
      \             |             /
       \            |            /
     [ finally {  lock.unlock;  }  ]
       /            |            \
      /             |             \
     /              |              \
    |               |               |
[System.out]    [throw RuntimeEx] [throw the original exception]

See that going along this graph edges you can visit final System.out statement after InterruptedException or some other exception. The correct graph must make three copies of finally block:

[ try { lock.newCondition().await(...) ...} ]
         /          |          \
        /           |           \
       /            |            \
Successful  InterruptedException  other exception
Execution           |                |
    |               |                |
[finally_copy1] [finally_copy2]   [finally_copy3]
    |               |                |
    |               |                |
[System.out]    [throw RuntimeEx] [throw the original exception]

This way you can reach the System.out statement only after successful try execution when object is surely assigned.

Comments