Thread safe singleton, locks

user_14649882 0
Viewed 120

I have a Singleton class which is a thread safe, do I need to lock it methods?

        private static volatile JsonWriter instance;
private static final Object mutex = new Object();

ReentrantReadWriteLock lock = new ReentrantReadWriteLock();

private JsonWriter() {
}

public static JsonWriter getInstance() {
    JsonWriter result = instance;
    if (result == null) {
        synchronized (mutex) {
            result = instance;
            if (result == null) {
                instance = result = new JsonWriter();
            }
        }
    }
    return result;
}

do I need to lock each method like this to ensure thread safety?

    public void write(String filePath, ArrayNode content) throws IOException {
    lock.writeLock().lock();
    File file = new File(MASTER_DIR + "/" + filePath);
    mapper.writerWithDefaultPrettyPrinter().writeValue(Files.newOutputStream(file.toPath()), content);

}
thread-safety multithreading singleton design-patterns java
2 Answers

The best performing and thread-safe Singleton implementation is the William Pugh Singleton. You don't need synchronized blocks and ReentrantReadWriteLock.

The William Pugh implementation ensures multi-thread safety and the best performances as it avoids eager creation. In fact, the static member INSTANCE, initialized at class level, will be created only when the nested class is loaded by the class loader, i.e. when the nested class will be actually used. In the following implementation, this can only happen when the getInstance() method is invoked. In fact, conversely from the EagerSingleton, this model actually allows using the enclosing class without causing an eager instantiation. This means that any other method offered by the enclosing class can be safely used without initializing INSTANCE; only the getInstance() method will cause it.

Your implementation could look like this:

class JsonWriter {

    private JsonWriter() {}

    /* ... other methods ... */

    public static JsonWriter getInstance() {
        return SingletonHelper.INSTANCE;
    }

    private static class SingletonHelper {
        private static final JsonWriter INSTANCE = new JsonWriter();
    }
}

A particular thanks to @Holger, who contributed with important clarifications and remarks in the comments.

user_44156250

1 Please dont use synchronized and Lock in one class

2 If you want to use synchronized:

private static volatile JsonWriter instance;   

private JsonWriter() {
}

public synchronized static JsonWriter getInstance() {
    JsonWriter result = instance;
    if (result == null) {
        instance = new JsonWriter();
    }
    return result;
}

3 Or you can use Locks:

private static volatile JsonWriter instance;
private static final Object mutex = new Object();

ReentrantReadWriteLock lock = new ReentrantReadWriteLock();

private JsonWriter() {}

public static JsonWriter getInstance() {
    lock.writeLock().lock();
    JsonWriter result = instance;
    if (result == null) {
       instance = result = new JsonWriter();
      }
    lock.writeLock().unlock();
    return result;
}
user_84059060
Related