view patches/icedtea-directaudio-close-trick.patch @ 857:3c358d6fd84e

Fix potential ALSA locks in DirectAudioDevice because of races on doIO var. 2008-05-08 Mark Wielaard <mark@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.
author Mark Wielaard <mark@klomp.org>
date Fri, 09 May 2008 02:30:42 +0200
parents bb8988602259
children
line wrap: on
line source

--- /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-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
 
+        // 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 +501,47 @@
             // align buffer to full frames
             bufferSize = ((int) bufferSize / format.getFrameSize()) * format.getFrameSize();
 
-            id = nOpen(mixerIndex, deviceID, isSource,
-                       encoding,
-                       hardwareFormat.getSampleRate(),
-                       hardwareFormat.getSampleSizeInBits(),
-                       hardwareFormat.getFrameSize(),
-                       hardwareFormat.getChannels(),
-                       hardwareFormat.getEncoding().equals(AudioFormat.Encoding.PCM_SIGNED),
-                       hardwareFormat.isBigEndian(),
-                       bufferSize);
+	    synchronized(lockLast) {
+	      id = nOpen(mixerIndex, deviceID, isSource,
+			 encoding,
+			 hardwareFormat.getSampleRate(),
+			 hardwareFormat.getSampleSizeInBits(),
+			 hardwareFormat.getFrameSize(),
+			 hardwareFormat.getChannels(),
+			 hardwareFormat.getEncoding().equals(AudioFormat.Encoding.PCM_SIGNED),
+			 hardwareFormat.isBigEndian(),
+			 bufferSize);
+	      
+	      if (id == 0) {
+		// Bah... Dirty trick. The most likely cause is an application
+		// already having a line open for this particular hardware
+		// format and forgetting about it. If so, silently close that
+		// implementation and try again. Unfortuantely we can only
+		// open one line per hardware format currently.
+		if (lastOpened != null
+		    && hardwareFormat.matches(lastOpened.hardwareFormat)) {
+		  lastOpened.implClose();
+		  lastOpened = null;
+		  
+		  id = nOpen(mixerIndex, deviceID, isSource,
+			     encoding,
+			     hardwareFormat.getSampleRate(),
+			     hardwareFormat.getSampleSizeInBits(),
+			     hardwareFormat.getFrameSize(),
+			     hardwareFormat.getChannels(),
+			     hardwareFormat.getEncoding().equals(AudioFormat.Encoding.PCM_SIGNED),
+			     hardwareFormat.isBigEndian(),
+			     bufferSize);
+		}
+		
+		if (id == 0) {
+		    // TODO: nicer error messages...
+		    throw new LineUnavailableException("line with format "+format+" not supported.");
+                }
+	      }
+	      lastOpened = this;
+	    }
 
-            if (id == 0) {
-                // TODO: nicer error messages...
-                throw new LineUnavailableException("line with format "+format+" not supported.");
-            }
             this.bufferSize = nGetBufferSize(id, isSource);
             if (this.bufferSize < 1) {
                 // this is an error!
@@ -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);
+            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: "