changeset in /hg/icedtea6: Fix potential ALSA locks in DirectAud...
Mark Wielaard
mark at klomp.org
Thu May 8 17:40:12 PDT 2008
changeset 3c358d6fd84e in /hg/icedtea6
details: http://icedtea.classpath.org/hg/icedtea6?cmd=changeset;node=3c358d6fd84e
description:
Fix potential ALSA locks in DirectAudioDevice because of races on doIO var.
2008-05-08 Mark Wielaard <mark at klomp.org>
* patches/icedtea-directaudio-close-trick.patch: Use new static
lockLast for nOpen/nClose guarding. Make lockNative non-static
again. Do all checks before native calls of doIO inside
lockNative guard. Set doIO to false after nClose before dropping
lockNative guard.
diffstat:
2 files changed, 129 insertions(+), 12 deletions(-)
ChangeLog | 8 +
patches/icedtea-directaudio-close-trick.patch | 133 ++++++++++++++++++++++---
diffs (179 lines):
diff -r d86e9eb1fa7d -r 3c358d6fd84e ChangeLog
--- a/ChangeLog Thu May 08 11:04:53 2008 -0400
+++ b/ChangeLog Fri May 09 02:30:42 2008 +0200
@@ -1,3 +1,11 @@ 2008-05-08 Lillian Angel <langel at redha
+2008-05-08 Mark Wielaard <mark at klomp.org>
+
+ * patches/icedtea-directaudio-close-trick.patch: Use new static
+ lockLast for nOpen/nClose guarding. Make lockNative non-static
+ again. Do all checks before native calls of doIO inside
+ lockNative guard. Set doIO to false after nClose before dropping
+ lockNative guard.
+
2008-05-08 Lillian Angel <langel at redhat.com>
Fixes Bug #150
diff -r d86e9eb1fa7d -r 3c358d6fd84e patches/icedtea-directaudio-close-trick.patch
--- a/patches/icedtea-directaudio-close-trick.patch Thu May 08 11:04:53 2008 -0400
+++ b/patches/icedtea-directaudio-close-trick.patch Fri May 09 02:30:42 2008 +0200
@@ -1,18 +1,19 @@
--- /home/mark/src/openjdk/jdk/src/share/classes/com/sun/media/sound/DirectAudioDevice.java 2008-04-13 01:05:30.000000000 +0200
-+++ openjdk/jdk/src/share/classes/com/sun/media/sound/DirectAudioDevice.java 2008-05-04 18:42:39.000000000 +0200
-@@ -394,7 +394,10 @@
++++ openjdk/jdk/src/share/classes/com/sun/media/sound/DirectAudioDevice.java 2008-05-09 02:18:21.000000000 +0200
+@@ -394,7 +394,12 @@
private float leftGain, rightGain;
protected volatile boolean noService = false; // do not run the nService method
-- protected Object lockNative = new Object();
-+ // Guards all native calls and the lastOpened static variable.
-+ protected static Object lockNative = new Object();
++ // Guards all native calls.
+ protected Object lockNative = new Object();
++ // Guards the lastOpened static variable in implOpen and implClose.
++ protected static Object lockLast = new Object();
+ // Keeps track of last opened line, see implOpen "trick".
+ protected static DirectDL lastOpened;
// CONSTRUCTOR
protected DirectDL(DataLine.Info info,
-@@ -496,20 +499,47 @@
+@@ -496,20 +501,47 @@
// align buffer to full frames
bufferSize = ((int) bufferSize / format.getFrameSize()) * format.getFrameSize();
@@ -25,7 +26,7 @@
- hardwareFormat.getEncoding().equals(AudioFormat.Encoding.PCM_SIGNED),
- hardwareFormat.isBigEndian(),
- bufferSize);
-+ synchronized(lockNative) {
++ synchronized(lockLast) {
+ id = nOpen(mixerIndex, deviceID, isSource,
+ encoding,
+ hardwareFormat.getSampleRate(),
@@ -73,12 +74,120 @@
this.bufferSize = nGetBufferSize(id, isSource);
if (this.bufferSize < 1) {
// this is an error!
-@@ -616,6 +646,8 @@
+@@ -580,12 +612,12 @@
+ }
+ synchronized (lockNative) {
+ nStop(id, isSource);
+- }
+
+- // need to set doIO to false before notifying the
+- // read/write thread, that's why isStartedRunning()
+- // cannot be used
+- doIO = false;
++ // need to set doIO to false before notifying the
++ // read/write thread, that's why isStartedRunning()
++ // cannot be used
++ doIO = false;
++ }
+ // wake up any waiting threads
+ synchronized(lock) {
+ lock.notifyAll();
+@@ -614,8 +646,12 @@
+ doIO = false;
+ long oldID = id;
id = 0;
- synchronized (lockNative) {
- nClose(oldID, isSource);
-+ if (lastOpened == this)
-+ lastOpened = null;
+- synchronized (lockNative) {
+- nClose(oldID, isSource);
++ synchronized (lockLast) {
++ synchronized (lockNative) {
++ nClose(oldID, isSource);
++ if (lastOpened == this)
++ lastOpened = null;
++ }
}
bytePosition = 0;
softwareConversionSize = 0;
+@@ -630,7 +666,8 @@
+ }
+ int a = 0;
+ synchronized (lockNative) {
+- a = nAvailable(id, isSource);
++ if (doIO)
++ a = nAvailable(id, isSource);
+ }
+ return a;
+ }
+@@ -644,9 +681,9 @@
+ int counter = 0;
+ long startPos = getLongFramePosition();
+ boolean posChanged = false;
+- while (!drained && doIO) {
++ while (!drained) {
+ synchronized (lockNative) {
+- if ((id == 0) || !nIsStillDraining(id, isSource))
++ if ((id == 0) || (!doIO) || !nIsStillDraining(id, isSource))
+ break;
+ }
+ // check every now and then for a new position
+@@ -686,7 +723,7 @@
+ lock.notifyAll();
+ }
+ synchronized (lockNative) {
+- if (id != 0) {
++ if (id != 0 && doIO) {
+ // then flush native buffers
+ nFlush(id, isSource);
+ }
+@@ -697,9 +734,10 @@
+
+ // replacement for getFramePosition (see AbstractDataLine)
+ public long getLongFramePosition() {
+- long pos;
++ long pos = 0;
+ synchronized (lockNative) {
+- pos = nGetBytePosition(id, isSource, bytePosition);
++ if (doIO)
++ pos = nGetBytePosition(id, isSource, bytePosition);
+ }
+ // hack because ALSA sometimes reports wrong framepos
+ if (pos < 0) {
+@@ -745,11 +783,12 @@
+ }
+ int written = 0;
+ while (!flushing) {
+- int thisWritten;
++ int thisWritten = 0;
+ synchronized (lockNative) {
+- thisWritten = nWrite(id, b, off, len,
+- softwareConversionSize,
+- leftGain, rightGain);
++ if (doIO)
++ thisWritten = nWrite(id, b, off, len,
++ softwareConversionSize,
++ leftGain, rightGain);
+ if (thisWritten < 0) {
+ // error in native layer
+ break;
+@@ -972,9 +1011,10 @@
+ }
+ int read = 0;
+ while (doIO && !flushing) {
+- int thisRead;
++ int thisRead = 0;
+ synchronized (lockNative) {
+- thisRead = nRead(id, b, off, len, softwareConversionSize);
++ if (doIO)
++ thisRead = nRead(id, b, off, len, softwareConversionSize);
+ if (thisRead < 0) {
+ // error in native layer
+ break;
+@@ -1209,7 +1249,8 @@
+ // set new native position (if necessary)
+ // this must come after the flush!
+ synchronized (lockNative) {
+- nSetBytePosition(id, isSource, frames * frameSize);
++ if (doIO)
++ nSetBytePosition(id, isSource, frames * frameSize);
+ }
+
+ if (Printer.debug) Printer.debug(" DirectClip.setFramePosition: "
More information about the distro-pkg-dev
mailing list