CODEWITHSUNDEEP
javamultithreadingconcurrency
Consty
Consty

Reputation: 1822

When using thread safe collections in Java, what's the best way to handle concurrency issues?

This might be a very naive of me, but I was always under assumption that the code example below would always work and not crash with a NullPointerException while using thread safe collections in Java. Unfortunately it would seem that the thread t2 is able to remove the item from the list in-between the call to containsKey() and get() methods below the two thread declarations. The commented section shows a way to handle this problem without ever getting a NullPointerException because it simply tests to see if the result from get() is null.

My question is, what's the right way to handle this problem in Java using thread safe collections? Sure I could use a mutex or a synchronized block, but doesn't this sort of defeat a lot of the benefits and ease of use surrounding thread safe collections? If I have to use a mutex or synchronized block, couldn't I just use non-thread safe collection instead? In addition, I've always heard (in academia) that checking code for null value is bad programming practice. Am I just crazy? Is there a simple answer to this problem? Thank you in advance.

package test;

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

public class Test {
    public static void main(String[] args) {
        final Map<Integer, Integer> test = new ConcurrentHashMap<>();

        Thread t1 = new Thread(new Runnable() {
            @Override
            public void run() {
                while(true) {
                    test.put(0, 0);
                    Thread.yield();
                }
            }           
        });

        Thread t2 = new Thread(new Runnable() {
            @Override
            public void run() {
                while(true) {
                    test.remove(0);
                    Thread.yield();
                }
            }           
        });

        t1.start();
        t2.start();

        while(true) {
            if (test.containsKey(0)) {
                Integer value = test.get(0);
                System.out.println(value);
            }
            Thread.yield();
        }   

        // OR
//      while(true) {
//          Integer value = test.get(0);
//          if (value != null) {
//              System.out.println(value);
//          }
//          Thread.yield();
//      }           
    }
}

Upvotes: 2

Views: 4811

Answers (4)

assylias
assylias

Reputation: 328598

In addition, I've always heard (in academia) that checking code for null value is bad programming practice.

with a generic Map, when you write Integer i = map.get(0); then if i is null, you can't conclude that 0 is not in the map - it could be there but map to a null value.

However, with a ConcurrentHashMap, you have the guarantee that there are no null values:

Like Hashtable but unlike HashMap, this class does not allow null to be used as a key or value.

So using:

Integer i = map.get(0);
if (i != null) ...

is perfectly fine.

Upvotes: 2

Sotirios Delimanolis
Sotirios Delimanolis

Reputation: 279920

This 

if (test.containsKey(0)) {
     Integer value = test.get(0);
     System.out.println(value);
}

is still not atomic. A thread can add/remove after you've checked for containsKey.

You need to synchronize on a shared resource around that snippet. Or check for null after you get.

All operations in a ConcurrentHashMap are thread-safe, but they do not extend past method boundaries.

Upvotes: 2

SLaks
SLaks

Reputation: 887345

You're misusing thread-safe collections.

The thread-safe collections cannot possible prevent other code from running between containsKey() and get().

Instead, they provide you with additional thread-safe methods that will atomically check and get the element, without allowing other threads to interfere.

This means that you should never use a concurrent collection through the base collection interfaces (Map or List).
Instead, declare your field as a ConcurrentMap.

In your case, you can simply call get(), which will atomically return null if the key is not found.
There is no alternative to checking for null here. (unlike more elegant function languages, which use the Maybe monad instead)

Upvotes: 2

Peter Lawrey
Peter Lawrey

Reputation: 533492

what's the right way to handle this problem in Java using thread safe collections?

Only perform one operation so it is atomic. It is faster as well.

    Integer value = test.get(0);
    if (value != null) {               
         System.out.println(value);
    }

I've always heard (in academia) that checking code for null value is bad programming practice. Am I just crazy?

Possibly. I think checking for null, if a value can be null is best practice.

Upvotes: 3

Related Questions