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

[Sergey Bylokhov]

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.

> 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
>>
>>


-- 
Best regards, Sergey.