Alex Rivera | Logout

Java ConcurrentHashMap not thread safe.. wth?

Asked 2010-09-16T00:57:54.617
9

I was using HashMap before like 

   public Map<SocketChannel, UserProfile> clients = new HashMap<SocketChannel, UserProfile>();

now I've switched to ConcurrentHashMap to avoid synchronized blocks and now i'm experiencing problems my server is heavily loaded with 200-400 concurrent clients every second which is expected to grow over time.

which now looks like this

public ConcurrentHashMap<SocketChannel, UserProfile> clients = new ConcurrentHashMap<SocketChannel, UserProfile>();

My server design works like this. I have a worker thread(s) for processing huge amounts of packets. Each packet is checked with a packetHandler sub-routine (not part of thread) pretty much any client can call it at anytime it's almost like static but it isn't.

My whole server is mostly single threaded except for the packet processing portion.

Anyways so when someone uses a command like count up all clients online and get some information from them.

It is also possible that clients can get disconnected and removed from ConcurrentHashMap while the counting is in progress (which causes my problems).

Also I'd like to add some code here.

                int txtGirls=0;
                int vidGirls=0;
                int txtBoys=0;
                int vidBoys=0;
                Iterator i = clients.values().iterator();
                while (i.hasNext()) {
                    UserProfile person = (UserProfile)i.next();
                    if(person != null) {
                        if(person.getChatType()) {
                            if(person.getGender().equals("m"))
                                vidBoys++;
                            else //<-- crash occurs here.
                                vidGirls++;
                        } else if(!person.getChatType()) {
                            if(person.getGender().equals("m")
java null try-catch nullpointerexception concurrenthashmap
Edit
Report

2 Answers

16

You need to read the javadocs for the ConcurrentHashMap.values() method, paying special attention to this description of how the iterator for the values() collection works:

"The view's iterator is a "weakly consistent" iterator that will never throw ConcurrentModificationException, and guarantees to traverse elements as they existed upon construction of the iterator, and may (but is not guaranteed to) reflect any modifications subsequent to construction."

The iterator does not give you a consistent snapshot of the state of the values collection, but it is thread-safe, and the expected range of behaviours is clearly specified.

If you want a Map implementation that gives you a consistent snapshot of the values (or keys, or entries) in the map AND allows you to iterate concurrently with modifications, you will probably need to create a custom Map wrapper class (that copies the collections atomically) ... or a full-blown custom Map implementation. Both are likely to be a lot slower than a ConcurrentHashMap for your use-case.

answered 2010-09-16T01:12:07.827
3

I don't see anything wrong with your code. Because it's unlikely that the crash actually occurs at the else, it's likely that the getGender() method is returning null.

answered 2010-09-16T01:16:01.427

Your Answer