On Mon, 16 Sep 2024 11:38:52 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

>> test/jdk/java/awt/List/KeyEventsTest/KeyEventsTest.java line 53:
>> 
>>> 51: 
>>> 52: public class KeyEventsTest {
>>> 53:     private volatile TestState currentState;
>> 
>> I'd rather not declare it a `volatile`. It could give a false impression of 
>> being thread-safe but it is not. The `volatile` modifier has a meaning only 
>> when it's written and read subsequently. If the reference doesn't change, it 
>> has no effect on the visibility of the internal object state.
>> 
>> The value is assigned to `currentState` while holding a lock `LOCK`.
>> 
>> At the same time, `currentState.setAction(true)` is called without any 
>> synchronisation and adding `volatile` won't make the change of the state 
>> thread-safe.
>
> Submitted [JDK-8340196](https://bugs.openjdk.org/browse/JDK-8340196): 
> _j.a/List/KeyEventsTest/KeyEventsTest: TestState currentState is not 
> thread-safe_.

I think as of now it should be good enough, why do you think volatile will not 
help here?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20861#discussion_r1979853674

Reply via email to