On Wed, 26 May 2021 00:25:10 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:

>> 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.

1. The changes in the AbstractDataLine.setActive() are fine, we do not post 
events there.
2. The changes in the AbstractDataLine.setStarted() are used and syncronised:
    used in AbstractDataLine:343- the setEOM() dead unused non-public method
    used in AbstractDataLine:205 - syncronised on synchronized(mixer); Also 
dead code, the setStarted() is never executed, since the line is stopped 
already a few lines above in the implStop();
    used in DirectAudioDevice:533 - synchronized on synchronized(lock) and 
synchronized(mixer)
    used in DirectAudioDevice:560 -  synchronized on synchronized(lock) and 
synchronized(mixer)
    used in DirectAudioDevice:707 -  synchronized on synchronized(lock)
    used in DirectAudioDevice:932 -  synchronized on synchronized(lock)
3. The changes in the AbstractLine.setOpen() are used and synchronised:
    used in AbstractDataLine:118 - syncronised on synchronized(mixer);
    used in AbstractDataLine:374 - syncronised on synchronized(mixer);
    used in AbstractMixer:288 - syncronised on this;
    used in AbstractMixer:377 - syncronised on this;
    used in PortMixer:277 - syncronised on synchronized(mixer);
    used in PortMixer:293 - syncronised on synchronized(mixer);

So every object uses its own high-level synchronization when it calls the 
methods in question.

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

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

Reply via email to