A quibble about a thread example

[Michael Farragher]

I've just read that the following is thread-safe, but I'm not too sure :

public class PersonSet{
   private final Set<Person> mySet = new HashSet<Person>();

   public synchronized void addPerson(Person person){
      myset.add(person);
   }

   public synchronized boolean containsPerson(Person person){
      return mySet.contains(person);
   }
}

(The guy who wrote this said he's not making any assumptions about the thread safety of type Person)

My thinking is :

The no - argument HashSet constructor creates a HashMap , so if the thread that creates this object is different to the thread(s) that access the synchronized methods ( which I assume access this map) , and there's no synchronization on the creation, this class can't be thread-safe.

Any thoughts ?

[Campbell Ritchie]

Michael Farragher wrote:I've just read . . .

Please always tell us where you read something, so we can assess the source for ourselves.
The methods don't access a backing map; that is done by the Set's methods. Please verify from the Java® Language Specification whether instantiation of the field (declaration and initialisation) completes before the constructor or not. If it is possible for thread1 to instantiate the object and thread2 to access it before the backing Set/Map is completely initialised, then it won't be completely thread‑safe. You are liable to suffer an exception from the add method.

[Campbell Ritchie]

Michael Farragher wrote:. . . the thread safety of type Person) . . .

I saw that and immediately thought about its hashCode() and equals() methods instead

[Tim Holloway]

The class methods are synchronizing implicitly on the "this" pseudo-member of the class instance. Since you cannot execute member methods on a class instance until that instance has been constructed, I'd rate it as thread safe, Providing, of course, that no one attempts to access any of the superclass methods.

There is, incidentally, a way to make a collection completely thread-safe, although I'd have to go look at the docs for details.

[Campbell Ritchie]

Tim Holloway wrote:. . . synchronizing . . . "this" . . .

So you think there is no risk of of access whilst the Set is incompletely initialised, and my problem won't occur?

[Stephan van Hulst]

Campbell Ritchie wrote:Please verify from the Java® Language Specification whether instantiation of the field (declaration and initialisation) completes before the constructor or not. If it is possible for thread1 to instantiate the object and thread2 to access it before the backing Set/Map is completely initialised, then it won't be completely thread‑safe. You are liable to suffer an exception from the add method.

Campbell Ritchie wrote:So you think there is no risk of of access whilst the Set is incompletely initialised, and my problem won't occur?

Your problem won't occur. The final keyword has special semantics that ensure that every thread will see the field in its initialized state.

[Tim Holloway]

Campbell Ritchie wrote:

Tim Holloway wrote:. . . synchronizing . . . "this" . . .

So you think there is no risk of of access whilst the Set is incompletely initialised, and my problem won't occur?

The set is initialised via a compile-time expression. I forget the precise term, but what in effect is done is that all assignments made in the class definition are gathered by the compiler and placed in anonymous method that's called before any explicit constructor methods are invoked.

The operator new/newInstance() method do not return until the appropriate constructor (if any) have completed. The construction process itself takes place solely under the thread that the operator new/newInstance() method is invoked from. So the only way you should see threading issues when constructing a new object is if the object construction code itself exposes itself to threading issues. Which java.util.Set does not.

That's why I assert that as long as the set is only accessed by the synchronised methods of PersonSet it's thread-safe.

[Michael Farragher]

So Stephan, if I go

final Object o = new MyObject();

not only will o always point at an instance of MyObject, but all the fields of this type (and their fields) will be initialised, and other threads will see this initialised state provided that it's accessed appropriately ?!?

So if this is class MyObject :

class MyObject{
   
  private Map<String, String> map = new HashMap<>();

  public synchronized Map<String, String> getMap(){
     return map;
  }
}


and thread A creates an instance of this :

final Object o = new MyObject();

and then thread B calls getMap() on this instance, I'd be ok ??!

Without the final modifier, I guess I'd have to synchronize on the assignment.

[Jj Roberts]

On a slight tangent, the first piece of code looks like a good example of where a ReadWriteLock would be more efficient, if the calls to contatinsPerson() are more frequent than to addPerson()

[Campbell Ritchie]

No.
You are returning a reference to the field without taking defensive copies; you are now letting a reference to that Map escape any synchronized context and the thread‑unsafe behaviour of an ordinary HashMap will soon become evident  What's more, any thread‑unsafe behaviour will be reflected back to your Map's original location.

[Michael Farragher]

So if I didn't return the map but just used it, I'd be ok ?

[Campbell Ritchie]

As long as all access is confined to the Map's enclosing instance

[Michael Farragher]

One final question ( and sorry to be pendantic) :

If I removed synchronized from the getMap method, would thread B still be pointing at the object initialised by thread A (of course, not a thread safe object)

