RFR: 8339561: The test/jdk/java/awt/Paint/ListRepaint.java may fail after JDK-8327401 [v5]
Alexey Ivanov
aivanov at openjdk.org
Tue Mar 4 21:21:54 UTC 2025
On Tue, 4 Mar 2025 16:53:36 GMT, Sergey Bylokhov <serb at 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20861#discussion_r1980247699
More information about the client-libs-dev
mailing list