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

31–40 of 90 posts

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

#31

Post by mmastrac »

gwbas1c wrote: post #20

I don't understand something: > stringCache = sc = layoutChars(true); How does this assign an uninitialized String to stringCache? I thought String is immutable and the String object is fully calculated at assignment? Where is the StringBuilder used when stringCache and sc are objects of type "String?" Shouldn't the "problem" be that two threads might call layoutChars at the same time, leading to some extra CPU cycle…

You are correct that this is allowed by the Java spec (1). Final fields are guaranteed to happen before reference assignment, and strings are explicitly mentioned. It appears to be an ARM JVM bug.

1. https://docs.oracle.com/javase/specs/jls/se7/html/jls-17.htm...

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

#32

Post by MichaelGG »

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.

It could cause other correctness issues that could break CAS, for instance (which yeah seems like a poor idea in hindsight since it's so complicated). If this is actually allowed it seems like a huge mistake in the .NET memory model. I've never seen a definitive answer on it (not saying you're wrong).

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

#33

Post by wruza »

Cthulhu_ wrote: post #18
benmmurphy wrote: post #12

is there even a good reason for this to be cached. this almost like a memory leak. if someone wants to cache the result of toString they should be doing it in client code. this shouldn't be something that is forced on everyone.

And yet, it's an interface, as a user you don't notice this performance improvement at all - it returns a string version of an (immutable) data structure, how it does that and whether it caches it is not something that the client can control, and thus not something they should care about too much. What they should and will care about is the performance of BigDecimal.toString(), and if it's cached then the 2nd and onw…

Fyi, there are well-known safe approaches like memoize() to do that instead of roll-your-own. Selecting hidden cache field for every damn value seems like the legacy architectural error. Also, on some platforms not caching this at all would be better than caching under atomic primitive.

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

#34

Post by pvg »

needusername wrote: post #15
pwagland wrote: post #7

Earlier quoted context omitted.

Using the Java terminology, there is no "happens-before" edge. So every action must appear as if it occurred in the specified order in that thread, but other threads are free to see those actions in a different order. 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…

Yes but String.value is final and initialized in the constructor. So it should be safely published.

That's a confusing and glossed over bit in the article. After all the talk of BigDecimal, the test produces an NPE in String but then the conclusion is the problem is in BigDecimal. The String thing should be far more surprising and unexpected than any secondary effect in BigDecimal.

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

#35

Post by aardvark179 »

MichaelGG wrote: post #32
johncolanduoni wrote: post #25

Earlier quoted context omitted.

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.

It could cause other correctness issues that could break CAS, for instance (which yeah seems like a poor idea in hindsight since it's so complicated). If this is actually allowed it seems like a huge mistake in the .NET memory model. I've never seen a definitive answer on it (not saying you're wrong).

It has to be legal for the VM to store the object before it has been entirely initialised, but the question is whether other threads will be able to see it in that uninitialised state. If other threads can see a partially initialised object then you remove most of the nice guarantees final fields give the JIT.

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.

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

#36

Post by alkonaut »

readittwice wrote: post #13
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…

AFAIU the JIT-compiler (or the CPU) could reorder the code: String x = allocate String; // allocate memory only (no initialization) initialize string x // constructor publish x // e.g. store in some global reordered to String x = allocate String; publish x // after publishing some other thread could read unintialized object initialize string x

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.

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

#37

Post by charleslmunger »

aardvark179 wrote: post #22
needusername wrote: post #15

Earlier quoted context omitted.

Yes but String.value is final and initialized in the constructor. So it should be safely published.

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 believe the guarantees around final fields were only added in java 7 - possibly the author is using something older?

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

#38

Post by _pmf_ »

exDM69 wrote: post #16
joncrocks wrote: post #9

Earlier quoted context omitted.

The issue may be to do with the guarantees that the JVM gives you about orders of operations and write visibility across threads. 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-con…

> the key being 'volatile' which is a Java keyword with a specific meaning that constrains the order of operations and write visibility across threads. 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 wit…

The most important thing being that in Java, volatile is a memory barrier, whereas in C it isn't.

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?

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

#40

Post by _pmf_ »

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…

> you can safely pass references to structs between threads with no locks, as long as you never mutate them.

Isn't it required to perform a safe handoff?

Post reply on HN