[gst-devel] [PATCH] SMP race condition in gstatomic.h

[Cameron Hutchison]

I've been banging my head against a bug in gstreamer for the last few
days (bug #92261). This bug can be triggered by the command:
$ gst-launch sinesrc ! { queue ! fakesink }

After a random number of iterations an assertion is violated when trying
to unref a buffer.

The problem is that the SWAP functions/macros in gstatomic.h are not
being compiled for SMP. That header uses the __SMP__ conditional macro,
but nothing sets that macro.

Following the method used in <asm/atomic.h>, I've changed __SMP__ to
CONFIG_SMP (defined in <linux/config.h>). This fixes the problem
("lock ;" now gets included in the asm code).

One thing to note about this is that all gstreamer code will be compiled
for SMP machines anyway, since <linux/config.h> is part of glibc (it
should never be a symlink to the kernel source), and config.h defines
CONFIG_SMP to 1. This is as it should be, otherwise binaries built on a
UP machine will fail to run on a SMP machine. This leads me to believe
that the part of gstatomic.h that is conditional on __SMP__ (now
CONFIG_SMP) should be removed and the SMP case should be there for all
builds. The only reason for the CONFIG_SMP conditional in <asm/atomic.h>
is that kernels are built for UP or SMP. This should not be the case for
userspace applications. All applications should be built to work on SMP
and UP without needing a recompile. Comments?

This should close bug #92261 which I reported a few days ago. Should I
close this myself or will a core member do it?

BTW. __SMP__ is also used in two tests:
  tests/bufspeed/gstmempool.c
  tests/memchunk/gstmemchunk.c

These should probably be changed to make the SMP case the only case.

Patch attached...

PS. Wouldn't you know it, I've just got some work for the next month
starting tomorrow - there goes most of my time I planned to use to muck
around with gstreamer.

-------------- next part --------------
Index: gstreamer/gst/gstatomic.h
===================================================================
RCS file: /cvsroot/gstreamer/gstreamer/gst/gstatomic.h,v
retrieving revision 1.3
diff -u -u -r1.3 gstatomic.h
--- gstreamer/gst/gstatomic.h	13 Jul 2002 23:08:04 -0000	1.3
+++ gstreamer/gst/gstatomic.h	5 Sep 2002 07:48:28 -0000
@@ -125,7 +125,7 @@
 
 #if defined (__i386__) && defined (__GNUC__) && __GNUC__ >= 2 
 
-# ifdef __SMP__
+# ifdef CONFIG_SMP
 #  define GST_ATOMIC_LOCK "lock ; "
 # else
 #  define GST_ATOMIC_LOCK ""