Avoid a mysterious NPE #321

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@drapp
drapp commented Nov 8, 2012

One user on a particular device was getting an NPE here, meaning that either the header map had a null value, or it was getting mutated in a different thread deleting a key that we expected to be there. I wasn't able to reproduce this locally, but this should protect against any eventuality.

@drapp drapp protect against a mysterious null
One user on a particular device was getting an NPE here, meaning that either the header map had a null value, or it was getting mutated in a different thread deleting a key that we expected to be there. I wasn't able to reproduce this locally, but this should protect against any eventuality.
ff2d112
@fernandezpablo85
Collaborator

I believe the cause for this is (from Map javadocs):

The set is backed by the map, so changes to the map are reflected in the set, and vice-versa. If the map is modified while an iteration over the set is in progress (except through the iterator's own remove operation), the results of the iteration are undefined.

So technically this doesn't prevent the errors from happening but just reduces the problem window (e.g. somebody could still remove stuff from the map after the conditional but before the body of the conditional).

@drapp
drapp commented Nov 8, 2012

It won't help in the case where another thread sets the value of a key to null at exactly the wrong time. Getting the entrySet rather than the keySet should help in the case where another thread removes a header at the wrong time. The null check will also fix the case where there's just a null value in the header map to begin with, which is what I'd put my money on the problem being, since it happens consistently.

@fernandezpablo85
Collaborator

So the fix is not about changing to entrySet but doing the null check, right?

@drapp
drapp commented Nov 8, 2012

I have both to be extra safe, but yeah, I think the null check is what's fixing the actual bug. I'd experiment to winnow down the fix a bit, but like I said, I can't reproduce this locally so I'd have to convince the customer to keep trying different jars

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment