2 ms·

I disagree that the others' points about Strings having final values make this irrelevant. It's the references that must be final in order for things to be irr

I disagree that the others' points about Strings having final values 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.

hyperpape 9y ago
How do you read the JLS bits about final fields that mmastrac referenced then? https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.5 https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.htm...
- LgWoodenBadger 9y ago
  You'd have to point out specifics you're mentioning, but the read of stringCache is not through a final field, and that's the source of the problem for any thread that did not initialize or otherwise set stringCache to a value.
  - aardvark179 9y ago
    The fact that value on string is a final field is meant to give you a guarantee that it will have been initialised before any other thread can see the object (assuming the object does not leak itself deliberately in the constructor). This does not depend on the reference to the string itself being final.
  - mmastrac 9y ago
    The JLS specifically points out string's final fields as being important to the memory model for security purposes. This is a JVM implementation bug. 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.
    - LgWoodenBadger 9y ago
      This is only true if you have a safe reference to that String. No safe reference has been established between stringCache and the String instance it points to. 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."; to stringCache = "this is a different value set by another thread without a safe publication of the stringCache reference."; without establishing a happens-before relationship through a memory barrier? 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.
      - cesarb 9y ago
        > Or to put it another way, why do you think other threads would see the value of stringCache change 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.
juancn 9y ago
At most, per the JMM this can only initialize the cached string in BigDecimal more than once, but you can never see a partially initialized string. This is a JVM bug.

hyperpape 9y ago

How do you read the JLS bits about final fields that mmastrac referenced then? https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.5 https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.htm...

LgWoodenBadger 9y ago
You'd have to point out specifics you're mentioning, but the read of stringCache is not through a final field, and that's the source of the problem for any thread that did not initialize or otherwise set stringCache to a value.
- aardvark179 9y ago
  The fact that value on string is a final field is meant to give you a guarantee that it will have been initialised before any other thread can see the object (assuming the object does not leak itself deliberately in the constructor). This does not depend on the reference to the string itself being final.
- mmastrac 9y ago
  The JLS specifically points out string's final fields as being important to the memory model for security purposes. This is a JVM implementation bug. 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.
  - LgWoodenBadger 9y ago
    This is only true if you have a safe reference to that String. No safe reference has been established between stringCache and the String instance it points to. 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."; to stringCache = "this is a different value set by another thread without a safe publication of the stringCache reference."; without establishing a happens-before relationship through a memory barrier? 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.
    - cesarb 9y ago
      > Or to put it another way, why do you think other threads would see the value of stringCache change 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.

juancn 9y ago

At most, per the JMM this can only initialize the cached string in BigDecimal more than once, but you can never see a partially initialized string. This is a JVM bug.