Concurrency for class with static methods and initialization method
How do I make the following threadafe class?
public class Helper
{
private static Map<String,String> map = null;
public static init()
{
map = new HashMap<String,String>();
// some loading stuff
}
public static getMap()
{
if(map==null) init();
return new HashMap<String,String>(map);
}
}
My ideas so far:
-
Make getMap () synchronized. Problem: Makes the program slower than necessary because synchronization is only required at the beginning of the program and never again.
-
Use a lock. The problem is, the "isLocked" method I'm using here doesn't exist. So what would be the best solution?
public class Helper { private static Map<String,String> map = null; private static Lock lock = new ReentrantLock(); public static init() { lock.lock(); map = new HashMap<String,String>(); // some loading stuff lock.unlock(); } public static getMap() { synchronized {if(map==null) init();} while(lock.isLocked()) {Thread.wait(1);] return new HashMap<String,String>(map); }
}
PS: Sorry for the problem with displaying the second source code. There seems to be a bug using the code after the enum.
PPS: I know that HashMap is not thread safe. But that means I can't write in parallel, reading shouldn't be a problem, right?
PPPS: My final version (inner class only), after John Wint:
protected static class LazyLoaded
{
static final Map<String,String> map;
static
{
Map<String,String> mapInit = new HashMap<>();
// ...loading...
map = Collections.unmodifiableMap(mapInit);
}
}
source to share
I would delegate to a child class
public class Helper
{
public static Map<String,String> getMap()
{
return HelperDelegate.map;
}
private static class HelperDelegate {
private static Map<String,String> map = new HashMap<String,String>();
static{
//load some stuff
}
}
}
Since class loading is thread safe and only happens once, this will allow you to lazily initialize your map.
source to share
Just syncing access to a map link does not solve the problem. After creating the map, you need to sync operations that allow and possibly modify the contents of the map. An exception would be if you knew that the card fills up once during initialization and then only read operations are performed. In this case, you might be fine without explicit synchronization, because your control thread takes care of this (knowing that your programming logic only allows reading after map initialization).
Otherwise, I would suggest using one of the implementations ConcurrentMap
, for example ConcurrentHashMap
, as they are relatively cheap, as long as there is no lock contention, and still provide the necessary thread safety as long as you read and write during the lifetime of the card.
As far as initialization is concerned, since it is a static field (so only one instance) and the cost of creating an empty map at a time is small, I would suggest that you simply declare your map like this:
private static final Map<String,String> map = new ConcurrentHashMap<String,String>();
This way, you don't need conditional code in other methods, there is no problem of link visibility, and the code becomes simpler.
source to share
Double null check in init method:
public init() {
if (map==null) {
synchronized(Helper.class) {
if (map==null)
map = new HashMap();
}
}
}
Btw:
- HashMap is not thread safe.
- putting a synchronized instance method is not thread safe because you are accessing a static variable, that is, it is a class variable, not an instance variable
source to share
First, subsequent map access also needs to be synchronized, unless the new HashMap () is replaced with a new java.util.ConcurrentHashMap (). Then, if, after initialization, the map variable is never set to null again (or to any other value), then it can be declared volatile, the getMap () method will remain unsynchronized, the w18> init () method will be synchronized. The init () method has to check again if the map is null.
By the way, in the source code, 2 HashMaps are generated. getMap () should return a variable, not a new HashMap ().
source to share
Synchronize init and add another null check there
public class Helper
{
private static Map<String,String> map = null;
public synchronized init()
{
if(map != null)
return;
map = new HashMap<String,String>();
// some loading stuff
}
public getMap()
{
if(map==null) init();
return new HashMap<String,String>(map);
}
}
source to share