<Sound Dev> [9] Review Request: 8156169 Some sound tests rarely hangs because of incorrect synchronization

[Alex Menkov]

On 18.05.2016 19:53, Sergey Bylokhov wrote:
> On 17.05.16 19:19, Alex Menkov wrote:
>> Most of the fix looks good to me, but I'm not sure change in
>> Sequence.deleteTrack is correct.
>> As far as I understand synchronized(track) are to make safe
>> getTickLength method (otherwise we can get
>> ArrayIndexOutOfBoundsException there)
>
> This is correct that the synchronization is necessary, but
> tracks.removeElement() is synchronized on "this" which is "tracks", so
> currently we synchronized on tracks twice, outside and inside of the
> method.

Oh, got it.

Approved.

--alex

>
>> On 17.05.2016 17:30, Sergey Bylokhov wrote:
>>> Hello, Audio Guru.
>>>
>>> Please review the fix for jdk9.
>>> While working on some bugs reported by the mach5 project, I found that
>>> some of our tests are quite unstable, and the reason was in this(or
>>> similar) pattern:
>>> ....
>>> clip.start();
>>> while(clip.isRunning());
>>> ....
>>> The status of the clip is run or not is updated on a different thread,
>>> but our clip implementation lacks of synchronization of getters and the
>>> hotspot inline such methods to while(true). There are some other flags
>>> which have the similar issue, I tried to fix all of them in the proposed
>>> version of the fix.
>>>
>>> Also I propose the small cleanup of Sequence.java
>>>  - It is not necessary to sync tracks.removeElement() on tracks, because
>>> this is synchronized method.
>>>  - tracks.toArray(new Track[tracks.size()]) should be synchronized on
>>> tracks, because the problem can occurs between tracks.size() and
>>> tracks.toArray(). But I decided to pass the empty array, so all the work
>>> will be done in toArray() which is synchronized method.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8156169
>>> Webrev can be found at:
>>> http://cr.openjdk.java.net/~serb/8156169/webrev.01
>>>
>>>
>
>