If this is a bug, it's a bug in the array cloning logic. It's easy to publish a reference to an incomplete data structure without a memory barrier before the assignment that publishes it - though x86/x64 platforms will save you by retiring writes in order [1]. You don't need to use the JDK to replicate this logic; you could write this code yourself. If it crashes the JVM, it's probably a violation of memory safety - it's not inconceivable that there's an exploit here, but at the very least it's a DoS vector for untrusted bytecode.
Why did you expect it was threadsafe? I can find nothing in its javadoc that would lead me to believe it was. Most of the Java standard library isn't threadsafe, that why there are concurrent objects in addition to their non-threadsafe cousins and language level synchronization statements.
https://docs.oracle.com/javase/8/docs/api/java/lang/reflect/...
They basically have to be thread safe, because they're globally accessible. If you can't control access to a call that isn't thread safe, then you can't call it safely at all if there's any code in your process that you don't control (like, say, third-party libraries, or even first-party libraries).
I think it's reasonable to assume that any documented API must at least be callable.
Indeed it must be so -- at least it would be very strange to have a thread-unsafe static method where the documentation doesn't spell out how to use it safely[1]. Even in that case it would be a weird thing to even have a thread-unsafe static method (except accidentally, of course).
[1] The JDK documentation is pretty good on spelling out such caveats, IME.
The `volatile_var = local_var = X` seems safe - even if you break it into two assignments it only uses the local var afterward, so SecureRandom should be fully initialized in that thread. And though SecureRandom is non-final, it does look safe to use with double-initialization - it self-seeds if not explicitly seeded (it's not in this case), so even double-init shouldn't repeat a value (which is as strong of a statement as SecureRandom allows here - no idea if it makes that claim!).
So I'd think it only matters if `volatile` doesn't guarantee that non-final object fields are fully initialized... and I can't find anything that explicitly states one way or another, so I'm not sure. If it doesn't make that guarantee, then yeah - this could publish a partly-initialized SecureRandom without a generator, which would probably crash. But the contents of https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.htm... imply to me that write-then-read is equivalent to a synchronized block or any other monitor sequence (and in the method they only use the local var, so it doesn't matter for the creator-thread), so I suspect it's fine. Just can't claim any further.
@exabrial: care to elaborate? What's unsafe / have you seen crashes due to uninitialized SecureRandom?
Keep in mind that data races in Java are not undefined behaviour. There are quite a few places where authors of standard library deliberately left out synchronization, for example in String.hashCode.
EDIT: In this case numberGenerator is volatile so it introduces happens-before relations between write and read.
Exactly, especially so because that would totally break sandboxing. Granted, applets are way beyond their best-before date, but there may be other reasons a SecurityManager sandbox is in place, perhaps for isolating multiple .wars from different sources, or maybe someone even wrote a sandbox for executing user-supplied code.
Java's raison d'être was arguably the byte code verifier. Being able to crash the JVM with validated byte code is always a bug.
Hm. Are other things in the reflect package threadsafe? By default I wouldn't expect it to be since it doesn't seem to claim it is, but it is a little surprising that a language-introspection feature could just crash because you're doing it at the same time as someone else. And it seems hard to guard against - you could carefully lock on the class for your own code, but you can't really guarantee libraries will work with that (e.g. they may get into deadlocks).
this crash is kind of interesting because i don't see any 'unsafe' code* in this getBounds() implementation so it shouldn't crash the JDK. like the code is broken in how it works but it shouldn't be possible to muck around with threading a race and then call clone and crash the JDK.
* the code is very abstract and touches a lot of classes so there could be some unsafe code hiding there i can't see.
It seems to me that the main cause of the crash is the fact that the author used a special [classloader](https://github.com/vmlens/jcstress-examples/blob/master/src/...) which always reloads the class and is not thread-safe intentionally (classloaders should be Thread-safe). I did not investigate in depth but that seems to be the main problem to me.
Indeed, the memory model on ARM does not provide the same guarantees as x86. Given the behaviour described, it seems likely one could weaponise this kind of race condition, on ARM.
[1] See https://bartoszmilewski.com/2008/11/05/who-ordered-memory-fe... for a readable discussion about what x86/x64 does and doesn't do.