Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Java.­math.­BigDecimal toString is not thread safe

vmlens.com

Post reply on HN
Print view View original

51–60 of 90 posts

Re: Java.­math.­BigDecimal toString is not thread safe

#51

Post by LgWoodenBadger »

hyperpape wrote: post #47
LgWoodenBadger wrote: post #44

Earlier quoted context omitted.

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 pro…

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.htm...

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.

Re: Java.­math.­BigDecimal toString is not thread safe

#52

Post by LgWoodenBadger »

pvg wrote: post #45
LgWoodenBadger wrote: post #42

Earlier quoted context omitted.

But the stringCache reference is not final, it's just transient. Because there is nothing within the toString() implementation that enforces a happens-before effect, nor is there anything similar on stringCache itself (such as volatile), threads are not guaranteed to see anything correctly with respect to stringCache, or the layoutChars() method. The problems in layoutChars also affect the thread-safe behavior of toE…

If you look at the trace, the failure is an NPE in String. No amount of 'thread unsafety' in a caller should cause String.length() to NPE out.

If you have a reference to a junk instance, which is what can happen with an unsafe publication (which is what is going on with the stringCache member variable) then all bets are off in terms of what you can expect to see.

Re: Java.­math.­BigDecimal toString is not thread safe

#53

Post by pvg »

LgWoodenBadger wrote: post #52
pvg wrote: post #45

Earlier quoted context omitted.

If you look at the trace, the failure is an NPE in String. No amount of 'thread unsafety' in a caller should cause String.length() to NPE out.

If you have a reference to a junk instance, which is what can happen with an unsafe publication (which is what is going on with the stringCache member variable) then all bets are off in terms of what you can expect to see.

Where is the 'unsafe publication' of String.value that you see? If you can create a 'junk instance' of String, your problem is with String.

Re: Java.­math.­BigDecimal toString is not thread safe

#54

Post by aardvark179 »

LgWoodenBadger wrote: post #51
hyperpape wrote: post #47

Earlier quoted context omitted.

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.htm...

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.

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.

Re: Java.­math.­BigDecimal toString is not thread safe

#55

Post by jmull »

johncolanduoni wrote: post #25
aardvark179 wrote: post #22

Earlier quoted context omitted.

Looking at the article I see he's running on a raspberry pi. I would guess the problem is that the openjdk aarch32 jit is not correctly implementing the java memory model as final fields should have been initialised before an object is visible in another thread. So it's not really that BigDecimal isn't thread safe, but that there's a bug in the JIT.

I'm not sure if this is the case for the JVM, but for .NET it is perfectly legal for the VM to store the object address somewhere after allocation but prior to the constructor running/becoming visible. All of the fields objects will be zeroed so this won't cause memory unsafety, but it's rarely what you expect to happen. Forgetting this is a big source of bugs in double checked locking in .NET.

That seems like a bad thing. I'd think that you'd want your language to guarantee an initializer has completed before letting an object/structure/whatever be visible. At least as the default.

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?

Re: Java.­math.­BigDecimal toString is not thread safe

#56

Post by mmastrac »

LgWoodenBadger wrote: post #51
hyperpape wrote: post #47

Earlier quoted context omitted.

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.htm...

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.

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.

Re: Java.­math.­BigDecimal toString is not thread safe

#57

Post by cesarb »

hyperpape wrote: post #26
dingo_bat wrote: post #21

I am a complete java noob, but it seems like in the test program the same variable testBigDecimal is being shared by different threads without any lock or mutex or any sort of concurrency control! Won't any function working on testBigDecimal be thread unsafe if it was not specifically written assuming it was working on a shared object? Why is this news? Disclaimer: I am so java illiterate that I am applying my C unde…

It's immutable, and immutable objects are thread safe. This is also true in C: you can safely pass references to structs between threads with no locks, as long as you never mutate them. Java just enforces that immutability if you write the class the right way. ...the bug is that it's not really immutable, insofar as it's using an unsafe mutation under the hood to implement its toString method. So it either needs to d…

The rules for C and Java are different. In Java, it's guaranteed that the assignments in the constructor to fields marked as final is visible to other threads before the newly constructed object is (unless you stash a reference to it before returning to the constructor), but C has no such guarantee, so in C you have to put the memory barriers yourself.

Re: Java.­math.­BigDecimal toString is not thread safe

#58

Post by lostmsu »

johncolanduoni wrote: post #25
aardvark179 wrote: post #22

Earlier quoted context omitted.

Looking at the article I see he's running on a raspberry pi. I would guess the problem is that the openjdk aarch32 jit is not correctly implementing the java memory model as final fields should have been initialised before an object is visible in another thread. So it's not really that BigDecimal isn't thread safe, but that there's a bug in the JIT.

I'm not sure if this is the case for the JVM, but for .NET it is perfectly legal for the VM to store the object address somewhere after allocation but prior to the constructor running/becoming visible. All of the fields objects will be zeroed so this won't cause memory unsafety, but it's rarely what you expect to happen. Forgetting this is a big source of bugs in double checked locking in .NET.

I doubt that statement is correct.

Re: Java.­math.­BigDecimal toString is not thread safe

#59

Post by sanxiyn »

readittwice wrote: post #43
alkonaut wrote: post #36

Earlier quoted context omitted.

It could, but as far as I know it then isn't a compliant JIT any more. That is, it's either a bug or a deliberate deviation from the specification.

Why is the JIT not allowed to do this? (Assuming publishing isn't volatile) The reordering could also be caused by the CPU, not only by the JIT. If the JIT isn't allowed to reorder, we also need to prevent the CPU from reordering. Otherwise it doesn't make sense to restrict the JIT from reordering if the CPU could still reorder later. We would need to emit memory barriers for non-volatile loads & stores to prevent re…

Yes, Java requires memory barriers for final fields. Yes, it is expensive on ARM, but if you don't do it you are not implementing Java.

Re: Java.­math.­BigDecimal toString is not thread safe

#60

Post by juancn »

TazeTSchnitzel wrote: post #6

I don't understand the race condition here. 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 br…

There isn't a race condition. There's a JVM bug.

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

Post reply on HN