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