stringCache is only ever assigned to with an initialised string (the output of stringCache()), so given Strings are immutable and assuming Java's assignment is atomic, how can you get a race condition that causes a problem? There shouldn't be a TOCTTOU issue either given stringCache is copied to sc before being null-checked.
What is the statement reordering doing here to break this? How can it be prevented? (Does it need write barriers or something?)
On some machines/CPUs/architectures, this is very easy to organise, since the writes can and will be re-ordered. When this happens you can get the pointer to the string being set, before the contents of the string are actually set. Within the thread, you won't see this, since reorders within the thread of context are done "safely", or at least are visible, but to a different thread/core/CPU those writes may arrive out of order.
So it's not really that BigDecimal isn't thread safe, but that there's a bug in the JIT.
Forgetting this is a big source of bugs in double checked locking in .NET.
NB. You can break all the JMM guarantees by having the constructor share the object reference with another thread. The JMM even says:
> If a reference to an object is shared with other threads during the initial construction of an object, most of the guarantees for final fields of that object can go kerflooey; this includes cases in which other parts of a program continue to use the original value of this field.
I think there's some good talks on the JMM on YouTube, and stuff on Shipilev"s blog.
I mean, that doesn't even give an object a chance to initialize a synchronization mechanism before it's thrown into the wild where it needs one.
It sounds like initializing fields to zero is guaranteed, but is that flexible enough to generally base synchronization on?
this.fooField = new Something()
It's also worth noting there's not anything that special about constructors here; there's a million other ways you can get similar issues when you try to handle concurrency at this level. For example if you do:
this.bar.changeSomeFields()
this.fooField = this.bar
Come to think of it, I objected because I thought it doesn't give you a chance to initialize whatever your object/structure/whatever's synchronization mechanism is. But that's not really true. The call to initialize your synchronization mechanism would contain the barrier you need so a general initialization doesn't generally need one.
Specifically the "third version" exposes the object before the effects of construction are necessarily visible on other threads.
I would not be surprised if this surfaces on other places as well, AFAIK the JDK is full of such benign data races: https://www.youtube.com/watch?v=UykhZ36W04I&index=13&list=PL...
The problems in layoutChars also affect the thread-safe behavior of toEngineeringString() as well.
private transient String stringCache;
public String toString() {
String sc = stringCache;
if (sc == null)
stringCache = sc = layoutChars(true);
return sc;
}
Another thread calling toString() may or may not see sc, or stringCache, assigned to a legitimate String instance, due to the Java Memory Model (happens-before, memory barriers, CPU implementation details, cache levels, etc.).
The data within the String created by layoutChars will be consistent to anybody who has established a happens-before relationship with respect to stringCache (either the thread who created it, or if stringCache was volatile, or if stringCache was final, or if a memory-barrier (synchronized, atomic primitive, etc.) has been erected around it.
None of that is present in this implementation of BigDecimal.toString(), which is why stringCache (and the vars used in layoutChars()) is not a valid publication in the Java Memory Model.
It has nothing to do with whether String is correct, and it's not a defect in the JRE implementation. No safe publication was established with respect to stringCache, therefore the data that other threads see is undefined.
It has everything to do with it. You have to explain how you ended up with a broken instance of String. The internal state of a String instance is hosed, which is a massive violation of all sorts of JVM guarantees and assumptions - this is the OG immutable class, after all. There are two options - a broken String implementation or a broken JVM implementation.
Step back a bit: where did the junk instance come from? The Java rules guarantee that a String created through the String constructors is valid on any thread that can see it. There are no "junk instances" of String. Also, writing to an object reference variable is indivisible: either a thread doesn't see the write, or it sees the full write.
So there are only two states in which a thread can see the stringCache field: either it is null, or it has a reference to a valid String. It might not see the field write from another thread, but in this case this only leads to duplicating work and leaving a bit of garbage on the heap for the GC to clean.
(Of course, once you start using reflection to manipulate internal fields of an object, then all bets are off, but that's not the case here.)
String x = allocate String; // allocate memory only (no initialization)
initialize string x // constructor
publish x // e.g. store in some global
String x = allocate String;
publish x // after publishing some other thread could read unintialized object
initialize string xThis doesn't make sense to me. Am I thinking wrong? Can you point me to documentation/reference that states that the JIT isn't allowed to do this?
Note that a String's value field is final.
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.So to recap: this shouldn't be possible for Strings, since the value field in the String class is final and therefore has a memory barrier. But it could happen for other classes with non-final fields.
String's value is final and assigned a copy of the array, so it's impossible to see a string that's not completely initialized as per the JMM
The author hints at a possible solution when they say "As we see a non-volatile field stringCache", the key being 'volatile' which is a Java keyword with a specific meaning that constrains the order of operations and write visibility across threads.
See http://tutorials.jenkov.com/java-concurrency/java-memory-mod... for a reasonable explanation.
edit: suggestion elsewhere that it could be a wonky JVM implementation, so may not be a BigDecimal problem after all
I must point out that "volatile" in Java means something completely different than what it means in C and related languages.
In multithreaded C code, "volatile" is almost always incorrect. There are only a few correct and portable uses of volatile (such as dealing with setjmp or unix signal handlers).
One thing I'm unsure of: I think in C, a volatile write to some location and a volatile write to another location (even without any data dependency) may not be reordered); is this correct?
All it really guarantees is that reads and writes won't be elided. For example:
*x = 42;
*x = 43;
Volatile predates multithreading in C (it was meant for memory-mapped IO and similar things) and hasn't been updated for it, so it has pretty much no useful properties for multithreading. There are no guarantees about reordering when it comes to multiple threads. You're guaranteed to see reads and writes in the correct order from the perspective of the thread your code is running on, but the compiler won't insert any memory barriers, so it's completely up for grabs how other threads might see it.
(More completely, it depends entirely on your CPU's memory model. If you're on an architecture which does strict ordering at the hardware level then you could potentially take advantage of that. If you aren't then you'll see whatever crazy results hardware reordering might produce.)
In contrast, Java's volatile is only about multi-threading. So really, the only thing that's similar between the two languages' use of volatile is how they spell the keyword.
The double checked locking is broken declaration describes this: https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedL...
Edit: nevermind, I believe others' points about Strings having a final value make this irrelevant.
It's the references that must be final in order for things to be irrelevant, but stringCache (nor the other instance-level variables used by layoutChars() such as scale, intCompact, etc.) is not final. It's just transient, which doesn't affect anything with respect to thread-safe behavior.
If stringCache was final then this problem wouldn't exist. Neither would it exist if stringCache was volatile (though you would potentially duplicate work).
The implementation of toString() (specifically the assignment of stringCache) is a perfect example of an unsafe publication in the Java Memory Model world.
Quote:
>String objects are intended to be immutable and string operations do not perform synchronization. While the String implementation does not have any data races, other code could have data races involving the use of String objects, and the memory model makes weak guarantees for programs that have data races. In particular, if the fields of the String class were not final, then it would be possible (although unlikely) that thread 2 could initially see the default value of 0 for the offset of the string object, allowing it to compare as equal to "/tmp". A later operation on the String object might see the correct offset of 4, so that the String object is perceived as being "/usr". Many security features of the Java programming language depend upon String objects being perceived as truly immutable, even if malicious code is using data races to pass String references between threads.
Assuming you have a safe reference (through a safe publication), then what you quoted comes into play. You do not need any synchronization mechanisms around a String in order to see its correct data, because the String instance is immutable.
Or to put it another way, why do you think other threads would see the value of stringCache change from
stringCache = "this is my value in thread 1.";
stringCache = "this is a different value set by another thread without a safe publication of the stringCache reference.";
You wouldn't. That's why there are AtomicLong, AtomicBoolean, and AtomicInteger. Long, Boolean, and Integer are all immutable, but you can't use them without establishing happens-before. AtomicReference could have been used to get the correct behavior intended by stringCache, but AtomicReference didn't exist until 1.5.
True, other threads might not see the value of stringCache change. However, if and when they see it change, they will see it change to another valid String. They will never see an incompletely initialized String.
Using AtomicReference or volatile gives you stronger guarantees, but they're not necessary here.