Singleton service in Java and multithreading

Question

I'm developing a Java service using JAX-RS API. I've decided to follow a singleton pattern for it, but now I've some doubts about how to manage concurrency.

The following is a simplified example of my code:

@Singleton 
@Path("/") 
public class NfvDeployer { 

  private static Map<String, List<String>> allocatedNodesOnHost; 
  private static Map<String, String> loadedHosts; 
  private static Map<String, String> loadedNodes; 

  ... 

  @POST 
  @Path("nffgs/{id}/nodes") 
  @Produces(MediaType.APPLICATION_XML) 
  @Consumes(MediaType.APPLICATION_XML) 
  public MyNode postNodeOnNFFG(MyNode Node, @PathParam("id") String id) { 
    ... 
    synchronized (this) { 
    ... 
    allocatedNodesOnHost.get(H).add(Node.ID); 
    } 
  } 

  @GET 
  @Path("hosts/{id}/nodes") 
  @Produces(MediaType.APPLICATION_XML) 
  public MyNodes getNodeFromNFFG(@PathParam("id") String id) { 
    ... 
    for(String S : allocatedNodesOnHost.get(id)) { 
    ... 
    } 
  } 
} 

Do you think that this approach could work? All the GET requests should happen simultaneously, while the POST requests should be serialized. Is it correct?

Answer 1

There are a few things that might not work as intended here. First, since you are declaring your NfvDeployer as a @Singleton, you don't strictly have to make all of your fields static. Since the framework will ensure that only one of these exists, that should cover it.

With regard to synchronization, there are problems with this as-is. Consider the case when you are writing to the List contained as a value in allocatedNodesOnHost and iterating over that same list at the same time. You will end up getting a ConcurrentModificationException. From the Javadoc:

For example, it is not generally permissible for one thread to modify a Collection while another thread is iterating over it.

There are at least two solutions to this, and they depend on what kind of guarantees you want.

Option 1: Consistent View of Data

Suppose the readers of the list and the writers to the list must have a consistent view of the list. Meaning: if somebody is writing a new value it is not ok for the readers to view a slightly older version of the values contained in the list. To account for this, we should synchronize all access to the map (and its list values):

public MyNode postNodeOnNFFG(MyNode node, @PathParam("id") String id) { 
    synchronized (allocatedNodesOnHost) { 
        allocatedNodesOnHost.get(id).add(node.ID); 
    } 
} 

public MyNodes getNodeFromNFFG(@PathParam("id") String id) { 
    synchronized (allocatedNodesOnHost) {
        for(String S : allocatedNodesOnHost.get(id)) { 
            ... 
        } 
    }
} 

You'll notice here that I'm synchronizing on allocatedNodesOnHost rather than this. By synchronizing on this, we have a very coarse grained lock which affects any other synchronization block we might set up. And again, to avoid ConcurrentModificationExceptions, all access to allocatedNodesOnHost needs to be synchronized.

Option 2: Inconsistent view of data

When I say "inconsistent", I don't mean wrong, I mean that it might be slightly out of date, and only in the case of concurrent modification. Essentially, we offload the synchronization we do in Option 1 to structures that the JVM provides that will do it for us.

First, we'll declare our field using a ConcurrentHashMap:

private Map<String, List<String>> allocatedNodesOnHost = new ConcurrentHashMap<>();

A ConcurrentHashMap provides us with a map that allows us to read from and write to a map without having to synchronize it. It avoids blocking where it can, so it's good to use this in a multithreaded application when you know you will have concurrent readers and writers.

And when we create a value in that map, we will use a CopyOnWriteArrayList, which is described as:

A thread-safe variant of ArrayList in which all mutative operations (add, set, and so on) are implemented by making a fresh copy of the underlying array. This is ordinarily too costly, but may be more efficient than alternatives when traversal operations vastly outnumber mutations, and is useful when you cannot or don't want to synchronize traversals, yet need to preclude interference among concurrent threads.

Note the warning: go with this option if you can't/won't synchronize and don't have frequent writers. If you have infrequent writes to a list, this is a viable option.

So when we add a new list to our allocatedNodesOnHost map, we do it like this:

allocatedNodesOnHost.put(hostName, new CopyOnWriteArrayList<>());

And then we can drop synchronization when we access allocatedNodesOnHost:

public MyNode postNodeOnNFFG(MyNode node, @PathParam("id") String id) { 
    allocatedNodesOnHost.get(id).add(node.ID); 
} 

public MyNodes getNodeFromNFFG(@PathParam("id") String id) { 
    for(String S : allocatedNodesOnHost.get(id)) { 
        ... 
    } 
}