[Tim Holloway]

As I said, the object does not officially exist until after the construction process has completed. So unless you explicitly replace that property value, that's the value it will have forever. And if forever is what you want, final will ensure that at a source code level.

An instance has one and only one slot for each of its named properties, so there's no way a given instance could have 2 different set instances assigned to a single set instance property name. For that matter, you have a similar restriction at the class (static) property level, except that in that case, the single property instance value applies to the class as a whole.

[Stephan van Hulst]

Michael Farragher wrote:So Stephan, if I go

final Object o = new MyObject();

not only will o always point at an instance of MyObject, but all the fields of this type (and their fields) will be initialised, and other threads will see this initialised state provided that it's accessed appropriately ?!?

No. The final keyword says nothing about fields of an object that you reference with a final variable.

If you make an instance field final, it ensures that all threads will observe the value (primitive value or reference) you initialize the field with. If you don't make the field final and you immediately access the object you just created from another thread, it's possible that the other thread will see the uninitialized field, even if you assign it a value in an initializer or constructor.

[Michael Farragher]

Which really takes me back to my first question.

If Thread A executes the mySet assignment ( which creates a HashMap) and other threads execute the synchronized methods, how do we know that the map won't be null and point to the correct object ?

[Tim Holloway]

Michael Farragher wrote:Which really takes me back to my first question.

If Thread A executes the mySet assignment ( which creates a HashMap) and other threads execute the synchronized methods, how do we know that the map won't be null and point to the correct object ?

Because they are instance methods, and therefore require an instance of the PersonSet to run against. And the only way to get a PersonSet is to construct one and the constructor will not return a PersonSet until it has been completely constructed. Including the dependent construction of its "mySet" property.

[Campbell Ritchie]

You will have to look through the JLS, as I said earlier, and confirm what Stephan said about the semantics of a final field. Try the section about execution order, maybe here.

[Stephan van Hulst]

Java's memory model is described in the JVMS, not the JLS.

[Michael Farragher]

Just to be clear, I'm not talking about the mySet reference or the final modifier, I'm talking about the map member that's set up in the HashSet constructor

[Campbell Ritchie]

No wonder I couldn't find the definitive answer!

[Mike Simmons]

Stephan van Hulst wrote:Java's memory model is described in the JVMS, not the JLS.

I see it in JLS 17.4 and subsequent sections, including 17.5 final Field Semantics.  There may also be JVMS info as well, though I don't see it in the JVMS Table of Contents.

[Stephan van Hulst]

If you use neither the final keyword nor the synchronized keywords, and PersonSet is accessed by a thread other than the one that creates the instance of PersonSet, it IS possible (but very unlikely) that mySet is initialized with a reference to a HashSet, but the map inside the HashSet has not been initialized.

I'm not a 100% certain, but I'm pretty sure that if you keep the two methods synchronized, other threads will never see the map inside mySet in an inconsistent state, because the synchronized keywords establish a happens-before relationship between calling the constructor in one thread and calling the methods in another thread. If that wasn't the case, a LOT of existing Java code would be broken.

But like I said, for the dirty details on happens-before relationships, you have to study the description of Java's memory model in the JVMS.

[Stephan van Hulst]

Mike, good catch. I guess I was confused.

[Mike Simmons]

Agreed on all points.  Michael may not think he's asking about the final on mySet, but that is still part of what's needed to ensure that all other threads will see the mySet reference (not null) once the constructor completes, and none of them will see that reference before it completes.  Which in turn means all internal details of the HashSet constructor completed before any other thread got a reference.   But most of the work (of making guarantees) is done by the "synchronized" keyword here.

[Tim Holloway]

final has nothing to do with threading, It's simply a marker to indicate that the property can be assigned to once and only once and only within the context of the construction process (or for code blocks, withing the containing block). Depending on the compiler implementation, it's possible you might not even be allowed to read said property before it was set with whatever immutable value you gave to it.

I would be very surprised to learn that any sort of access to a HashSet by multiple threads could scramble its innards that seriously. First of all, because again, the basic elements should have been already in place before the hash constructor returned the newly-constructed hash. Secondly, because the original Java collections were very careful about threading issues even when they didn't contain explicit synchronization elements a là Vector.

[Mike Simmons]

Tim Holloway wrote:final has nothing to do with threading

Right.   That, of course, is why the Java Language Specification put a section titled 17.5 final Field Semantics within its chapter Chapter 17.  Threads and Locks.  Because they have nothing to do with one another.

