-4

I have following code fragment:

List<SomeObject> allObjects = getNotNullCollection(SomeObject.class, getExpression());
Enumeration2Child mightBeNull = getEnumeration2();
try {
   for (SomeObject someObject : allObjects) {
       if (someObject == null) {
           LOGGER.warn("Null Object " + getId() + " expression : " + getExpression().toString());
           StringBuilder errorMsg = new StringBuilder();
           for (Object element : allObjects) {
               errorMsg.append(element != null ? element.toString() : "Null").append(" - ");
           }
           LOGGER.warn("allObjects: ", errorMsg.toString());
           return false;
       }
       if (someObject.getValue() != null && someObject.getValue().equals(Enumeration1.VALUE) && !(Enumeration2.VALUE2.equals(mightBeNull) || Enumeration2.VALUE3.equals(mightBeNull))) {
           return false;
       }
   }

} catch (NullPointerException e) {

}

And there is an NPE thrown in line:

if (someObject.getValue() != null && someObject.getValue().equals(Enumeration1.VALUE) && !(Enumeration2.VALUE2.equals(mightBeNull) || Enumeration2.VALUE3.equals(mightBeNull)))

As you can see above I check if someObject is null, i also check if someObject.getValue() is null so I should be safe in this part. And the rest of the if statements are just equals calls (and then I assume i would get NPE in equals method and not in if statement). What else might be wrong that I miss here? Thank you

EDIT: Hello, to give us closure on the issue. You were indeed right I didn't notice a crucial info but it was hard to investigate initially. I have added additional logging and found out there are 3 threads executing the same code. Now, this would not be a problem but due to implementation of DB caching getExpression() that returned allObjects , was shared between those threads. When one was checking against null values in this fragment: 'if (someObject.getValue() != null && someObject.getValue().' first check returned not Null and then another thread erased the value, hence the NPE was thrown by someObject.getValue(). So basically a concurrency issue

guruk
  • 55
  • 8

1 Answers1

1

NPEs can only be thrown by the '.' (dot operator) which dereferences an object. Looking at that line, there are 6 variables that you are dereferencing that you don't check for null that I see: Enumeration1, Enumeration2, and Enumeration3 and the VALUE for each. If the NPE were thrown inside an equals, you would see that line instead.

As noted in the comment below, there are exceptions to this:

  1. if you explicitly throw a NullPointerException in which case the stack trace will point to the line with the throw statement.
  2. auto-boxing can result in NPEs. Essentially, it's adding '.' dot operators to the compiled code. This can be nasty. In my experience, the stack trace will show the exception being thrown from a weird place where there is no code (line 1 of the source IIRC.)
JimmyJames
  • 1,356
  • 1
  • 12
  • 24
  • 2
    "NPEs can only be thrown by the '.' (dot operator)" That's not true. `throw new NullPointerException();` has no dot in it, nor does `Integer v = null; v *= 2;`. – Andy Turner Apr 05 '16 at 19:50
  • You are correct, but Enumaration values are static fields in my code. – guruk Apr 06 '16 at 07:44
  • static fields can be set to null. try adding this to the code: `System.out.println("any null? " + (Enumeration1 == null || Enumeration2 == null || Enumeration3 == null));` – JimmyJames Apr 06 '16 at 13:17
  • That is my plan. Just that deployment of changes takes time. – guruk Apr 07 '16 at 08:39