Why SafePoint class from concurrency in practice book marked as @ThreadSafe?

Question

In the Java Concurrency in Practice book you can find following code:

@ThreadSafe
public class SafePoint { 
    @GuardedBy("this") private int x, y;
    private SafePoint(int[] a) { this(a[0], a[1]); }
    public SafePoint(SafePoint p) { this(p.get()); }
    public SafePoint(int x, int y) { 
        this.x = x;
        this.y = y;
    }
    public synchronized int[] get() { return new int[] { x, y };
    }
    public synchronized void set(int x, int y) { this.x = x;
        this.y = y;
    }
}

This marked as @ThreadSafe.

I am pretty sure that this class is not thread-safe (if I understand this term correctly).

Example:

 SafePoint racePublishedSafePoint; // field

 // thread 1:
 racePublishedSafePoint = new SafePoint(1,1);

 // thread 2:
 SafePoint sp;
 while(true){
   SafePoint sp = racePublishedSafePoint;
   if(sp != null) break;
 }
 System.out.println(sp.get()[0]);
 System.out.println(sp.get()[1]);

I believe that there is several possible outcomes:

1. The application doesn't finish
   else
2. If application finished, we can see
   a) 0 0
   b) 0 1
   c) 1 0
   d) 1 1

Am I right?

If true, why did the author mark the class as thread safe? I thought that thread-safe class - class which can be used in concurrent application without sophisticated analysis.

What did author want to say?

P.S.

I have read the Private constructor to avoid race condition

...and my topic is not duplicate.

Northwesterly

"Am I right?" No. You're wrong in all the claims you make in your post, and you seem to claim that you know better than the writers of "Java Concurrency in Practice" when you say "I am fully sure that this class is not thread-safe". – Lederer

@Lederer I fully understand that book authors know more than me, but they interpret his thougts to text and then I try to understand this text. This chain sometimes distort the orifinal thought. wheteher I incorrect understand thread-safe whether something else. And why do you remove your post? – Northwesterly

Let's get back to the basics: SafePoint is thread-safe. Your example code is not thread-safe. – Lederer

@Lederer lets repeat one more time. Please provide difference between java.util.Date and SafePoint. What exact means thread-safety? – Northwesterly

You can use an instance of SafePoint from multiple threads safely, whereas using 'Date` concurrently will result in the corruption of the internal state of the object. YOUR BROKEN EXAMPLE CODE DOES NOT CORRUPT THE INTERNAL STATE OF SafePoint. IT DOES NOT SAFELY PUBLISH THE INSTANCE, SO IT'S BROKEN CODE BUT IT DOES NOT BREAK SafePoint. – Lederer

@Lederer I can wrap Date with synchronized and it will work correct. I just provided race publication. For example immutable objects can be publicated in any way – Northwesterly

That still doesn't make Date a thread-safe class. It just means that you're using external code to (try to) use it in a thread-safe fashion. – Lederer

@Lederer thread safety classes should be publicated safety? immutable - more safety than thread-safe? – Northwesterly

@Lederer I know that you disagree but in provided example you could see corrupted internal state from another thread – Northwesterly

No, the internal state isn't corrupted. Your code is just broken so it displays broken values. I am getting tired of arguing with you. You don't understand things, but you still insist on saying that black is white. – Lederer

@Lederer state is not corrupted but we want to provide visibility atomically, but at this case - it is ptoblem – Northwesterly

Yes it's a problem. You caused that problem by writing broken code. Don't do that. Also don't blame SafePoint. It's you who wrote the broken code, nobody else. If you write broken code, don't ask "why does this code not work correctly", since you obviously know that the code is broken (based on you naming the variable racePublishedSafePoint). – Lederer

@Lederer but if replace SafePoint with String - code will correct – Northwesterly

Yeah well it's not very useful to compare SafePoint and String. – Lederer

@Lederer Why? String Thread-safety? – Northwesterly

They're entirely different classes. – Lederer

@Lederer but my bad code works good for this class – Northwesterly

Do you think that means your code isn't bad? If it works with String, but doesn't work with SafePoint. Are you still blaming the SafePoint class? – Lederer

@Lederer I mereley don't understand criterias if good or bad code. – Northwesterly

Answer 1

I agree with the OP that the example seems to violate the usual understanding of @ThreadSafe guarantees. The unsafe publication races are very real, and of course you can see puzzling behaviors when publishing SafePoint via the race. Some real-life thread-safe classes survive racy publication (String being a notorious example of this), adding up to the confusion.

As far as JCIP narrative goes, I have no paper or electronic copy handy, so reached out to Doug Lea (one of the primary authors), who said:

I think the part of confusion is in the JCIP text. I don't think @ThreadSafe covers publication races, or any other races that could be encountered during construction. Publication safety is treated separately.

Still, I can see how people could think otherwise. It is one of the reasons for us exploring always placing release fences in constructors.

The last part Doug is talking about is briefly described in "All Fields Are Final", including the motivation, experimental patches, and performance estimates.

Stott

@Aleksey could you add the source where you've found Doug Lea's quote? – Hypoblast