Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Synchronization in Constructors to make it Happens-before

I have a question about how to make an object guaranteed to be thread-safe by the Java Memory Model.

I have read a lot which say that writing a synchronized scope in a constructor does not make sense, but why doesn't it? Yes, it is true that as long as the object under construction is not shared among threads (, which it shouldn't be), no threads other than the constructing one can reach any synchronized(this){...}, so there are no need to make that scope in the constructor in order to exclude them. But synchronized scopes are not only for exclusion; they are also used to create happens-before relationships. JLS.17.4

Here is a sample code to make my point clear.

public class Counter{

    private int count;

    public Counter(int init_value){
        //synchronized(this){
            this.count = init_value;
        //}
    }

    public synchronized int getValue(){
        return count;
    }

    public synchronized void addValue(){
        count++;
    }
}

Think about the case where a thread t0 creates a Counter object and another thread t1 uses it. If there were the synchronized statement in the constructor, it would obviously be guaranteed to be thread-safe. (Since all actions in synchronized scopes have a happens-before relationship with each other.) But if not, i.e. no synchronized statement, does the Java Memory Model still guarantee that the initializing write by t0 of count can be seen by t1? I think not. It is just like f.y can see 0 in the sample code 17.5-1 in JLS.17.5. Unlike the case of JSL.17.5-1, now the second thread accesses the field only from synchronized methods, but I think synchronized statements have no guaranteed effect in this situation. (They don't create any happens-before relationship with any action by t0.) Some say that the rule about a happens-before edge at the end of a constructor guarantees it, but the rule seems to be just saying a constructor happens-before finalize().

Then should I write the synchronized statement in the constructor to make the object thread-safe? Or are there some rules or logics about the Java Memory Model I have missed and actually no need for that? If I am true, even Hashtable of openjdk (though i know it is obsolete) seems not to be thread-safe.

Or am I wrong about the definition of being thread-safe and about the policy for concurrency? If I transfer that Counter object from t0 to t1 by a thread-safe way, e.g. through a volatile variable, there seems to be no problem. (In that case, the construction by t0 happens-before the volatile write, which happens-before the volatile read by t1, which happens-before everything t1 does to it.) Should I always transfer even thread-safe objects (but not immutable) among threads through a way that causes a happens-before relationship?

like image 406
JumpingBeans Avatar asked Jan 06 '23 09:01

JumpingBeans


1 Answers

If the object is safely published (for instance, by instantiating it as someVolatileField = new Foo()), then you don't need synchronization in the constructor. If it's not, then synchronization in the constructor is not enough.

There was a somewhat lengthy discussion on the java concurrency-interest list a few years back about this; I'll provide the summary here. (Full disclosure: I started that discussion, and was involved throughout it.)

Remember that the happens-before edge only applies between one thread releasing the lock, and a subsequent thread acquiring it. So, let's say you have:

someNonVolatileField = new Foo();

There are are three significant sets of actions here:

  1. the object being allocated, with all its fields set to 0/null
  2. the constructor running, which includes an acquire and release of the object's monitor
  3. the object's reference being assigned to someNonVolatileField

Let's say another thread then uses the reference, and calls a synchronized doFoo() method. Now we add two more actions:

  1. reading the someNonVolatileField reference
  2. invoking doFoo(), which includes an acquire and release of the object's monitor

Since the publication to someNonVolatileField wasn't safe, there is a lot of reordering that the system can do. In particular, the reading thread is allowed to see things happening in this order:

  1. the object being allocated, with all its fields set to 0/null
  2. the object's reference being assigned to someNonVolatileField
  3. reading the someNonVolatileField reference
  4. invoking doFoo(), which includes an acquire and release of the object's monitor
  5. the constructor running, which includes an acquire and release of the object's monitor

In this case, there's still a happens-before edge, but goes the other way around from what you want. Specifically, the call to doFoo() formally happens-before the constructor.

This does buy you a little bit; it means that any synchronized method (or block) is guaranteed to see either the full effects of the constructor, or none of those effects; it won't see only part of the constructor. But in practice, you probably want to guarantee that you see the effects of the constructor; that's why you wrote the constructor, after all.

You can get around this by having doFoo() not be synchronized, and instead set up some spin-loop waiting for a flag that says the constructor has run, followed by a manual synchronized(this) block. But by the time you get to that level of complexity, it's probably better to just say "this object is thread safe assuming its initial publication was safe." That's the de-facto assumption for most mutable classes that bill themselves as thread-safe; immutable ones can use final fields, which is thread-safe even in the face of unsafe publication, but which doesn't require explicit synchronization.

like image 75
yshavit Avatar answered Jan 13 '23 08:01

yshavit