On Tue, 4 Mar 2025 21:19:09 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> I think as of now it should be good enough, why do you think volatile will >> not help here? > >> 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? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20861#discussion_r2047865655
