On Tue, 25 May 2021 19:01:00 GMT, Alexander Zvegintsev <azveg...@openjdk.org> 
wrote:

>> It could make sense only if the "sendEvents" flag will be used under some 
>> kind of synchronization as well, otherwise, it is possible to get 
>> sendEvents=true twice, it was not changed after this fix:
>> synchronized (this) {
>>     if (this.started != started) {
>>         this.started = started;
>>         sendEvents = true;
>>       }
>> }
>> <<< here the other thread could set "started" to the opposite state and we 
>> post events twice.
>> if (sendEvents) {
>> .....
>> }
>
> The case with setting opposite state you are describing is a different one. 
> We will post two different events like start and stop.
> 
> I am talking about calling `setStarted()` with same parameter from different 
> threads.
> 
> For example we have two threads T1 and T2 calling `setStarted(true)`.
> Before the fix only one of thread will set `sendEvents` to `true` due to 
> `synchronized` block.
> 
> After the fix following is possible:
> 
> // this.started  == false, started == true
> if (this.started != started) { //T1 passed this check
>     // <<<<< at this moment T2 checks the statement above
>     this.started = started; // T1 and T2 assigns same new value to 
> this.started
>     sendEvents = true; // both threads sets sendEvents to true.
> }
> // sending two start events.

My example was about the thread-safety of the method, after synchronized block, 
the other thread may change the state, then post event, and then this thread 
will post event. So we will post the event twice in the wrong order. So this 
method as-is is not thread-safe, we should not call it in parallel. I'll post 
data for its usage here.

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

PR: https://git.openjdk.java.net/jdk/pull/4141

Reply via email to