Re: [Gap-dev-discuss] Cynthiune crash (AO)

[Sebastian Reitenbach]

On Monday, May 7, 2012 09:22 CEST, Riccardo Mottola <address@hidden> wrote: 
 
> Hi,
> 
> On 05/07/12 09:04, Sebastian Reitenbach wrote:
> >
> >
> > Has it happen while jumping from one song to the next? Did those songs
> >
> > had different bit rate or number of channels?
> > If yes, maybe its a race condition in the prepareDevice... method
> > when closing the device, and the handle is already NULL.
> > For me, it crashed in ao_plugin_close and in ao_plugin_device_clear,
> > because those functions were called with the NULL as parameter.
> > PPC seems to be a bit more picky about NULL pointer dereferences.
> > After preventing to call those methods with NULL as parameter,
> > I also got the double free, and Cynthiune aborted. Maybe when I test
> > it more and longer on i386, I'd get the same problem.
> >
> > On OpenBSD I have libao-1.1.0.
> >
> >
> I will check the version at home. The player was going from one song to 
> the other and at the 2nd or 3rd it crashed.
> 
> Usually MP3s area always stereo and 16bit. The bitrate of the song might 
> change, but that shouldn't impact the backend, or am i wrong?
> 
> In any case, the two lines before the crash:
> ao_alsa WARNING: ao_plugin_close called with uninitialized 
> ao_device->internal
> ao_alsa WARNING: ao_plugin_device_clear called with uninitialized 
> ao_device->internal
> 
> Point out to something illegal which can of course cause any kind of havoc.
> 
> Before, when I played a bit with next/previous buttons, nothing 
> happened. If it were consistent it would be easier to run it through GDB.

The patch below works for me on macppc. I did put the lock outside the if clause
in prepareDevice... method.


Index: Bundles/AO/ao.m
===================================================================
RCS file: /sources/gap/gap/user-apps/Cynthiune/Bundles/AO/ao.m,v
retrieving revision 1.2
diff -u -r1.2 ao.m
--- Bundles/AO/ao.m     5 May 2012 12:23:41 -0000       1.2
+++ Bundles/AO/ao.m     7 May 2012 15:45:21 -0000
@@ -85,19 +85,22 @@
 - (BOOL) prepareDeviceWithChannels: (unsigned int) numberOfChannels
                            andRate: (unsigned long) sampleRate
 {
+  BOOL result = NO;
+
   format.channels = (int)numberOfChannels;
   format.rate = (int)sampleRate;
   /* FIXME : this should somehow come from the input bundle */
   format.bits = 16;
   format.byte_format = AO_FMT_LITTLE;
-  if (dev) {
-    [devlock lock];
-    ao_close(dev);
-    dev = ao_open_live(driver, &format, NULL);
-    [devlock unlock];
-    return ((dev == NULL) ? NO : YES);
-  }
-  return YES;
+  [devlock lock];
+  if (dev)
+    {
+      ao_close(dev);
+      dev = ao_open_live(driver, &format, NULL);
+      result = (dev == NULL) ? NO : YES;
+    }
+  [devlock unlock];
+  return result;
 }
 
 - (BOOL) openDevice




> 
> Riccardo
>