CODEWITHSUNDEEP
java
Code Junkie
Code Junkie

Reputation: 7788

Multi threading with a ConcurrentHashMap

I'm trying to create a method with a ConcurrentHashMap with the following behavior.

  1. Read no lock
  2. Write lock
  3. prior to writing,
    1. read to see if record exist,
    2. if it still doesn't exist, save to database and add record to map.
    3. if record exist from previous write, just return record.

My thoughts. 

private Object lock1 = new Object();

private ConcurrentHashMap<String, Object> productMap;

private Object getProductMap(String name) {

    if (productMap.isEmpty()) {
        productMap = new ConcurrentHashMap<>();
    }

    if (productMap.containsKey(name)) {
        return productMap.get(name);
    }

    synchronized (lock1) {
        if (productMap.containsKey(name)) {
            return productMap.get(name);
        } else {
            Product product = new Product(name);
            session.save(product);
            productMap.putIfAbsent(name, product);
        }
    }

}

Could someone help me to understand if this is a correct approach?

Upvotes: 0

Views: 144

Answers (1)

Makoto
Makoto

Reputation: 106410

There are several bugs here.

  • If productMap isn't guaranteed to be initialized, you will get an NPE in your first statement to this method.
  • The method isn't guaranteed to return anything if the map is empty.
  • The method doesn't return on all paths.
  • The method is both poorly named and unnecessary; you're trying to emulate putIfAbsent which half accomplishes your goal.
  • You also don't need to do any synchronization; ConcurrentHashMap is thread safe for your purposes.

If I were to rewrite this, I'd do a few things differently:

  • Eagerly instantiate the ConcurrentHashMap
  • Bind it to ConcurrentMap instead of the concrete class (so ConcurrentMap<String, Product> productMap = new ConcurrentHashMap<>();)

  • Rename the method to putIfMissing and delegate to putIfAbsent, with some logic to return the same record I want to add if the result is null. The above absolutely depends on Product having a well-defined equals and hashCode method, such that new Product(name) will produce objects with the same values for equals and hashCode if provided the same name.

  • Use an Optional to avoid any NPEs with the result of putIfAbsent, and to provide easier to digest code.

A snippet of the above:

public Product putIfMissing(String key) {
    Product product = new Product(key);
    Optional<Product> result =
         Optional.ofNullable(productMap.putIfAbsent(key, product));
    session.save(result.orElse(product));
    return result.orElse(product);
}

Upvotes: 1

Related Questions