How can the map within java.util.HashSet become null

Question

I got NullPointerException at myset.contains(obj) and stacktrace like this:

java.lang.NullPointerException: null
at java.util.HashSet.contains(HashSet.java:203) ~[?:1.8.0_131]

I looked in to source code of HashSet,

private transient HashMap<E,Object> map;
...
202 public boolean contains(Object o) {
203     return map.containsKey(o);
204 }

so seems map is null, and my HashSet object is not. But every init method of HashSet creates a HashMap Object and assigns to map, like

public HashSet() {
    map = new HashMap<>();
}

So my question is, why can map become null in line 203?

This happens sometime in our web server, myset is used by multiple threads. I understand there could be inconsistent issue on a non-threadsafe HashSet, but I don't get why it became null.

Thanks in advance.

Post my code here:

Set<String> tags = data.getTags();
if (tags.contains(tmp.toString())) {
    return true;
}

class definition of data, which is accessed by multiple threads:

class Data
private Set<String> tags;

public Set<String> getTags() {
    if (tags == null) {
        tags = new HashSet<String>();
        // add something to tags
    }
    return tags;
}

Anything can happen if you improperly publish a non-thread-safe object. There's no reason to expect it not to be null. — shmosel, Jul 25 '18 at 07:10
Thanks @shmosel, but I can not find a way to set the private HashSet.map to null, I understand the content of map could be changed by other threads, but why any code can change it to null? — Yulian, Jul 25 '18 at 07:16
It's not *changing* to null, it's null when it's instantiated. — shmosel, Jul 25 '18 at 07:20
@shmosel What do you mean? Given a HashSet instance, we can assume that the HashSet constructor ran (and all constructors we can see create the hashmap object)... How do you explain *"it's null when it's instantiated"*? — ernest_k, Jul 25 '18 at 07:23
@ernest_k Incorrect. You can't assume anything in a multithreaded environment. Statements may appear to run out of order (if at all) from another thread. — shmosel, Jul 25 '18 at 07:28
Yes, make sense @shmosel. So you mean during thread A creates a HashSet instance, thread B accessed it after the object is created but constructor is not called? — Yulian, Jul 25 '18 at 07:38
Yes, that's a possibility. You can avoid it by making the set `final`, or `volatile`, or by using an immutable set, or by synchronizing on access. — shmosel, Jul 25 '18 at 07:40
@shmosel We can assume that these constructors are run by a single thread. There's an explanation to this issue, but it cannot be simply that the app is incorrectly multi-threaded (something is changing `map` **after** construction of the set, same thread or not) — ernest_k, Jul 25 '18 at 07:40
@ernest_k The constructor is run by a single thread, sure, but another thread may access the object instance in a [partially constructed](https://pveentjer.wordpress.com/2007/03/18/immutability-doesnt-guarantee-thread-safety/) state without proper synchronization. — shmosel, Jul 25 '18 at 07:44
I added the definition of class *data*. So I think what happened is that thread-A calls getTags() first and tries to construct the *tags* object, during HashSet construction, BEFORE HashMap is created for *map*, thread-B calls getTags(), tags is not null at this time, so it returned to thread-B, then thread-B calls set.contains(), NPE happened. Thank you both shmosel ernest_k — Yulian, Jul 25 '18 at 07:54
@shmosel, seems I can not mark your comment as answer? Would you please post something as answer so I can choose it? Thank you. — Yulian, Jul 25 '18 at 07:58
@shmosel, I found problem in my previous comment, **tags = new HashSet();**, before the HashSet is **Fully** constructed, it will not be assigned to _tags_, so thread-B can not access a "partially constructed" HashSet, it can be an empty HashSet whose _map_ is not null. And this is different from the duplicated question I think. — Yulian, Jul 25 '18 at 09:05
*before the HashSet is Fully constructed, it will not be assigned to tags*... That's not correct. See my previous comments above and see the linked duplicate. There's a reason why the locking and volatile are necessary, despite the temporary variable. — shmosel, Jul 25 '18 at 09:07
I think the linked duplicate discusses loading the map, not instantiate. I read _As soon as you complete the line someMap = new HashMap... then someMap is no longer null._ I think "new HashSet()" always returns after "HashSet.map = new HashMap()" finishes, until then, tags stays null. So at least my previous explanation is not correct. — Yulian, Jul 25 '18 at 09:21
*I think "new HashSet()" always returns after "HashSet.map = new HashMap()" finishes, until then, tags stays null.* Again, incorrect assumption. Between threads, statements can appear to run out of order, if not properly synchronized. — shmosel, Jul 25 '18 at 17:05

score 0 · Answer 1 · answered Jul 25 '18 at 07:35

0

I think there can be an issue with some other thread, because in this code, it will never change the map. It is just doing conatinsKey check, whoch will not alter the map.

You need to check which places (in various threads), changes are being done in the map, in order to verify that.

answered Jul 25 '18 at 07:35

Yashi Srivastava

187
3
13

Thanks Yashi, this answer may not be very accurate, but it indeed inspired me to look into other places to find root cause. – Yulian Jul 25 '18 at 08:15

How can the map within java.util.HashSet become null

1 Answers1