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

[Sergey Bylokhov]

On Tue, 25 May 2021 19:01:00 GMT, Alexander Zvegintsev <azvegint at 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