Instance variable in a Singleton class accessed by multiple threads

Question

I have a singleton class:

public class School { private HashMap<String, String> students; private static School school; private School(){ students = new HashMap<String, String>(); } public static School getInstance(){ if(school == null){ school = new School(); } return school; } //Method to add student protected void addStudent(String id, String name){ students.put(id,name); } //Method to remove student protected void removeStudent(String id){ students.remove(id); } }

As you can see above, in the singleton class, I have a students variable (a HashMap), there are methods to add & remove student in the class.

In my application, there could be multiple threads using this School class to getInstance() , then adding & removing student. To make the access (especially the access to students instance) be thread safe, I am thinking to use synchorized keyword for getInstanc() method, like:

public synchronized static School getInstance(){ if(school == null){ school = new School(); } return school; }

But I think my trivial change only can make sure only one School instance be created in multi-thread environment. What else do I need to do in order to make it thread safe for accessing the students instance by multiple threads as well. Any good suggestion or comment is appreicated, thanks!

possible duplicate of What is an efficient way to implement a singleton pattern in Java? — chrylis -cautiouslyoptimistic-
– chrylis -cautiouslyoptimistic-, Commented Sep 22, 2013 at 13:15
Does ConcurrentHashMap help ? It looks like that is what you need. docs.oracle.com/javase/7/docs/api/java/util/concurrent/… — Jayan
– Jayan, Commented Sep 22, 2013 at 13:18

Sergey Kalinichenko · Accepted Answer · 2013-09-22 13:18:19Z

Leaving the conversation on whether singletons are evil or not, let's consider only the thread safety issues in your School class:

The shared object is created "lazily" - this needs synchronization to avoid making two instances of School; you have correctly identified and fixed this issue. However, since initializing School does not take much time, you might as well make getInstance() a trivial getter by initializing school = new School() eagerly.
The hash map inside the School - concurrent access to the hash map will result in exceptions. You need to add synchronization around the code that adds, removes, and iterates students to avoid these exceptions.
Access to individual students - once the callers get a Student object, they may start modifying it concurrently. Therefore the Student object needs concurrency protection of their own.

Hi, for individual student, each thread only need to access the getter method(e.g. getStudentName()), is your 3rd point still an issue?
@Mellon If your threads only get Students' properties, but not set them, then you do not need synchronization there.

user1083009 · Accepted Answer · 2013-09-22 13:16:50Z

Synchronizing method makes them thread safe it means only one thread can execute that method at a time.

However, in above situation, i would suggest to synchronized addStudent and removeStudent method only. Or you can synchronized students hash map also using -

Collections.synchronizedMap(new HashMap());

Srikanth · Accepted Answer · 2013-09-22 13:18:45Z

You can use a ConcurrentHashMap or a Collections.synchronizedMap

This article gives a good explanation

Joni · Accepted Answer · 2013-09-22 13:20:21Z

The HashMap implementation is not thread-safe, so bad things may happen if several thread operate on it at the same time. A quick fix is making the map itself synchronized:

 students = Collections.synchronizedMap(new HashMap<String, String>());

Note that if you iterate on this map, the iteration also has to be made in a synchronized block; otherwise other threads may modify the map while you are iterating.

A thread-safe alternative to HashMap is ConcurrentHashMap

Collectives™ on Stack Overflow

Instance variable in a Singleton class accessed by multiple threads

4 Answers 4

2 Comments

Comments

Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

Comments

Comments

Comments

Linked

Related