Dori
Dori

Reputation: 18403

Synchronized blocks in constructors

I have a class with a static var like so

private static Object sMyStaticVar;

if i want to assign a value to this var in the constructor I have code like

if(sMyStaticVar == null) sMyStaticVar = new CustomObject(someRuntimeObject);

where someRuntimeObject is an object that is not available at the time my class is loaded and therefore prevents me from declaring my static var like the below

private static Object sMyStaticVar = new CustomObject(someRuntimeObject);

my question is, is the initialization of the static var object in the constructor thread safe? My instincts tell me its not and i should synchronise using the non-runtime class type as the lock, like the below

synchronized(MyClass.class)
{
    if(sMyStaticVar == null) sMyStaticVar = new CustomObject(someRuntimeObject);
}

(as opposed to the runTime type obtained from getClass())

but as my instincts are usually wrong I would be grateful if anyone could shed some light on this for me!

Upvotes: 3

Views: 274

Answers (3)

axtavt
axtavt

Reputation: 242686

This syncrhonization is needed, but it's not enough to achieve thread safety.

You also need to ensure visibility of the field's value when you access it. So, you should either declare that field as volatile or add the same synchronization to every access of that field.

Upvotes: 1

NPE
NPE

Reputation: 500227

You are right, the following is open to race conditions:

if(sMyStaticVar == null) sMyStaticVar = new CustomObject(someRuntimeObject);

Two threads could check sMyStaticVar at the same time, see null, create two objects, etc...

This means that you need synchronization. You could either synchronize on some existing object (there are multiple choices), or you could create an object just for the prurpose, so that you don't have to share the lock with anyone else, risking unnecessary contention:

private static Object sMyStaticVar;
private static Object sMyStaticVarLock = new Object();

Then, in the constructor:

synchronized(sMyStaticVarLock)
{
    if(sMyStaticVar == null) sMyStaticVar = new CustomObject(someRuntimeObject);
}

Upvotes: 1

Bozho
Bozho

Reputation: 597046

If it is static, you should not assign it in the constructor. Make a static initializer method that does that public static synchronized void initialize(someRuntimeObject).

Note the synchronized keyword: it is is the same as synchronizing on MyClass.class

Upvotes: 3

Related Questions