Re: [GnomeMeeting-devel-list] Crash in PFactory::~PFactory()



Am Donnerstag, 12. Mai 2005 02:33 schrieb Craig Southeren:
>
> I'll see what I can do in the short time before I leave for vacation. A
> simple program that demonstrates the problem would be very helpful.
>
> Also realise that I do not have ALSA on my Linux box :(
>
>    Craig

Hi Craig,

to see the double registration, even the small hello.cxx from the pwlib docu 
is enough. Just set a breakpoint on "PFactory<PSoundChannel, 
PString>::Register_Internal(...)" and you will see it is hit 2 times per 
audio plugin.

I have created a patch which fixes the issue for me and I think is correct. It 
is a little bit ugly because I have to find the key the worker is registered 
for, it would be nicer if the Worker class would save the keyname. The 
destructor is just the counterpart of the constructor and will fix the 
dangling pointer problem.

The second part of the patch makes sure that there is only a single concrete 
class per factory. The singleton flag IMHO is wrong, as it is for instances, 
not for unique keys. There can only be one concrete class per key, but there 
may be multiple instances (in real life, almost every VoIP application will 
have at least two PSoundChannel(ALSA|OSS) instances ...). Maybe there should 
even be a warning message in PFactory<>::Register if it is called for an 
already existing key.

Greetings,

Stefan



-- 
Stefan Brüns  /  Kastanienweg 6 - Zimmer 1206  /  52074 Aachen
mailto:lurch gmx li  http://www.kawo1.rwth-aachen.de/~lurchi/
   phone: +49 241 169-4206     mobile: +49 160 3797725
--- include/ptlib/plugin.h.orig	2004-08-16 13:57:47.000000000 +0200
+++ include/ptlib/plugin.h	2005-05-13 18:54:35.000000000 +0200
@@ -64,6 +64,25 @@
           : PFactory<_Abstract_T, _Key_T>::WorkerBase(singleton)
         {
           PFactory<_Abstract_T, _Key_T>::Register(key, this);
+        }
+
+        ~Worker()
+        {
+          typedef typename PFactory<_Abstract_T, _Key_T>::WorkerBase WorkerBase_T;
+          typedef std::map<_Key_T, WorkerBase_T *> KeyMap_T;
+          _Key_T key;
+
+          KeyMap_T km = PFactory<_Abstract_T, _Key_T>::GetKeyMap();
+
+          typename KeyMap_T::const_iterator entry;
+          for (entry = km.begin(); entry != km.end(); ++entry) {
+            if ( entry->second == this ) {
+              key = entry->first;
+              break;
+            }
+          }
+
+          PFactory<_Abstract_T, _Key_T>::Unregister(key);
         }
 
       protected:
@@ -88,7 +110,10 @@
     typedef PDevicePluginFactory<DeviceBase> Factory_T;
     typedef typename Factory_T::Worker Worker_T;
     void CreateFactory(const PString & device)
-    { new Worker_T(device, TRUE); }
+    {
+      if( !(Factory_T::IsRegistered(device)) )
+        new Worker_T(device, FALSE);
+    }
 };
 
 #define PWLIB_PLUGIN_API_VERSION 0

Attachment: pgp33o08oNAjK.pgp
Description: PGP signature



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