Monday, November 25, 2013

Let's say we have very simple Java class MyClass.
public class MyClass {
   private int number;

    public MyClass(int number) {
        this.number = number;
    }

    public int getNumber() {
        return number;
    }

    public void setNumber(int number) {
        this.number = number;
    }
}
There are three ways to construct thread-safe Java class which has some state:
  1. Make it truly immutable
    public class MyClass {
       private final int number;
    
       public MyClass(int number) {
        this.number = number;
       }
    
       public int getNumber() {
        return number;
       }
    
    }
  2. Make field number volatile.
    public class MyClass {
       private volatile int number;
    
       public MyClass(int number) {
        this.number = number;
       }
    
       public int getNumber() {
           return number;
       }
    
       public void setNumber(int number) {
           this.number = number;
       }
    }
  3. Use a synchronized block. Classic version of this approach described in Chapter 4.3.5 of Java Concurrency in practice. And the funny thing about that it has an error in the example which is mentioned in a errata for this book.
    public class MyClass {
       private int number;
    
       public MyClass(int number) {
           setNumber(number);
       }
    
       public synchronized int getNumber() {
           return number;
       }
    
       public synchronized void setNumber(int number) {
           this.number = number;
       }
    }
There is one more fact that should be added to the context of discussion. In a multhithreaded environment JVM is free to reorder instructions outside of synchronized block preserving a logical sequence and happens-before relationships specified by JVM. It may cause publishing object which is not properly constructed yet to another thread.
I've got a couple of questions regarding the third case.
  1. Will it be equivalent to a following piece of code:
    public class MyClass {
       private int number;
    
       public MyClass(int number) {
           synchronized (this){
               this.number = number;
           }
       }
    
       public synchronized int getNumber() {
           return number;
       }
    
       public synchronized void setNumber(int number) {
           this.number = number;
       }
    }
  2. Will a reordering be prevented in the third case or it possible for JVM to reorder intstructions and therefore publish object with default value in field number?
  3. If an answer for the second question is yes than I have one more question.
     public class MyClass {
       private int number;
    
       public MyClass(int number) {
           synchronized (new Object()){
               this.number = number;
           }
       }
    
       public synchronized int getNumber() {
           return number;
       }
    
       public synchronized void setNumber(int number) {
           this.number = number;
       }
    }
This strange-looking synchronized (new Object()) is supposed to prevent reordering effect. Will it work?
Just to be clear, all these examples don't have any practical applications. I'm just curious about nuances of multithreading.
share|improve this question
add comment

2 Answers

synchronized(new Object()) will do nothing, since synchronization is only on the object you synchronize on. So if thread A synchronizes on oneObject, and thread B synchronizes onanotherObject, there is no happens-before between them. Since we can know for a fact that no other thread will ever synchronize on the new Object() you create there, this won't establish a happens-before between any other thread.
Regarding your synchronzied in the constructor, if your object is safely published to another thread, you don't need it; and if it's not, you're probably in a mess of trouble as it is. I asked this question on the concurrency-interest list a bit ago, and an interesting thread resulted. See in particular this email, which points out that even with your constructor synchronized, in the absence of safe publication another thread could see default values in your fields, and this email which (imho) ties the whole thing together.
share|improve this answer
2 
It does do something (Force everything in the block to complete before anything that happens after it in the constructing thread, so if the constructing thread then immediately passes the reference to another thread after the constructor returns, you don't need to worry about non-final fields.) It just doesn't do enough to coverall concurrency hazards. –  Affe Mar 8 '12 at 18:14
1 
@Affe Within a thread you're already guaranteed to see things in the order the code says them -- no synchronization needed. But another thread isn't required to see those effects in the same order, due to data races. In other words, if thread A does A, B, C (where for instance, A is the sync, B is the field assignment, C is setting the reference after new), thread B is allowed to see C A B if there's no happens-before edge -- which synchronized(new Object()) will never provided. For instance, if thread B is on a different core, C could get flushed before A. –  yshavit Mar 8 '12 at 18:58 
2 
synchronizing within the first thread around assignment B forces it to be visible before setting reference C. The memory consistency side effects of the synchronized block aren't coupled to exclusion side effects. They still apply for operations performed in the first thread whether or not the other thread contends for the object monitor. –  Affe Mar 8 '12 at 19:05
 
@Affe I believe in practice, in hotspot, you're right; but the JMM doesn't make any such guarantees that I know of. –  yshavit Mar 8 '12 at 19:14
1 
No, whether my comments are strictly correct under the JSL or not, which I don't have time to research at the moment, synchronizing in the constructor like that is not an absolute fail safe against unsafely published references. –  Affe Mar 8 '12 at 20:21
add comment
In question #3, synchronized(new Object()) is a no-op and will prevent nothing. The compiler can determine that no other threads could possibly synchronize on that object (since nothing else can access the object.) This is an explicit example in Brian Goetz's paper "Java theory and practice: Synchronization optimizations in Mustang".
Even if you did need to synchronize in a constructor, and even if your synchronized(new Object())block was useful - ie, you were synchronizing on a different long-lived object, since your other methods are synchronizing on this, you have visibility problems if you're not synchronizing on the same variable. That is to say, you do indeed want your constructor to also use synchronized(this).
An aside:
Synchronizing on this is considered poor form. Instead, synchronize on some private final field. Callers may synchronize on your object, which could lead to a deadlock. Consider the following:
public class Foo
{
    private int value;
    public synchronized int getValue() { return value; }
    public synchronized void setValue(int value) { this.value = value; }
}

public class Bar
{
    public static void deadlock()
    {
        final Foo foo = new Foo();
        synchronized(foo)
        {
            Thread t = new Thread() { public void run() { foo.setValue(1); } };
            t.start();
            t.join();
        }
    }
}
It's not obvious to callers of the Foo class that this would deadlock. Best to keep your locking semantics internal and private to your class.
share|improve this answer
 
Re point 1, you can unsafely publish an object without leaking this from the constructor. For instance, you could assign it to a non-volatile static field. In that case, another thread would be well within its right to see the reference but not the writes to its fields. –  yshavit Mar 8 '12 at 17:50
 
@yshavit: agreed (although I would consider that case "leaking this" if I'm reading you correctly). Goetz had another article in Java theory and practice about safe construction techniques that I'll add a link to. – Edward Thomson Mar 8 '12 at 17:58
1 
I agree with @yshavit - Java Language Specification 17.5 specifically says (and shows in an example) that non-final fields may need synchronization. More specifically: "An object is considered to be completely initialized when its constructor finishes. A thread that can only see a reference to an object after that object has been completely initialized is guaranteed to see the correctly initialized values for that object's final fields.". Note the penultimate word: no such guarantees exist for non-final fields. –  Ted Hopp Mar 8 '12 at 18:01
 
@TedHopp: Dang, you're both absolutely right. –  Edward Thomson Mar 8 '12 at 18:48
1 
A discussion of this problem is at jeremymanson.blogspot.com/2007/03/…. For some reason I was convinced that the acquire would force a read from main memory, but rereading JLS 17.4 shows this is clearly not the case and a previous release is required. –  Edward Thomson Mar 8 '12 at 18:58
show 2 more comments

No comments: