Breaking java.lang.String • Hacker News Board

Re: Breaking java.lang.String

#101

Post by taejo » Wed, Jul 12, 2023, 11:30 AM UTC

layer8 wrote: post #74

josephcsible wrote: post #3

This is the exact kind of bug that Rust solves with its borrowing system. The problem is that Java has no way to express the concept of "something that nothing else can modify while I'm looking at it".

Arguably, C++ arguably already solved that specific problem with const safety. I was flabbergasted back in 1995 that Java had dropped that notion.

If the constructor here would take a const reference, it would have the same problem (the existence of a const reference doesn't preclude non-const references to the same object). If it took an argument by value, it wouldn't make a difference whether it was const or not, because no other thread could have access to the same object, because it would be copied.

In Rust, on the other hand, if there's a mutable reference to an object, there can be no other references to the same object at the same time.

Re: Breaking java.lang.String

#102

Post by pshirshov » Wed, Jul 12, 2023, 11:31 AM UTC

moring wrote: post #96

shultays wrote: post #95

Earlier quoted context omitted.

char[] is not an immutable class. Thread safety issue happens on that type, not the fault of String type. It is weird Java considered it a bug, I expect it to be resolved as "wont do"

Thread safety of char[] is the cause of the bug, but the effect is that String instances can violate their invariants, which they shouldn't even in case of thread-unsafe code.

In my opinion the abstraction is leaky but everything works according to the spec.

I might tell you about another "bug" and "invariant violation" which is possible but is not a bug. Try to use Sets or Maps with keys having broken hashcode.

Re: Breaking java.lang.String

#103

Post by marginalia_nu » Wed, Jul 12, 2023, 11:32 AM UTC

pshirshov wrote: post #99

coekie wrote: post #75

Earlier quoted context omitted.

Author of the blog here. Yes, this is a bug. While the javadoc doesn't state it explicitly, immutable classes in the core library are expected to be thread safe. Java tries to be and mostly succeeds at being a safe language, where (by default) the guarantees of its internals cannot be broken no matter what user code does. The JVM preserves its own integrity. There are some other deliberate holes in that safety, such…

In my opinion this is not a bug and there is no general fix for that without significant overhaul of the whole runtime and standard library. In my opinion the author might need to read the vm and memory model spec and understand that such things are expected in Java. There are real bugs in standard library singletons (e.g. concurrent Filesystem calls might fail during singleton initialization, while plugins are loade…

Eh, I don't think the author is to be ridiculed. We're all wrong from time to time, I'm wrong in most of what I say, I'm sure you've been wrong on occasions, this is fine.

The author's views on how concurrency works is very common and taught in schools and textbooks, but also quite inadequate.

Instead let's take this opportunity to talk and teach about the JMM and deepen the collective understanding of the many unintuitive behaviors of multi-threaded applications.

Re: Breaking java.lang.String

#104

Post by haspok » Wed, Jul 12, 2023, 11:34 AM UTC

marginalia_nu wrote: post #93

haspok wrote: post #92

Earlier quoted context omitted.

Something like a ConcurrentModificationException would be nice to have in the "weird shit happens" case, because you may not even be aware of it, and then it might cause headache much further down the line. If you could point upstream that would save a lot of headache and grey hair...

There's no way to reliably detect this type of concurrent modification that doesn't involve introducing expensive barrier operations that evict the CPU cache on multiple points along the code flow and choke out-of-order execution and limit what the JIT can do. If you do not have such a barrier, the CPU is permitted to assume the data hasn't been changed, and may cache it, making detection of concurrent modification i…

> it looks like doSomething can't modify val or oldValue

...sure, if you are using primitives. But here we have an array (reference), the contents of which can be modified by `doSomething(val)` in your example. It is only the reference value that cannot be modified.

Re: Breaking java.lang.String

#105

Post by friendzis » Wed, Jul 12, 2023, 11:36 AM UTC

gizmo686 wrote: post #25

cempaka wrote: post #20

Earlier quoted context omitted.

That's true, but in the case of Strings in particular they are generally considered to be thread-safe by virtue of being immutable (and the Javadocs themselves say this in many places). Concurrently modifying the input character array may seem like willful abuse in this case, but I suppose there might be some carelessly written code out there which does it and the post shows how it would create some weird and very ha…

As the article points out, the only thing the Javadoc guarantees is the subsequent modifications of the character array have no effect. It says nothing about concurrent modifications. The type whose thread safety is in question here is not actually String, but char[]. I'm not going to say it is always wrong to share char[] between threads (as a primitive array of something other than double and long, the Java Spec do…

I don't think this is a fair attack on Java's type system. In both examples you do what is effectively casting to void pointer - escape type system guarantees.

Re: Breaking java.lang.String

#106

Post by marginalia_nu » Wed, Jul 12, 2023, 11:38 AM UTC

haspok wrote: post #104

marginalia_nu wrote: post #93

Earlier quoted context omitted.

There's no way to reliably detect this type of concurrent modification that doesn't involve introducing expensive barrier operations that evict the CPU cache on multiple points along the code flow and choke out-of-order execution and limit what the JIT can do. If you do not have such a barrier, the CPU is permitted to assume the data hasn't been changed, and may cache it, making detection of concurrent modification i…

> it looks like doSomething can't modify val or oldValue ...sure, if you are using primitives. But here we have an array (reference), the contents of which can be modified by `doSomething(val)` in your example. It is only the reference value that cannot be modified.

I was trying to reduce the amount of concepts I had to invoke to make the example make sense.

java.lang.String is a final class so in this case so doSomething can't be overridden, and strong assumptions can be made about doSomething's behavior and in practice it's likely inlined given that this is an extremely hot method.

Since this is a constructor, the reference can't be aliased by e.g. fields in the class, so static analysis of the code is surprisingly easy. The compiler just needs to answer whether this particular reference is mutated by the invoked function(s).

Re: Breaking java.lang.String

#107

Post by coekie » Wed, Jul 12, 2023, 11:54 AM UTC

marginalia_nu wrote: post #90

coekie wrote: post #75

Earlier quoted context omitted.

Author of the blog here. Yes, this is a bug. While the javadoc doesn't state it explicitly, immutable classes in the core library are expected to be thread safe. Java tries to be and mostly succeeds at being a safe language, where (by default) the guarantees of its internals cannot be broken no matter what user code does. The JVM preserves its own integrity. There are some other deliberate holes in that safety, such…

Dunno, this seems like you're violating the java memory model and then then fully expected weird shit happens. If you're mutating a shared state between threads without proper synchronization, and there is no way for String to do this on its own, the CPU is permitted to do stuff like out-of-order execution as there is no happens-before relationship between the write and the read. The JVM is further permitted to optim…

> The reality is that it's permitted to do all these things

Yes, given the code in the String class, the JVM is allowed to execute it in this way. I'm not claiming that is wrong, I'm claiming the code in the String class should not have been written in a way that allowed the JVM to do that.

> If you violate that assumption, you get bizarre and often unpredictable behavior.

Java is (supposed to be) a safe language, that maintains its integrity. It does not have the effect that doing something that was not documented as being safe leads to _undefined behaviour_ and therefor all bets are off (like e.g. C).

I'm pretty sure the JDK maintainers intended for String to be a thread safe class with guarantees that can not be broken with race conditions. It's true that the thread safety of String is not explicitly documented, but I believe that's just an oversight in the docs, that String being thread safe just goes without saying. I realize that's hand-wavy, and I don't have any hard proof for that right now, so I understand not everyone might be convinced of that. I'll wait for the bugfix to make that more clear. I don't expect that fix to have a significant performance impact. Fixing it doesn't necessarily require more barriers or more defensive copies. See e.g. this draft patch (incomplete, but good enough to give a rough idea of how it could be done): https://gist.github.com/amaembo/83b4aeab1cd190f2e1dbd75eacb3... .

But the combination with interning, the "hello world".startsWith("hello") example, is clear evidence that it is a bug. Some code running in the same JVM having triggered some race condition earlier should not break unrelated code, it should not put the JVM in a broken state where such code behaves in nonsensical ways.

Re: Breaking java.lang.String

#108

Post by haspok » Wed, Jul 12, 2023, 11:57 AM UTC

marginalia_nu wrote: post #106

haspok wrote: post #104

Earlier quoted context omitted.

> it looks like doSomething can't modify val or oldValue ...sure, if you are using primitives. But here we have an array (reference), the contents of which can be modified by `doSomething(val)` in your example. It is only the reference value that cannot be modified.

I was trying to reduce the amount of concepts I had to invoke to make the example make sense. java.lang.String is a final class so in this case so doSomething can't be overridden, and strong assumptions can be made about doSomething's behavior and in practice it's likely inlined given that this is an extremely hot method. Since this is a constructor, the reference can't be aliased by e.g. fields in the class, so stat…

OK, this makes sense, thanks.

Re: Breaking java.lang.String

#109

Post by rocqua » Wed, Jul 12, 2023, 11:58 AM UTC

pjmlp wrote: post #52

josephcsible wrote: post #8

Earlier quoted context omitted.

Yes, but Java will happily accept code that doesn't use them where it needs to, leading to bugs like this one. Rust catches that mistake at compile time instead.

Not if the memory has been allocated on a shared memory segment, Rust has no control over what other processes might do.

Sound Rust code would either make functions touching the shared memory marked unsafe, or would do a defensive copy out of shared memory.

Re: Breaking java.lang.String

#110

Post by haspok » Wed, Jul 12, 2023, 12:12 PM UTC

marginalia_nu wrote: post #103

pshirshov wrote: post #99

Earlier quoted context omitted.

In my opinion this is not a bug and there is no general fix for that without significant overhaul of the whole runtime and standard library. In my opinion the author might need to read the vm and memory model spec and understand that such things are expected in Java. There are real bugs in standard library singletons (e.g. concurrent Filesystem calls might fail during singleton initialization, while plugins are loade…

Eh, I don't think the author is to be ridiculed. We're all wrong from time to time, I'm wrong in most of what I say, I'm sure you've been wrong on occasions, this is fine. The author's views on how concurrency works is very common and taught in schools and textbooks, but also quite inadequate. Instead let's take this opportunity to talk and teach about the JMM and deepen the collective understanding of the many unint…

Maybe there could be a JMM 2.0 where at least some of the unexpected behaviour is removed. Maybe even at the cost of performance. If something is misunderstood by >99% of the average programmer population then clearly it is not the best solution to the problem.