<Sound Dev> RFR: 8266421: Deadlock in Sound System

[Alexander Zvegintsev]

On Tue, 25 May 2021 18:06:21 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> src/java.desktop/share/classes/com/sun/media/sound/AbstractDataLine.java line 322:
>> 
>>> 320: 
>>> 321:         if (this.started != started) {
>>> 322:             this.started = started;
>> 
>>> I doubt that this type of synchronization may help something - the fields are volatile and we use sendEvents flag after synchronization block, so I removed it as well. Any thoughts?
>> 
>> These fields are volatile, but the comparison and assignment is not atomic.
>> So I believe it is possible the case when we will have `sendEvents == true` in two threads, hence we will send a duplicate event.
>> 
>> So we probably might want to use `AtomicBoolean` if you want to get rid of `synchronized`.
>
> 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.

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

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