<Sound Dev> [12] Review Request: 8207150 Clip.isRunning() may return true after Clip.stop() was called

[Phil Race]

I don't know what your notation for the race condition means. Please expand and explain in more detail how this happens and add it to the bug report as well as here.

-Phil.

> On Aug 3, 2018, at 2:14 PM, Sergey Bylokhov <Sergey.Bylokhov at oracle.com> wrote:
> 
> Hello, Audio Guru.
> 
> Please review the fix for jdk12.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8207150
> Webrev: http://cr.openjdk.java.net/~serb/8207150/webrev.00
> 
> This bug was found by the test added in JDK-8202264, this test checks this sequence:
>  clip.loop();
>  clip.stop();
>  delay(20 sec);
>  //check that Audio thread was stopped.
> 
> The test fails because the thread was not stopped, the reason is that "autoclose" mechanics cannot stop the thread of the clip which is in "running" state. So the test can be simplified to:
>  clip.loop();
>  clip.stop();
>  if(clip.isRunning()) BOOM();
> 
> The root cause description:
>  The problem exists in our DirectAudioDevice implementation. Before we play the audio we have this check in write():
> if (!isActive() && doIO) {
>    setActive(true);
>    setStarted(true);
> }
> 
> And to stop the clip we have this:
> synchronized(lock) {
>    // need to set doIO to false before notifying the
>    // read/write thread, that's why isStartedRunning()
>    // cannot be used
>    doIO = false;
>    lock.notifyAll();
> }
> setActive(false);
> setStarted(false);
> 
> 
> Note that it is possible to get the next race.
> 1 write() -> if(doIO) is true
> 2 stop() -> doIO=false + setActive(false) + setStarted(false)
> 3 write() -> setActive(true) + setStarted(true)
> 
> After the fix the access to doIO and setActive/setStarted is done under the lock.
> 
> -- 
> Best regards, Sergey.