CODEWITHSUNDEEP
javamultithreadingthread-safety
Jonathan
Jonathan

Reputation: 2758

Where am I not Threadsafe?

I'm using an object, which serves roughly as a wrapper for a map of Strings:

public class Wrapper {
  private Map<String, String> values;
  private Formatter formatter;

  public BigDecimal getSpecialValue() {
    String result = values.get("Special");
    return formatter.formatNumber(result);
  }
}

The abovementioned formatter serves roughly as a mapper for a SimpleDateFormat

 public class Formatter {
   private static final NumberFormat NUMBER_FORMAT;

   public BigDecimal formatNumber(String s) {
     Number num = NUMBER_FORMAT.parse(s);
     if (num instanceof Integer) {
       return new BigDecimal((Integer) num);
     } else if (num instanceof Double) {
       return new BigDecimal((Double) num);
     } ...
   }
 }

When I access the getSpecialValue() method by several threads at once, some behaviour accurs, which can only be explained by the concurrent access, for example there may be a NumberFormatException or a ParseException where the parsed string is .430.430 instead of .430 and so on.

There are two aspects which arouse my interest: 1.) The wrapper is only accessed read-only. Although access to the collection is not synchronized, I was under the impression that this should always work. 2.) In one of the first attempts to find the problem I changed the constructor of the Wrapper class to execute the formatNumber method (obviously single threaded), which eliminated all Exceptions down the execution.

Can someone explain?

Edit: The map in the Wrapper class is filled in the constructor, which most definitly happens single threaded. Afterwards the wrapper is designed such that the map is immutable.

Upvotes: 2

Views: 207

Answers (2)

JB Nizet
JB Nizet

Reputation: 691635

  1. NumberFormat is not thread-safe. So storing it in a static variable and use it from multiple threads will lead to problems. Either synchronize every use of this object, or store it in a ThreadLocal variable.

  2. Even if the wrapper is only accessed read-only, there must be some time where the map of values is initialized and populated, and where the formatter is created, else there wouldn't be anything to format. You didn't show the real, complete code of the class, so you might have at least visibility problems in this class as well.

Upvotes: 1

Voo
Voo

Reputation: 30206

Just searching the jdoc for "thread" would've found you the following for the NumberFormat class: Number formats are generally not synchronized. It is recommended to create separate format instances for each thread. If multiple threads access a format concurrently, it must be synchronized externally.

Upvotes: 9

Related Questions