CODEWITHSUNDEEP
javadesign-patternssingleton
xun yan
xun yan

Reputation: 69

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

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();
    }

}

Upvotes: 3

Views: 198

Answers (2)

Barak
Barak

Reputation: 1429

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<>();
    }
}

Upvotes: 2

igorepst
igorepst

Reputation: 1240

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)

Upvotes: 1

Related Questions