>Number:         5446
>Category:       pkg
>Synopsis:       xmame sound code doesn't protect critical sections
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    gnats-admin (GNATS administrator)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue May 12 06:35:00 1998
>Last-Modified:
>Originator:     Dave Sainty
>Organization:
DTSP Ltd.
>Release:        12/5/98
>Environment:
System: NetBSD tequila.dave.dtsp.co.nz 1.3E NetBSD 1.3E (TEQUILA) #2: Tue May 12 16:39:18 NZST 1998 dave@tequila.dave.dtsp.co.nz:/vol/tequila/userC/NetBSD-current/src/sys/arch/i386/compile/TEQUILA i386


>Description:
	xmame sound code doesn't protect critical sections.  The NetBSD sound
	support currently outputs sound on SIGALRM, and eventually the signal
	will occur within a critical section.  It takes on average a few
	minutes on my machine.

>How-To-Repeat:
	Level your sights on an alien, say "I'm taking you doooown!", and get
	annoyed when the application spontaneously exits.

>Fix:
Add this patch to the package.  This stops the immediate problem, but there is
still a free() within the signal handler that definitely shouldn't be there:

--- sound.c.orig	Wed May 13 00:13:36 1998
+++ sound.c	Wed May 13 00:30:11 1998
@@ -12,6 +12,9 @@
 #include <sys/time.h>
 #include <signal.h>
 static int bytes_per_timer_alarm;
+
+/* Set of signals to block during critical sections */
+static sigset_t blockingset;
 #endif
 
 #include "xmame.h"
@@ -33,7 +36,7 @@
  * this is not applicable to data samples ( not fixed size ... )
  **************************************************************************/
  
-/* this should be enought ....*/
+/* this should be enough ....*/
 #define ALLOC_TABLE_SIZE 64
 
 static SAMPLE_T		SampleTTable[ALLOC_TABLE_SIZE];
