Live data from Hacker News

Breaking java.lang.String

wouter.coekaerts.be

191–200 of 206 posts

Re: Breaking java.lang.String

#191

Earlier quoted context omitted.

I haven't done much C++ in a few years but IIRC you can remove const as long as the "original" value isn't const. So `const_cast((*const Foo)foo)` is fine if foo is not const.

Isn’t the linker entitled to put a constant object in a read-only page of the binary if it doesn’t require a ctor at runtime?

Yes. That is why global static `const`s (with exceptions like if they have mutable fields) can't legally have their `const` casted away.

Re: Breaking java.lang.String

#192

Is this actually a bug? The default assumption in Java is that types are not thread-safe unless otherwise specified. Attempting to use types in a way that exceeds their documented thread safety has always been allowed to leave your program in an inconsistent state.

My thinking is the same. I doubt this is an oversight. Making the String constructor thread-safe would likely slow things significantly. Great point about things being assumed not thread-safe. The JDK is pretty thorough with documenting thread-safety.

Coming back to this the answer is probably not strictly about concurrency and so there is a possibility something would need addressed about this.

Re: Breaking java.lang.String

#193

Earlier quoted context omitted.

If you have a data race, your program can have all sorts of inconsistent state in all sorts of objects -- even standard ones. In Java, I don't think you have anything like "undefined behavior", sometimes jokingly but meaningfully called catch-fire or launch-the-nukes semantics, but that doesn't prevent a data race from breaking program invariants in ways that don't immediately or ever generate exceptions. There was a…

That's a lot of talking around the issue and I think you're missing it. We're talking about constants being corrupted in code which has has no control or data flow relationship with the code doing the corrupting. Code which is totally bug free can exhibit impossible behavior. That might not sound disturbing to you if you're taking about strlen(), because languages with unrestricted pointers are full of undefined beha…

This is perhaps an unnecessary clarification, but there's plenty of non-determinism that's perfectly fine. Data races are non-deterministic and bad, but "non-determinism" itself is not a scare word.

I spent a good decade of my career writing C#, which is similar enough to Java for these discussions, and the strlen issue I talked about is not limited to C -- it is also an issue for a managed language like C# or Java. I think you would do well to discuss the problem, instead of dismissing it because myself and the original post I referenced are tainted by the nasal demons of undefined behavior in C. Don't use that as an escape hatch from dealing with the general issue presented in the problem.

I know of no language in existence where even in the face of data races, there is a guarantee that all data invariants are maintained otherwise an exception is thrown. Note that Rust is not such a language, as its goal is to make data races compilation errors, but unsafe code can introduce data races that do not generate compilation errors.

I would also suggest you offer even a sketch of a fix for the String(char[]) constructor if you think that is the source of the bug. I don't think one is possible. I'm not aware of any method to unilaterally atomically copy an unbounded char[], for example. Even if there is, I suggest you go look for other functions that take other kinds of mutable objects as parameters -- I doubt that for each and every case there is a method to unilaterally atomically deep copy those objects. Such copying is just one suggestion, feel free to resolve it any way you think works.

If you don't want to sketch a solution, another experiment you can try is to use a non-thread-safe mapping type like TreeMap, and make structural modifications (add/remove elements, not just modify existing values) from multiple concurrent threads without any synchronization. Can you guarantee that in all cases where invariants are broken, that an exception will be thrown?

I think it would be prudent to do something in intern(), and one could arguably call this a bug in intern(). The string intern table is invisibly and pervasively read shared state, and relatively uncommonly mutated. It would make sense to take extra care when mutating the shared state to attempt to ensure no invalid strings are added.

Re: Breaking java.lang.String

#194

Earlier quoted context omitted.

Well it's trading "bad code can populate the program's string intern table with invalid string objects" for "bad code can instantly deadlock the program", which is not much of an upgrade. And wouldn't you need to do this in every function that uses more than 2 or more related mutable objects 1 time each, or uses 1 mutable object more than 1 time? Do you know of any systems that work like this? This is basically a ver…

It has to be almost deliberately bad to cause a deadlock. This solution stops you from having random occasional breakage.

Are you really going to put while(true) around every nearly every access of mutable state in your programs? That is the implication I get from your comment. I really think you should think hard before trying to dig your way out of a hole like that.

Re: Breaking java.lang.String

#195
post #82
post #48

Earlier quoted context omitted.

"The type whose thread safety is in question here is not actually String, but char[]" Not quite, it is String constructor that has the race condition, char array is incidental there.

The thread-safety guarantees can only be obeyed if the arguments are thread-safe as well. For example, take a java.util.concurrent.ConcurrentHashMap, which is thread-safe. If your value object is mutated concurrently, and is not thread-safe, you can't expect anything sensible to happen if you call containsValue(). Likewise, String's char[] constructor can only be thread safe if there are no concurrent changes to the…

