<Sound Dev> RFR: 8266421: Deadlock in Sound System [v2]

[Sergey Bylokhov]

On Wed, 26 May 2021 00:25:10 GMT, Sergey Bylokhov <serb at 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