BUG: race condition triggered in cReceiver due to incorrect use by femon

  Hi,

I've just got a pure virtual function call in cDevice::Action() for
receiver[i]->Receive(b, TS_SIZE);

It has happened while closing femon's OSD:

(gdb) bt
#0  0xffffe410 in __kernel_vsyscall ()
#1  0xb7f2fc4e in __lll_mutex_lock_wait () from /lib/libpthread.so.0
#2  0xb7f2ba3c in _L_mutex_lock_88 () from /lib/libpthread.so.0
#3  0xb7f2b42d in pthread_mutex_lock () from /lib/libpthread.so.0
#4  0x08124281 in cMutex::Lock (this=0xb762c48) at thread.c:190
#5  0x080a7cdc in cThread::Lock (this=0xb762c38) at thread.h:91
#6  0x080a5548 in cDevice::Detach (this=0xb762c38, Receiver=0xb099ab0)
at device.c:1298
#7  0x080effb9 in cReceiver::Detach (this=0xb099ab0) at receiver.c:60
#8  0x080f0090 in ~cReceiver (this=0xb099ab0) at receiver.c:43
#9  0xb799ac29 in cFemonReceiver::~cFemonReceiver () from
/soft/vdr-1.4.6/lib/vdr/libvdr-femon.so.1.4.5
#10 0xb7998b05 in cFemonOsd::~cFemonOsd () from
/soft/vdr-1.4.6/lib/vdr/libvdr-femon.so.1.4.5
#11 0x08131da7 in main (argc=20, argv=0xbf90e914) at vdr.c:885
#12 0xb7ce6f9c in __libc_start_main () from /lib/libc.so.6
#13 0x08091111 in _start ()
(gdb)

cReceiver::~cReceiver() calls Detach(), but at that time, it is simply
too late as the virtual method table entry for Receive() has already
been restored to 0.

I suggest to call Detach() from cFemonReceiver::~cFemonReceiver() to fix
this issue.

Furthermore I suggest to add some debug output to ~cReceiver() in the
case where the receiver is still attached. I've been using femon for
quite some time now and it was the first time, that VDR dumped a core
regarding a pure virtual function call. Such a debug output would have
triggered way more early.

Bye.
  

On Mon, 14 May 2007, Reinhard Nissl wrote:

> I suggest to call Detach() from cFemonReceiver::~cFemonReceiver() to fix
> this issue.

Thanks. A fixed plugin, vdr-femon-1.1.3, is now available on its' 
homepage.

--
rofa
  

Reinhard Nissl wrote:
> I've just got a pure virtual function call in cDevice::Action() for
> receiver[i]->Receive(b, TS_SIZE);
> 
> cReceiver::~cReceiver() calls Detach(), but at that time, it is simply
> too late as the virtual method table entry for Receive() has already
> been restored to 0.
> 
> I suggest to call Detach() from cFemonReceiver::~cFemonReceiver() to fix
> this issue.

Calling virtual functions from destructors is generally not a good idea. 
It works if the call is in the deepest nested destructor, but within a 
base class destructor, virtual functions are already being destructed 
and reverted to the base class. (here: to a pure virtual dummy)

The best method to avoid this is to do all cleanup involving virtual 
functions before destroying the object. If this is not possible, it also 
works to do the cleanup in EVERY derived class' destructor.

This is extremely nasty within a cThread, where you have to make sure 
that the thread got stopped _before_ the destructor gets called, or you 
risk to 'steal' the virtual functions from the running thread.

Cheers,

Udo