On Wed, 16 Apr 2025 22:36:50 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>>> I think as of now it should be good enough >> >> That's the problem, the presence of the `volatile` modifier creates a false >> sense of thread-safety, this is why I'd rather not add it. >> >>> why do you think volatile will not help here? >> >> The `volatile` modifier guarantees that a thread which reads from the >> `volatile` field will see everything that occurred before a (new) reference >> was written to the `volatile` field. Yet there are no guarantees another >> thread will see any modifications to fields of the object the reference to >> which is stored in the `volatile` field. >> >> And this test does exactly this: it writes a reference into the >> `currentState` field and then it modifies the fields of the object stored in >> `currentState`. There are no guarantees that `currentState.setAction(true)` >> will result in `currentState.getAction()` returning `true`. >> >> If you also declare `TestState.action` as `volatile` or use `AtomicBoolean`, >> it could be enough to safely access the `currentState` field from two >> threads. Other fields are final, so they can't be modified, and therefore >> they don't change after another thread sees the reference to `TestState` >> object. > > @aivanov-jdk Do you have any other comments? Sorry, for the delay, I've been busy recently. I'll take another look as soon as possible. Thank you for your understanding. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20861#discussion_r2048598821