> The thread-safety guarantees can only be obeyed if the arguments are thread-safe as well.

While that’s generally true it’s not the case here, char[] does not invalidate anything upon mutation it just changes.

The issue is that the ctor checks for a property (all chars are under 256) then assumes that holds indefinitely.

Re: Breaking java.lang.String

#196

Is this actually a bug? The default assumption in Java is that types are not thread-safe unless otherwise specified. Attempting to use types in a way that exceeds their documented thread safety has always been allowed to leave your program in an inconsistent state.

My thinking is the same. I doubt this is an oversight. Making the String constructor thread-safe would likely slow things significantly. Great point about things being assumed not thread-safe. The JDK is pretty thorough with documenting thread-safety.

> Making the String constructor thread-safe would likely slow things significantly.

By my reckoning it would speed things up, at least going by the Java code.

Re: Breaking java.lang.String

#197

Earlier quoted context omitted.

It has to be almost deliberately bad to cause a deadlock. This solution stops you from having random occasional breakage.

Are you really going to put while(true) around every nearly every access of mutable state in your programs? That is the implication I get from your comment. I really think you should think hard before trying to dig your way out of a hole like that.

It's not very different from grabbing a mutex on the string. Except that someone is more likely to hang a mutex forever than to make this code hang forever. So yes that's fine. Why wouldn't it be?

And "every time a string is copied" is pretty far from "nearly every access of mutable state".

Also unless the string-changing code has a very elaborate timing attack, won't it only make the constructor loop again about half the time at most?

Re: Breaking java.lang.String

#198
post #82

Earlier quoted context omitted.

The thread-safety guarantees can only be obeyed if the arguments are thread-safe as well. For example, take a java.util.concurrent.ConcurrentHashMap, which is thread-safe. If your value object is mutated concurrently, and is not thread-safe, you can't expect anything sensible to happen if you call containsValue(). Likewise, String's char[] constructor can only be thread safe if there are no concurrent changes to the…

> The thread-safety guarantees can only be obeyed if the arguments are thread-safe as well. While that’s generally true it’s not the case here, char[] does not invalidate anything upon mutation it just changes. The issue is that the ctor checks for a property (all chars are under 256) then assumes that holds indefinitely.

> The issue is that the ctor checks for a property (all chars are under 256) then assumes that holds indefinitely.

You could say that this is a classical TOCTOU bug as the ctor assumes that a mutable argument does not mutate.

The core question[s] here are whether this assumption is and should be in the contract.

Protection against these kinds of bugs is only possible with deep copies of mutable arguments or COW semantics. So you either implement callee defensively and slow down general case (remember, this is stdlib), or you leave concurrency control to the caller. Probably every sane general purpose language does the latter.

Re: Breaking java.lang.String

#199

Earlier quoted context omitted.

It only loops if you modify the string in certain ways partway through the loop. Is that a significant problem? As soon as you stop your indefinite loop of race-condition writes, this loop is guaranteed to finish.

Well it's trading "bad code can populate the program's string intern table with invalid string objects" for "bad code can instantly deadlock the program", which is not much of an upgrade. And wouldn't you need to do this in every function that uses more than 2 or more related mutable objects 1 time each, or uses 1 mutable object more than 1 time? Do you know of any systems that work like this? This is basically a ver…

I get it, you find the infinite loop to be distateful. Here's an alternative: Remove the loop wrapper. Add `throw new ConcurrentModificationException();` at the end. Don't modify the two helper functions. This way, the constructor only gets one chance to try doing the encoding. Change the Javadoc to say that if the array contents are modified while the constructor is executing, the function may (but is not guaranteed to) throw ConcurrentModificationException.

Re: Breaking java.lang.String

#200

Earlier quoted context omitted.

> The thread-safety guarantees can only be obeyed if the arguments are thread-safe as well. While that’s generally true it’s not the case here, char[] does not invalidate anything upon mutation it just changes. The issue is that the ctor checks for a property (all chars are under 256) then assumes that holds indefinitely.

> The issue is that the ctor checks for a property (all chars are under 256) then assumes that holds indefinitely. You could say that this is a classical TOCTOU bug as the ctor assumes that a mutable argument does not mutate. The core question[s] here are whether this assumption is and should be in the contract. Protection against these kinds of bugs is only possible with deep copies of mutable arguments or COW seman…

> You could say that this is a classical TOCTOU bug as the ctor assumes that a mutable argument does not mutate.

Indeed.

> Protection against these kinds of bugs is only possible with deep copies of mutable arguments or COW semantics.

In the general case yes, but in this case no, you can solve the issue and improve performances by implementing the thing correctly, in a single pass over the input. This way you don’t get a split-brain situation.

Post reply on HN