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




Reply via email to