On Mon, 22 Jan 2024 21:06:01 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> Again, thread-safety isn't an issue here. Even if the OS were to invoke the >> IM methods on different threads (which is a terrifying thought) these >> adapters are created on a per-call basis so each thread would get its own >> instance. This also means those extra eight bytes are transitory. >> >> If there's an established JavaFX pattern of using AtomicReference instead of >> an array I will gladly make the changes. But this does require manual >> testing (I still haven't figured out how to automate this) so I'm reluctant >> to do this based on personal style preferences. > > Here is more info [0]. > > We **are** dealing with multiple threads. > > I've seen weird cases where modifying memory from different threads without > explicit synchronization resulted in subtle bugs - we don't know all the > details of cache coherence [1] implementation in multi-core systems, so I > would strongly suggest to use AtomicReference. > > [0] > https://stackoverflow.com/questions/70381029/java-atomincinteger-vs-one-element-array-for-streams > > [1] https://en.wikipedia.org/wiki/Cache_coherence Sorry, I was still thinking about the original thread-safety concern which involved simultaneous access to the fields in this adapter object. Not the issue here. I agree that explicit synchronization needs to happen but I thought that was what `runAndWait` was for. @kevinrushforth also states that `runAndWait` should be sufficient. If you think there's a cache coherence problem that's much, much bigger than this PR since this pattern is used throughout the code base. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1337#discussion_r1462483419
