CVS: winex/dlls/winmm/winealsa audio.c,1.46,1.47

Subject: winex/dlls/winmm/winealsa audio.c,1.46,1.47Update of /var/lib/cvsd/cvsroot/winex/dlls/winmm/winealsa
In directory agravaine:/tmp/cvs-serv24792/dlls/winmm/winealsa

Modified Files:
	audio.c 
Log Message:
Previously, if the asynchronous PCM callback was called while the critical
section was already being held by the same thread, bad things could happen,
including loss of audio or even deadlocks. I've attempted to prevent this
from happening by managing the critical section locking more carefully.


Index: audio.c
===================================================================
RCS file: /var/lib/cvsd/cvsroot/winex/dlls/winmm/winealsa/audio.c,v
retrieving revision 1.46
retrieving revision 1.47
diff -u -d -r1.46 -r1.47
--- audio.c	30 Mar 2007 21:32:40 -0000	1.46
+++ audio.c	30 Mar 2007 21:34:31 -0000	1.47
@@ -2036,9 +2036,30 @@
     snd_pcm_uframes_t         mmap_app_position;
     const snd_pcm_channel_area_t * mmap_areas_last;
     snd_pcm_uframes_t         mmap_async_commit;
+    LONG                      mmap_copy_pending;
     BOOL playing;
 };
 
+#define INIT_COPY(dsdb) \
+  do { \
+    EnterCriticalSection(&(dsdb)->mmap_crst); \
+    DSDB_MMAPCopy(dsdb); \
+    LeaveCriticalSection(&(dsdb)->mmap_crst); \
+  } while (0)
+#define ENTER_LOCK(dsdb) \
+  do { \
+    EnterCriticalSection(&(dsdb)->mmap_crst); \
+  } while (0)
+#define LEAVE_LOCK(dsdb) \
+  do { \
+    LONG recursion = (dsdb)->mmap_crst.RecursionCount; \
+    LeaveCriticalSection(&(dsdb)->mmap_crst); \
+    if (recursion <= 1) { \
+      while (InterlockedExchange(&(dsdb)->mmap_copy_pending, 0)) \
+        INIT_COPY(dsdb); \
+    } \
+  } while (0)
+
 static inline snd_pcm_t* DSDB_get_handle(IDsDriverBufferImpl* pdbi)
 {
     return WOutDev[pdbi->drv->wDevID].p_handle;
@@ -2166,7 +2187,19 @@
     /* snd_pcm_t *               handle = snd_async_handler_get_pcm(ahandler); */
     IDsDriverBufferImpl*      pdbi = snd_async_handler_get_callback_private(ahandler);
     TRACE("callback called\n");
+    /* try to acquire critical section (must not block) */
+    if (!TryEnterCriticalSection(&pdbi->mmap_crst)) {
+        InterlockedIncrement(&pdbi->mmap_copy_pending);
+        return;
+    }
+    /* don't accept a recursive lock, it means we interrupted something */
+    if (pdbi->mmap_crst.RecursionCount > 1) {
+        InterlockedIncrement(&pdbi->mmap_copy_pending);
+        LeaveCriticalSection(&pdbi->mmap_crst);
+        return;
+    }
     DSDB_MMAPCopy(pdbi);
+    LeaveCriticalSection(&pdbi->mmap_crst);
 }
 
 static int DSDB_CreateMMAP(IDsDriverBufferImpl* pdbi)
@@ -2198,6 +2231,7 @@
     pdbi->mmap_buflen_frames = frames;
     pdbi->mmap_buflen_bytes = snd_pcm_frames_to_bytes( handle, frames );
     pdbi->mmap_async_commit = 0;
+    pdbi->mmap_copy_pending = 0;
 #if 0
     pdbi->mmap_buffer = HeapAlloc(GetProcessHeap(),0,pdbi->mmap_buflen_bytes);
     if (!pdbi->mmap_buffer)
@@ -2330,7 +2364,7 @@
 
     DSDB_CheckXRUN(This);
 
-    EnterCriticalSection(&This->mmap_crst);
+    ENTER_LOCK(This);
     handle = DSDB_get_handle(This);
 
     /* get playing position */
@@ -2443,7 +2477,7 @@
 	    err = snd_pcm_mmap_begin(handle, &This->mmap_areas_last, &offset, &frames);
 	    if (err < 0) {
 		ERR("mmap_begin failed, reason: %s\n", snd_strerror(err));
-		LeaveCriticalSection(&This->mmap_crst);
+		LEAVE_LOCK(This);
 		return E_FAIL;
 	    }
 
@@ -2500,7 +2534,7 @@
 	if (pdwLen2) *pdwLen2 = 0;
     }
 
-    LeaveCriticalSection(&This->mmap_crst);
+    LEAVE_LOCK(This);
 
     TRACE("=>(%p,%ld,%p,%ld)\n", *ppvAudio1, *pdwLen1,
 				 ppvAudio2 ? *ppvAudio2 : NULL,
@@ -2526,7 +2560,7 @@
 
     DSDB_CheckXRUN (This);
 
-    EnterCriticalSection(&This->mmap_crst);
+    ENTER_LOCK(This);
     handle = DSDB_get_handle(This);
 
     dwWritePos = ((LPBYTE)pvAudio1) - ((LPBYTE)This->mmap_areas_last[0].addr);
@@ -2626,7 +2660,7 @@
 	    move = snd_pcm_mmap_commit(handle, offset, left);
 	    if (move < 0) {
 		ERR("mmap_commit failed, reason: %s\n", snd_strerror(move));
-		LeaveCriticalSection(&This->mmap_crst);
+		LEAVE_LOCK(This);
 		return E_FAIL;
 	    }
 	    if (!move) {
@@ -2641,7 +2675,7 @@
     }
     TRACE("app_pos=%ld\n", This->mmap_app_position);
 
-    LeaveCriticalSection(&This->mmap_crst);
+    LEAVE_LOCK(This);
 
     state = snd_pcm_state(handle);
     if ( state == SND_PCM_STATE_PREPARED && This->playing )
@@ -2807,7 +2841,7 @@
     /** we need to track down buffer underruns */
     DSDB_CheckXRUN(This);
 
-    EnterCriticalSection(&This->mmap_crst);
+    ENTER_LOCK(This);
     handle = DSDB_get_handle(This);
 
     if (!handle) {
@@ -2816,7 +2850,7 @@
 	    *lpdwPlay = 0;
 	if (lpdwWrite)
 	    *lpdwWrite = 0;
-	LeaveCriticalSection(&This->mmap_crst);
+	LEAVE_LOCK(This);
 	return DS_OK;
     }
 
@@ -2855,7 +2889,7 @@
 	    *lpdwWrite = snd_pcm_frames_to_bytes(handle, hw_ptr) % This->mmap_buflen_bytes;
     }
 #endif
-    LeaveCriticalSection(&This->mmap_crst);
+    LEAVE_LOCK(This);
 
     TRACE("hw_ptr=%ld, playpos=%ld, writepos=%ld\n", hw_ptr, lpdwPlay?*lpdwPlay:-1, lpdwWrite?*lpdwWrite:-1);
     return DS_OK;
@@ -2879,7 +2913,7 @@
     if ( state == SND_PCM_STATE_PREPARED )
     {
 	if (This->mmap_async_commit)
-	    DSDB_MMAPCopy(This);
+	    INIT_COPY(This);
 	err = snd_pcm_start(handle);
     }