Except, of course, when they do.  Admittedly, it's often difficult to actually observe the counterintuitive and nonsensical-seeming results that are theoretically possible.  In many modern JVMs the de facto guarantees of either the processing environment or the JVM implementation may make these things impossible.  And even if they're possible, they're often very unlikely.  Still, they're legal, and possible enough that there was a big kerfluffle about the problems with the previous Java memory model, which was largely resolved with JDK 1.5.  Sorry you missed it.  But one of the take-aways of the JDK 1.5 resolution was, use final to protect access to fields that you think are effectively immutable (at least the reference).  Because if you don't, and you use threads, your "immutable" field is not actually immutable.

And don't get me started on the design flaws in Vector and Hashtable.  But at least there, they were relying on synchronization basically everywhere internally (even though that's useless for multi-method sequences of operations).  And if you weren't synchronizing when accessing mutable data from multiple threads, it was understood that things could get really screwed up.  If you've never seen an ArrayList throw a NullPointerException or ArrayAccessException on an add(), well, I have.  I'm sure we could scramble the innards of a HashSet too, given "any sort of access".  For the case described by Michael, if we only drop the final, well it's probably going to be hard to observe any weird behavior.  But as far as guaranteed behavior according to the JLS, we really need the final, to be sure.

[Tim Holloway]

Well, I was limiting myself to sane usages. And in particular what they call "completely constructed" objects, especially since that's what the class under discussion has been dealing with.

The behavior described in 17.5 actually reminds me of the "volatile" variable attribute in C. But in any event, just because you can do multi-threading things in a constructor doesn't mean you should. The fact that the consequences have been analyzed and codified doesn't excuse the fact that if I inherit code like that that I won't come after someone with a baseball bat.

[Mike Simmons]

Oh no - I'm not talking about multi-threading in a constructor.  Not intentionally, anyway.  But the reality is that if you construct in one thread, and then pass the reference to other threads, those other threads may see your object as if it hasn't been fully initialized.  Unless you use final, or synchronization, or locks, etc.  "Sane" usages effectively means, in any potentially multi-threaded environment, if you can possibly make a field final, do so.

[Stephan van Hulst]

Agree with Mike. The fact of the matter is that they made the final keyword function similarly to the volatile keyword in order to deal with edge cases that MAY occur in sane code. Write once, run anywhere. This includes JVMs that take take what the language allows them to do and stretches it to the maximum.

[Tim Holloway]

Mike Simmons wrote:Oh no - I'm not talking about multi-threading in a constructor.  Not intentionally, anyway.  But the reality is that if you construct in one thread, and then pass the reference to other threads, those other threads may see your object as if it hasn't been fully initialized.

I don't think so. I'm pretty sure that application code doesn't get the object reference until the object is - as the spec says - "fully constructed". Thus you' cannot pass the references of a construction-in-progress to other threads.

Obviously if the constructor code itself passes a reference, that's different, but that's the case, as I said, where I bring out the baseball bat.

[Mike Simmons]

Well, here's a relatively clear statement to the contrary, from the oft-quoted JLS 17.5 final Field Semantics:

Example 17.5-1. final Fields In The Java Memory Model wrote:
The program below illustrates how final fields compare to normal fields.

class FinalFieldExample { 
    final int x;
    int y; 
    static FinalFieldExample f;

    public FinalFieldExample() {
        x = 3; 
        y = 4; 
    } 

    static void writer() {
        f = new FinalFieldExample();
    } 

    static void reader() {
        if (f != null) {
            int i = f.x;  // guaranteed to see 3  
            int j = f.y;  // could see 0
        } 
    } 
}

The class FinalFieldExample has a final int field x and a non-final int field y. One thread might execute the method writer and another might execute the method reader.

Because the writer method writes f after the object's constructor finishes, the reader method will be guaranteed to see the properly initialized value for f.x: it will read the value 3. However, f.y is not final; the reader method is therefore not guaranteed to see the value 4 for it.

Emphasis added for the last sentence.  The only difference between x and y is that x is final and y is not, and because of that, reader() may not see the properly initialized value 4.  Unstated is that the only other possible value that could be observed is the default value, 0.

No, it doesn't make sense.  But weird stuff happens in the Java memory model.  In this case, I believe that the main culprit is that JVMs can re-order instructions any way it wants, provided (a) for a single thread executing the code, the result is the same, and (b) for multiple threads, the specific guarantees made by the memory model must be obeyed.  But if you're in a multithreaded environment and you don't find a specific guarantee to prevent bad behavior, you need to be prepared for the possibility that nonsensical results may occur.

[Tim Holloway]

Again, I don't think so.

You cannot pass an object to another thread  before construction is complete. The sole exception is that case where the constructor itself passes itself to another thread (baseball bat time). Final or not, both x and y are required to have been initialized by the time the constructor method exits and the new FinalFieldExample object is a viable object reference (or even a knowable object reference, outside the constructor).

