[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [nas] nasd outputs only part of a sample, client stalls --exceptunder strace



On Sun, 2 Sep 2007, Erik Auerswald wrote:

Hi,

On Mon, Jul 23, 2007 at 08:54:00AM -0400, mmurray wrote:

To the best of my reasoning, the SIGALRM is being lost in a race somewhere.

This seems to be correct -- sending a SIGALRM to nasd whenever the playback hangs results in continued playback.


Ok. Attached is a patch that reworks the signal handling in the voxware server. I do not have a spare machine to test the newer kernels on, so I have no idea if this will affect the problem you guys are seeing.

  I do believe it (the patch) to be the correct way of doing it
  though.  In addition, DIA dispatch is now able to properly block the
  appropriate signal when it needs to, which it couldn't before
  (requiring the racy block/unblocks in intervalProc).  Make sure dia/
  gets rebuilt as well after applying this patch. Or just do a 'make
  World' to be sure :)

  Let me know if this corrects anything (and especially if it breaks
  anything).  I'll wait before committing until I get some kind of
  feedback.


--
Jon Trulson
mailto:jon@xxxxxxxxxxx #include <std/disclaimer.h>
"No Kill I" -Horta
Index: server/dda/voxware/auvoxware.c
===================================================================
--- server/dda/voxware/auvoxware.c	(revision 263)
+++ server/dda/voxware/auvoxware.c	(working copy)
@@ -316,6 +316,75 @@
 static void setPhysicalInputGainAndLineMode(AuFixedPoint gain,
                                             AuUint8 lineMode);
 
+
+/* internal funtions for enabling/disabling intervalProc using sigaction
+   semantics */
+
+static void intervalProc(int sig);
+
+/* use this in disableIntervalProc() instead of SIG_IGN for testing */
+#if 0
+static void ignoreProc(int sig)
+{
+  osLogMsg("SIGNAL IGNORE: ENTRY\n");
+  return;
+}
+#endif
+
+static void enableIntervalProc(void)
+{
+    struct sigaction action;
+
+    action.sa_handler = (void (*)(int))intervalProc;
+    action.sa_flags = 0;
+    sigemptyset(&action.sa_mask);
+    sigaddset(&action.sa_mask, SIGALRM);
+
+    if (sigaction(SIGALRM, &action, NULL) == -1)
+        {
+          osLogMsg("enableIntervalProc: sigaction failed: %s\n", 
+                   strerror(errno));
+        }
+  
+    return;
+}
+
+static void disableIntervalProc(void)
+{
+    struct sigaction action;
+
+    action.sa_handler = (void (*)(int))SIG_IGN;
+    action.sa_flags = 0;
+
+    if (sigaction(SIGALRM, &action, NULL) == -1)
+        {
+          osLogMsg("disableIntervalProc: sigaction failed: %s\n", 
+                   strerror(errno));
+        }
+  
+    return;
+}
+
+AuBlock _AuBlockAudio(void)
+{                                                          
+  sigset_t set;
+  sigemptyset(&set); 
+  sigaddset(&set, SIGALRM);
+  sigprocmask(SIG_BLOCK, &set, NULL);
+  return 0;
+}
+
+void _AuUnBlockAudio(AuBlock _x)
+{
+  sigset_t set;
+  sigemptyset(&set);
+  sigaddset(&set, SIGALRM);
+  sigprocmask(SIG_UNBLOCK, &set, NULL);
+  return;
+}
+
+
+/* ### SCO ### */
 #ifdef sco
 
 AuBlock
@@ -1034,9 +1103,7 @@
     }
 
     setTimer(0);
-#ifndef sco
-    signal(SIGALRM, SIG_IGN);
-#endif /* sco */
+    disableIntervalProc(); 
 
 #if defined(AUDIO_DRAIN)
     if (sndStatOut.isPCSpeaker)
@@ -1064,46 +1131,18 @@
     }
 }
 
-
 static void
 intervalProc(int sig)
 {
     extern void AuProcessData();
 
-#ifndef sco
-    signal(SIGALRM, SIG_IGN);
+#if !defined(sco)
+    disableIntervalProc();   
 
-#if defined(linux) || defined(__CYGWIN__)
-    /* this looks funny and stupid, but BSD not only provide
-     * reliable signals but also block this signal while executing
-     * this handler
-     * So the timer may expires before the handler is ready
-     * and it will be recalled immediately again preventing us
-     * executing normal stuff
-     * this happens if we get blocked by the write, i.e.
-     * It will happen as our timer is a little bit to fast
-     * so we should really ignore signals
-     */
-    {
-        sigset_t set;
-        sigemptyset(&set);
-        sigaddset(&set, SIGALRM);
-        sigprocmask(SIG_BLOCK, &set, NULL);
-    }
-#endif /* linux or cygwin */
-    if (processFlowEnabled) {
+    if (processFlowEnabled)
         AuProcessData();
-#if defined(linux) || defined(__CYGWIN__)
-        /* unblock the signal after processing the data */
-        {
-            sigset_t set;
-            sigemptyset(&set);
-            sigaddset(&set, SIGALRM);
-            sigprocmask(SIG_UNBLOCK, &set, NULL);
-        }
-#endif /* linux or cygwin */
-        signal(SIGALRM, intervalProc);
-    }
+
+    enableIntervalProc();   
 #else
     if (!scoAudioBlocked && processFlowEnabled)
         AuProcessData();
@@ -1267,38 +1306,29 @@
             setMixerDefaults();
         }
     }
-#ifdef sco
+
+#if defined(sco)
     if (!processFlowEnabled) {
-#endif /* sco */
-
         processFlowEnabled = AuTrue;
-
-#ifndef sco
-        signal(SIGALRM, intervalProc);
-        if (NasConfig.DoDebug > 1) {
-            osLogMsg("enableProcessFlow() - set SIGALRM handler to intervalProc\n");
-        }
-#else
         setTimer(sndStatOut.curSampleRate);
+    }
+#else  /* sco */
+    processFlowEnabled = AuTrue;
 #endif /* sco */
 
-#ifdef sco
-    }
-#endif /* sco */
 }
 
 static void
 disableProcessFlow(void)
 {
-
 #ifndef sco
     int rate;
-    signal(SIGALRM, SIG_IGN);
 #endif /* sco */
 
     if (NasConfig.DoDebug) {
         osLogMsg("disableProcessFlow() - starting\n");
     }
+
 #ifdef sco
     if (processFlowEnabled) {
 #endif /* sco */
@@ -1958,7 +1988,9 @@
         return AuFalse;
     }
 #else
-    signal(SIGALRM, intervalProc);
+
+     enableIntervalProc(); 
+
 #endif /* sco */
 
     setTimer(0);
Index: server/dda/voxware/auvoxware.h
===================================================================
--- server/dda/voxware/auvoxware.h	(revision 263)
+++ server/dda/voxware/auvoxware.h	(working copy)
@@ -75,9 +75,16 @@
 #include <signal.h>
 
 typedef int AuBlock;
+
 #if defined(linux)  || defined(__CYGWIN__)
-#define        AuBlockAudio()          0
-#define        AuUnBlockAudio(_x)
+
+/* use functions defined in auvoxware.c */
+AuBlock  _AuBlockAudio(void);
+void _AuUnblockAudio(AuBlock _x);
+
+#define AuBlockAudio()     _AuBlockAudio()
+#define AuUnBlockAudio(_x) _AuUnBlockAudio(_x)
+
 #else /* defined(linux)  */
 #ifndef sco
 #if defined(SVR4) || defined(SYSV)