Concern about Thread safety

[Dennis Grimbergen]

Hi guys,

I'm not sure about the thread safety of one of my classes. Here is some code.

  
import java.util.ArrayList;
import java.util.List;


public class Main {
    public static void main(String[] args) {
        ThreadSafety threadSafety = ThreadSafety.getInstance();
        threadSafety.doItSafe();
    }
}

class ThreadSafety {
    private static ThreadSafety threadSafety;
    private IMustBeSafe iMustBeSafe;

    private ThreadSafety() {
        iMustBeSafe = new IMustBeSafe();
    }

    public static ThreadSafety getInstance() {
        if(threadSafety == null) {
            threadSafety = new ThreadSafety();
        }
        return threadSafety;
    }

    public synchronized void doItSafe() {
        iMustBeSafe.doItSafe();
    }
}


class IMustBeSafe {
    private List<String> secretStuff;

    IMustBeSafe() {
        secretStuff = new ArrayList<String>();
    }

    public void doItSafe() {
        //ThreadSafety is a Singleton with one synchronized method.
        //This method is called from a synchronized context.
        //Does that mean that any instance variables (like 'secretStuff')
        //and any action (like creating instances of any objects) on this class is thread safe?
    }
}
  

I assume it's Thread safe, but I'm just not sure. Can someone confirm or comment on this?

[Ireneusz Kordal]

Hi,

it's not thread safe.
You must synchronize access to getInstance method.
In case when your singleton is not yet initialized (threadSafety == null) and two (or more) threads call getInstance method at the same time,
there is a chance that two (or more) singletons will be created.
To make this code thread safe either getInstance must be synchronized or singleton object must be created in the static initialization block like that:

static {
    threadSafety = new ThreadSafety();  
}
public static ThreadSafety getInstance() {  
       return threadSafety;
 }      

[Dennis Grimbergen]

Thnx! I didn't notice that.