omniORB 4.0.1 destroy race condition

Teemu Torma <[email protected]> Thu, 8 May 2003 21:51:56 +0200
Newsgroups gmane.comp.corba.omniorb.devel
Message-ID <[email protected]> 
To whom it may concern.

Whilst testing forthcoming valgrind 1.9.6 snapshot with omniORB 
application, it started reporting of mutex being unlocked by another 
thread than locker, or destroying a mutex still in use.

By reviewing poa.cc it seems that this problem can happen in two 
different places where we are doing

     while( pd_destroyed != 2 )  pd_deathSignal.wait();

but not holding a reference to the poa.  Thus when pd_deathSignal.wait() 
returns, in re-aquires the pd_lock, but at that point the poa might be 
being destructed or already completely deleted depending on the 
scheduling issues.

By making the necessary changes to have a reference count to the poa in 
these two places, the problem went away, and valgrind has been happy 
ever since.  Please see the attached patch.

Teemu

_______________________________________________
omniORB-dev mailing list
[email protected]
http://www.omniorb-support.com/mailman/listinfo/omniorb-dev
poa.diff (text/x-diff, 1.6 KB) 
Index: src/lib/omniORB/orbcore/poa.cc
===================================================================
RCS file: /trema/cvs/fk/tools/omniorb/src/lib/omniORB/orbcore/poa.cc,v
retrieving revision 1.4
diff -c -u -r1.4 poa.cc
--- src/lib/omniORB/orbcore/poa.cc	26 Mar 2003 23:15:42 -0000	1.4
+++ src/lib/omniORB/orbcore/poa.cc	8 May 2003 19:43:16 -0000
@@ -698,22 +698,37 @@
   // invocations) and then remove self from parent.
 
   {
-    omni_tracedmutex_lock sync(pd_lock);
+    pd_lock.lock();
 
-    if( pd_destroyed )  OMNIORB_THROW(OBJECT_NOT_EXIST,
-				      OBJECT_NOT_EXIST_POANotInitialised,
-				      CORBA::COMPLETED_NO);
+    if( pd_destroyed ) {
+      pd_lock.unlock ();
+      OMNIORB_THROW(OBJECT_NOT_EXIST,
+		    OBJECT_NOT_EXIST_POANotInitialised,
+		    CORBA::COMPLETED_NO);
+    }
 
     if( pd_dying ) {
       // Need to be able to handle multiple concurrent calls to
       // destroy.  If destruction is in progress and wait_f_c is
       // true, must wait to complete.  Otherwise can just return.
-      if( wait_for_completion )
+      if( wait_for_completion ) {
+
+	incrRefCount();
+
 	while( pd_destroyed != 2 )  pd_deathSignal.wait();
+
+	pd_lock.unlock ();
+	decrRefCount();
+
+      } else {
+	pd_lock.unlock ();
+      }
       return;
     }
 
     pd_dying = 1;
+
+    pd_lock.unlock ();
   }
 
   {
@@ -2193,8 +2208,10 @@
   if( pd_dying ) {
     // If being destroyed by another thread, then we just
     // have to wait until that completes.
+    incrRefCount();
     while( pd_destroyed != 2 )  pd_deathSignal.wait();
     pd_lock.unlock();
+    decrRefCount();
     return;
   }