When using the singleton design pattern, do other methods need to use synchronized keyword to ensure thread safety?

Question

I want to ensure that the following class is thread-safe, should I use the synchronized keyword for other methods? Or use a thread-safe data structure to store Email. What should I do?

public class RecycleStation {
    private static volatile RecycleStation uniqueInstance;
    private static List<Email> recycleEmailList ;

    private RecycleStation() {
        recycleEmailList = new ArrayList<>();
    }

    public static RecycleStation getInstance() {
        if (uniqueInstance == null) {
            synchronized (RecycleStation.class) {
                if (uniqueInstance == null) {
                    uniqueInstance = new RecycleStation();
                }
            }
        }
        return uniqueInstance;
    }

    public void RecycleEmail(Email email) {
        recycleEmailList.add(email);
    }

    public void deleteEmail(Email email) {
        recycleEmailList.remove(email);
    }

    public void clear() {
        recycleEmailList.clear();
    }

}

This is a good question; however I do feel with research you can find better answers than those which I can provide. Read this answer: https://stackoverflow.com/questions/12289105/java-preferred-design-for-using-a-mutable-object-reference-in-another-thread — Haris Nadeem, Apr 04 '19 at 02:12

Barak · Answer 1 · 2019-04-04T11:23:33.820

2

It is better to use a thread-safe, atomic data structure for your Emails than setting synchronized in each of the update methods. Synchronization uses the lock mechanism of the OS, which is an expensive operation and should be avoided.

I'd use ConcurrentLinkedQueue (example below).

Regarding the class itself - The Lazy-Initialization design pattern for a thread-safe singleton might be a good solution too.

It has mostly pro's in your case:

You avoid the use of synchronized in getInstance()
Slightly improves start-up time
Thread-safe :)

Edit: Have a look at this article to better understand why this implementation is thread-safe. @Rafael has made a good point: lazy Initialization, by its own, doesn't necessarily mean thread-safety.

After all is said and done, here's an implementation:

public class RecycleStation  {
    private static RecycleStation uniqueInstance; //You don't need `volatile` here
    private static ConcurrentLinkedQueue<Email> recycleEmailList;

    // Here it all begins:
    private static class SingletonHolder {
        private static RecycleStation  instance = new RecycleStation();
    }
    public static RecycleStation getInstance() {
        return SingletonHolder.instance;
    }
    private RecycleStation () {
        recycleEmailList = new ConcurrentLinkedQueue<>();
    }

    // class functions:
    public void RecycleEmail(Email email) {
        recycleEmailList.add(email);
    }

    public void deleteEmail(Email email) {
        recycleEmailList.remove(email);
    }

    public void clear() {
        recycleEmailList = new ConcurrentLinkedQueue<>();
    }
}

edited Apr 04 '19 at 11:23

answered Apr 04 '19 at 03:02

Barak

1,390
15
27

How can you assume that's thread-safe. What if two threads try to lazily initialize the singleton simultaneously? Lazy initialization and thread-safety are two different things. – Rafael Apr 04 '19 at 03:04
1

That's a good point, I had to give a little further explanation for this specific implementation: The `SingletonHolder` class won't be loaded until we reference it. This means that when we import the `RecyclerStation` class, the instance won't be created until the moment the `getInstance() `method is called (the `SingletonHolder` is referenced). Since only a single thread loads classes (which is the main thread only in this case), only one instance will be created, and therefore - thread-safety & lazy initialization. But correct me if I have any mistake. – Barak Apr 04 '19 at 03:19
Have a look at [this article](https://crunchify.com/lazy-creation-of-singleton-threadsafe-instance-without-using-synchronized-keyword/) – Barak Apr 04 '19 at 03:24
How can it be thread safe if more than one thread will use functions that manipulate `email` list??? – igorepst Apr 04 '19 at 07:50
Another good point. I was too focused on the class initialization - I fixed my answer. However, be advised that setting the methods to `synchronized` isn't the most efficient solution in terms of speed. Have a look at my edit. – Barak Apr 04 '19 at 10:14

score 1 · Accepted Answer · answered Apr 04 '19 at 07:54

First, a Singleton pattern is best implemented as Enum in Java Second, each email manipulation function (clear, Recycle, delete) should be synchronized to ensure thread safety (the link is about an Enum, but the same holds about each and every Sinlgeton implementation):

public synchronized void RecycleEmail(Email email)

When using the singleton design pattern, do other methods need to use synchronized keyword to ensure thread safety?

2 Answers2

Linked