CODEWITHSUNDEEP
javamultithreadingconcurrencynullpointerexception
Laxmikant
Laxmikant

Reputation: 1611

Getting NullPointerException in multi thread environment in spite of null check

I have a global cache named statisticsCache which is being modified and read by multiple threads simultaneously. Even after I have applied null check but sometime it throws NullPointerException in load run. See below for details:

static Map<String, List<Statistics>> statisticsCache = new ConcurrentHashMap<String, List<Statistics>>();

// method to read the global  cache
List<Statistics> getStatisticsForQueue(String name) {
    List<Statistics> statsCopy = Collections.emptyList();
    List<Statistics> statistics = statisticsCache.get(name);
    if (statistics != null && !statistics.contains(null)) //Here is the check to avoid NPE but sometimes does not works
        statsCopy = new ArrayList<Statistics>(statistics);
    return statsCopy;
}

//method to write into global cache
private void setStatisticsListForQueue(String name) {
    // flushing all pending Last writes of buckets of a queue to DB
    flushStatisticToDB(name);
    if (!statisticsCache.containsKey(name)) {
        statisticsCache.put(name, new ArrayList<Statistics>(1));
    }
    List<Statistics> queueStatisticsList = queueServiceMetaDao
            .findStatisticsByname(name);
    if (queueStatisticsList != null && !queueStatisticsList.isEmpty()) { 
        for (Statistics statistic : queueStatisticsList) {
            // to avoid NPE
            if (statisticsCache.get(name).contains(statistic)) {
                statisticsCache.get(name).remove(statistic);
            }
            statisticsCache.get(name).add(statistic);
        }
    } else {
        statisticsCache.put(name, new ArrayList<Statistics>(1));
    }
}

//method where I am getting NPE 
public long getSize(String name) {
    long size = 0L;
    List<Statistics> statistics = getStatisticsForQueue(name);
    for (Statistics statistic : statistics) {
        size += statistic.getSize(); //Sometimes it throws NullPointerException
    }
    return size;
}

What preventive check should I apply to avoid this?

Upvotes: 0

Views: 2024

Answers (3)

Stephen C
Stephen C

Reputation: 718798

Even after I have applied null check but sometime it throws NullPointerException in load run.

OK, so if you have multiple threads executing this code, then (IMO) the most likely explanation is that the code is not synchronizing properly. Sure, the map itself is a ConcurrentHashMap, so therefore should be thread-safe. However, you have multiple threads creating, accessing and modifying the ArrayLists without any mutual exclusion or other synchronization on the lists.

There are a variety of things that could go wrong. One possibility is that one thread removes an element from a list, and a second thread is simultaneously calling getSize() on the same list. One possible outcome is that the iteration in getSize() will see a stale value of the list's size, and return an array element that has been nulled by the other thread's removal. Since there is no synchronization of the two threads' operations on the list, "all bets are off" with respect to the visibility of one thread's list changes to the other thread.

No matter what exact mechanism leads to the NPEs, what you are doing here does not meet the JLS requirements (see JLS 17.4) that must be met to guarantee predictable behavior.

What preventive check should I apply to avoid this?

You can't solve the problem that way. You need proper synchronization on the lists for ensure that reads and updates cannot overlap. You also need to use putIfAbsent rather than if (! containsKey) { put ... } to deal with another race condition.

Upvotes: 2

Murat Karag&#246;z
Murat Karag&#246;z

Reputation: 37594

The problem is actually not the getSize() method since long cannot be null. The actually NPE is this 

 List<Statistics> statistics = getStatisticsForQueue(name);

for (Statistics statistic : statistics)

If statistics is null the for loop will have a NPE. So what you could do to avoid that is

if(statistics != null)
  for (Statistics statistic : statistics)

Upvotes: 0

dvelle
dvelle

Reputation: 1

I think that statistic.getSize() might be null so you are trying to do:

size += statistic.getSize();

Which throws a NullPointerException

You should check if all Statistics objects have their property "size" != null

Upvotes: 0

Related Questions