On Wed, 26 May 2021 00:25:10 GMT, Sergey Bylokhov <[email protected]> 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