The difference between x and y has nothing to do with timing in this example. All "final" says is that now and forevermore this instance (and since it's constant, all instances) of that FinalFieldExample have an immutable x value of 3.

On the other hand, while the initial instance value of y is 4, once the object is constructed y can have some other value assigned by any thread that has access to the "y" property of that object instance. At that point it's fair game. The value of y will never be 0 unless someone post-construction explicitly sets it to y for any other code or thread than the constructor code (or constructor code chain) and the thread that the object was constructed under. Because, again, there is not yet a known object handle for other threads to screw around with.

[Stephan van Hulst]

Tim Holloway wrote:I'm pretty sure that application code doesn't get the object reference until the object is - as the spec says - "fully constructed".

Only for the thread constructing the object. From the perspective of other threads, the JVM is allowed to perform instructions in any order, meaning a reference to an object can be returned from an object creation expression before the object has been fully constructed.

Thus you' cannot pass the references of a construction-in-progress to other threads.

This is exactly what may happen if you assign the result of an object creation expression to a shared variable.

[Mike Simmons]

[replying to Tim without seeing Stephan's response]

Well, we can want Java to behave that way.  And we can want the JLS to support that.  And yet...

[Tim Holloway]

Stephan van Hulst wrote:
This is exactly what may happen if you assign the result of an object creation expression to a shared variable.

We're going in circles here. The result of an "object creation expression" is a fully-created object. The object reference being assigned does not exist in the general application code space until construction is complete, shared or not.

[Stephan van Hulst]

Tim Holloway wrote:You cannot pass an object to another thread  before construction is complete.

You absolutely can. As Mike pointed out, the JVM is allowed to reorder the execution of instructions, as long as to the thread that executes the instructions, it appears as if the instructions were executed in order. The important thing to realize is that this restriction does not apply to other threads. An important goal of synchronization is not just delineating blocks of code that must be executed atomically, but also establishing the aforementioned happens-before relationships. From the perspective of a thread that executes a block of code, each instruction in the block happens-before the next instruction. The only way to ensure that the same relationship is observed by other threads is to hit memory barriers established by the keywords synchronized, volatile and final.

The value of y will never be 0 unless someone post-construction explicitly sets it to y for any other code or thread than the constructor code (or constructor code chain) and the thread that the object was constructed under.

Untrue. It's a bit unfortunate that the example in the JLS doesn't use a private field for y. The entire point of that example is to point out that the value of y can be observed to be 0 even if it is initialized to a non-zero value and it is never modified thereafter.

[Mike Simmons]

Tim Holloway wrote:

Stephan van Hulst wrote:
This is exactly what may happen if you assign the result of an object creation expression to a shared variable.

We're going in circles here. The result of an "object creation expression" is a fully-created object. The object reference being assigned does not exist in the general application code space until construction is complete, shared or not.

The JLS doesn't use the term "fully constructed" or "fully created", but it does define "completely initialized".  (Guess what section?)

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.

Conspicuously absent is any guarantee that the thread will see correctly initialized values for nonfinal fields.  "Completely initialized" is a useful idea, but it doesn't actually include all the completeness that you might imagine.

Note also, in the example 17.5-1 previously quoted, it says

int j = f.y;  // could see 0

It doesn't say it could see some other value, imagining that some other code not shown here might be setting f.y = 42 or any other value.  It's talking about 0 because that's something that could happen in the code they've actually shown, because even though y = 4 was set in the constructor, it might not be visible here.  Because even in a completely initialized object, a nonfinal field might not appear to have been initialized.

If that's still not convincing you, ask yourself this.  Why did they put section 17.5 in the JLS at all?  The main point of final has  already been well-established by JLS 15.26.1

A variable that is declared final cannot be assigned to (unless it is definitely unassigned (§16 (Definite Assignment))), because when an access of such a final variable is used as an expression, the result is a value, not a variable, and so it cannot be used as the first operand of an assignment operator.

So what, I wonder, are they going on about so much in section 17.5, under Threads?  Is there anything worth saying about final and Threads, that wasn't already covered in preceding sections that weren't about threads?  Why do you suppose that section is there?

[Mike Simmons]

Stephan van Hulst wrote:

Tim Holloway wrote:You cannot pass an object to another thread  before construction is complete.

You absolutely can.

Technically I would say that you cannot pass a reference to an object to another thread before that object is completely initialized, as per the definition just given.  It's just that "completely initialized" doesn't mean anything useful, for non-final fields in a multithreaded environment.  In particular, it does not mean what Tim wants it to mean, that all fields will of course be able to reliably see the initialized values.

[Mike Simmons]

Stephan van Hulst wrote:It's a bit unfortunate that the example in the JLS doesn't use a private field for y.

Yeah, good point.  They could have tweaked it a little bit to make that clearer.