@@ -64,7 +67,7 @@
 SAMPLE_T  *AllocSampleT(void) {
 #if 1
     SAMPLE_T *pt;
-    if (SampleTPointer<0) return (SAMPLE_T *) NULL; /* no items availables */
+    if (SampleTPointer<0) return (SAMPLE_T *) NULL; /* no items available */
     pt = &SampleTTable[SampleTIndex[SampleTPointer--]];
     return pt;
 #else
@@ -238,6 +241,10 @@
 	audio_timer_freq  = AUDIO_TIMER_FREQ;
 #ifdef USE_TIMER
 	bytes_per_timer_alarm=audio_sample_freq/audio_timer_freq;
+ 
+ 	/* Initialise the signal set to block during critical sections */
+ 	sigemptyset(&blockingset);
+ 	sigaddset(&blockingset, SIGALRM);
 #endif
 	InitSampleTTable();
         for (i = 0; i < AUDIO_NUM_VOICES; i++) {
@@ -250,6 +257,9 @@
 
 void osd_set_mastervolume(int volume)
 {
+#ifdef USE_TIMER
+	sigset_t oset;
+#endif
 	int channel;
 	float old_master_volume_divider;
 	
@@ -258,15 +268,29 @@
         old_master_volume_divider=master_volume_divider;
         master_volume_divider=(float)256 * 100 / volume;
         
+#ifdef USE_TIMER
+	sigprocmask(SIG_BLOCK, &blockingset, &oset);
+#endif
+
 	for (channel=0; channel < AUDIO_NUM_VOICES; channel++) {
 	   SAMPLE_T *pt=voices[channel].sample;
 	   for(;pt;pt=pt->next_sample) 
 	      pt->vol *= old_master_volume_divider/master_volume_divider;
 	}
+
+#ifdef USE_TIMER
+	if (!sigismember(&oset, SIGALRM))
+	  sigprocmask(SIG_UNBLOCK, &blockingset, NULL);
+#endif
 }
 
 void osd_play_sample(int channel,signed char *data,int len,int freq,int volume,int loop)
 {
+#ifdef USE_TIMER
+	sigset_t oset;
+#endif
+	SAMPLE_T *new_samp;
+	
         if ((play_sound == 0) || (channel >= AUDIO_NUM_VOICES)) return;
 	if ((freq<10) || (freq> 100000)) 
 	{ 
@@ -275,36 +299,60 @@
 #endif
 	   return;
 	}
-        sound_active=1;
+
+#ifdef USE_TIMER
+ 	sigprocmask(SIG_BLOCK, &blockingset, &oset);
+#endif
 
 	osd_stop_sample(channel);
-	if ( !(voices[channel].sample=AllocSampleT()) ) return;
-	if ( !(voices[channel].sample->data=(signed char *)malloc(len+1)) ) {
-		FreeSampleT(voices[channel].sample);
-		voices[channel].sample=(SAMPLE_T *)NULL;
+
+	if (!(new_samp = AllocSampleT()) ||
+	    !(new_samp->data = (signed char *)malloc(len+1))) {
+	  if (new_samp)
+	    FreeSampleT(new_samp);
+	  
+#ifdef USE_TIMER
+	  if (!sigismember(&oset, SIGALRM))
+	    sigprocmask(SIG_UNBLOCK, &blockingset, NULL);
+#endif
 		return;
 	}
+
+        sound_active=1;	
+
+	voices[channel].sample = new_samp;
+
 #ifdef FANCY_SOUND
         /* calculate one sample more as the actual length for interpolation */
-	if (loop) voices[channel].sample->data[len]=data[0];
-	 else voices[channel].sample->data[len]=0;
+ 	if (loop) new_samp->data[len]=data[0];
+	 else new_samp->data[len]=0;
 #endif	 
-	voices[channel].current_data_pt = voices[channel].sample->data;
-	voices[channel].sample->end_data_pt = voices[channel].sample->data + len - 1;
+	voices[channel].current_data_pt = new_samp->data;
+	new_samp->end_data_pt = new_samp->data + len - 1;
 	voices[channel].pos_frac = 0;
-	voices[channel].sample->next_sample = (SAMPLE_T *) NULL;
-	voices[channel].sample->loop_stream = loop;
- 	voices[channel].sample->vol = (float)volume / master_volume_divider;
-	voices[channel].sample->freq_fac = (float)freq / audio_sample_freq;
-        memcpy(voices[channel].sample->data,data,len);
+	new_samp->next_sample = (SAMPLE_T *) NULL;
+	new_samp->loop_stream = loop;
+ 	new_samp->vol = (float)volume / master_volume_divider;
+	new_samp->freq_fac = (float)freq / audio_sample_freq;
+        memcpy(new_samp->data,data,len);
 #ifdef SOUND_DEBUG
         fprintf(stderr,"play() chan:%d len:%d frec:%d vol:%d loop:%d\n",channel,len,freq,volume,loop); 
 #endif
+
+#ifdef USE_TIMER
+	if (!sigismember(&oset, SIGALRM))
+	  sigprocmask(SIG_UNBLOCK, &blockingset, NULL);
+#endif
+	
 	return;
 }
 
 void osd_play_streamed_sample(int channel,signed char *data,int len,int freq,int volume)
 {
+#ifdef USE_TIMER
+	sigset_t oset;
+#endif
+
 	SAMPLE_T *new_samp, *last_samp=NULL;
 
         if ((play_sound == 0) || (channel >= AUDIO_NUM_VOICES)) return;
@@ -315,25 +363,32 @@
 #endif
 	   return;
 	}
+
+#ifdef USE_TIMER
+	sigprocmask(SIG_BLOCK, &blockingset, &oset);
+#endif
+
+	if (!(new_samp = AllocSampleT()) ||
+	    !(new_samp->data = (signed char *)malloc(len+1))) {
+	  if (new_samp)
+	    FreeSampleT(new_samp);
+	  
+#ifdef USE_TIMER
+	  if (!sigismember(&oset, SIGALRM))
+	    sigprocmask(SIG_UNBLOCK, &blockingset, NULL);
+#endif
+	  
+	  return;
+	}
+	
         sound_active=1;
 
 	if(voices[channel].sample == NULL) {
-	    voices[channel].sample = AllocSampleT();
-	    new_samp= voices[channel].sample;
+	  voices[channel].sample = new_samp;
 	} else {
 	    last_samp = voices[channel].sample;
 	    while (last_samp->next_sample) last_samp=last_samp->next_sample;
-	    last_samp->next_sample = AllocSampleT();
-	    new_samp = last_samp->next_sample;
-	}
-	if (!new_samp) return; /* no resources availables */	
-	new_samp->data = (signed char *)malloc(len+1);
-	if (! new_samp->data) {
-	    if ( new_samp == voices[channel].sample ) 
-		 voices[channel].sample = NULL;
-	    else last_samp->next_sample = NULL;
-	    FreeSampleT(new_samp);
-	    return; /* cannot malloc data space: free sampleT and return */
+	  last_samp->next_sample = new_samp;
 	}
 #ifdef FANCY_SOUND
         /* calculate one sample more as the actual length for interpolation */
@@ -351,10 +406,19 @@
 	new_samp->vol = (float)volume / master_volume_divider;
 	new_samp->freq_fac = (float)freq / audio_sample_freq;
         memcpy(new_samp->data,data,len);
+	
+#ifdef USE_TIMER
+	if (!sigismember(&oset, SIGALRM))
+	  sigprocmask(SIG_UNBLOCK, &blockingset, NULL);
+#endif
 }
 
 void osd_adjust_sample(int channel,int freq,int volume)
 {
+#ifdef USE_TIMER
+	sigset_t oset;
+#endif
+
 	SAMPLE_T *next_samp;
         if (play_sound == 0 || channel >= AUDIO_NUM_VOICES) return;
 	if ((freq<10) || (freq> 100000)) 
@@ -364,6 +428,11 @@
 #endif
 	   return;
 	}
+
+#ifdef USE_TIMER
+	sigprocmask(SIG_BLOCK, &blockingset, &oset);
+#endif
+	
 	if(voices[channel].sample != NULL) {
 	    next_samp = voices[channel].sample;
 	    while(next_samp != NULL) {
@@ -372,14 +441,27 @@
 	    	next_samp = next_samp->next_sample;
 	    }
 	}
+
+#ifdef USE_TIMER
+	if (!sigismember(&oset, SIGALRM))
+	  sigprocmask(SIG_UNBLOCK, &blockingset, NULL);
+#endif
 }
 
 void osd_stop_sample(int channel)
 {
+#ifdef USE_TIMER
+	sigset_t oset;
+#endif
+  
 	SAMPLE_T *next_samp;
 	SAMPLE_T *curr_samp;
         if (play_sound == 0 || channel >= AUDIO_NUM_VOICES) return;
 
+#ifdef USE_TIMER
+	sigprocmask(SIG_BLOCK, &blockingset, &oset);
+#endif
+	
 	if(voices[channel].sample != NULL) {
 	    next_samp = voices[channel].sample;
 	    voices[channel].sample = NULL;
@@ -392,12 +474,31 @@
 	    	FreeSampleT(curr_samp);
 	    }
 	}
+  
+#ifdef USE_TIMER
+	if (!sigismember(&oset, SIGALRM))
+	  sigprocmask(SIG_UNBLOCK, &blockingset, NULL);
+#endif
 }
 
 void osd_restart_sample(int channel)
 {
+#ifdef USE_TIMER
+	sigset_t oset;
+#endif
+
 	if (play_sound == 0 || channel >= AUDIO_NUM_VOICES) return;
+
+#ifdef USE_TIMER
+	sigprocmask(SIG_BLOCK, &blockingset, &oset);
+#endif
+  
 	voices[channel].current_data_pt = voices[channel].sample->data;
+  
+#ifdef USE_TIMER
+	if (!sigismember(&oset, SIGALRM))
+	  sigprocmask(SIG_UNBLOCK, &blockingset, NULL);
+#endif
 }
 
 int osd_get_sample_status(int channel)

>Audit-Trail:
>Unformatted: