Re: [omniORB] omniORBpy deadlock / termination problem.

Harri Pasanen <[email protected]> Mon, 13 Oct 2003 11:17:43 +0200
Newsgroups gmane.comp.corba.omniorb.devel
Message-ID <[email protected]> 
On Friday 03 October 2003 17:23, you wrote:
> On Monday 22 September, Harri Pasanen wrote:
> > I don't have time to dig in further at this time.  Following
> > backtrace shows what the threads are doing at the moment of the
> > deadlock, which seems to be over the python's GIL, threads 7 & 1,
> > pyThreadCache.cc
>
> Thread 7 is trying to acquire the GIL. Thread 1 is waiting for
> thread 7 to exit.
>
> I suspect the problem is caused by either thread 1 or thread 3,
> both of which have come through your application code. Is one of
> them holding the GIL?  If so, that would explain the deadlock.

Attached is our latest patch to omniORBpy to remedy the situation.  A 
collegue, (Teemu) took up from where I had arrived to and analyzed 
what was happening and came up with this patch.

Changes introduced:

1. A shutdown method that is registered with Python's atexit module.  
This is required to properly close pyThreadCache, which needs to run 
Python code, so python still needs to be present.   Otherwise it can 
be that  when pyThreadCache is destructed at omniORB cleanup during  
process exit, the python interpreter has already gone.

2. pyThreadCache join needs to be in python's ALLOW_THREADS macros, 
otherwise pyThreadCache cannot be shut down from a python method.  
(And the previous atexit change should ensure it is always called 
from a python method).

3. pyThreadCache table mutex was split from thread kill mutex to 
optimize the thread killing, the former mutex being too long lived.

Let us know what you think of the patch.

Harri

_______________________________________________
omniORB-dev mailing list
[email protected]
http://www.omniorb-support.com/mailman/listinfo/omniorb-dev
omniorbpy-deadlock-fix.diff (text/x-diff, 3.8 KB) 
Index: modules/omnipy.cc
===================================================================
RCS file: /trema/cvs/fk/tools/omniorbpy/modules/omnipy.cc,v
retrieving revision 1.1.1.1
retrieving revision 1.2
diff -c -u -r1.1.1.1 -r1.2
--- modules/omnipy.cc	7 Oct 2003 12:07:35 -0000	1.1.1.1
+++ modules/omnipy.cc	11 Oct 2003 12:07:04 -0000	1.2
@@ -515,6 +515,15 @@
     return Py_None;
   }
 
+  static PyObject*
+  omnipy_shutdown(PyObject* self, PyObject* args)
+  {
+    omnipyThreadCache::shutdown();
+
+    Py_INCREF(Py_None);
+    return Py_None;
+  }
+
   ////////////////////////////////////////////////////////////////////////////
   // CDR stream marshalling/unmarshalling                                   //
   ////////////////////////////////////////////////////////////////////////////
@@ -844,6 +853,7 @@
     {(char*)"cdrMarshal",        omnipy_cdrMarshal,              METH_VARARGS},
     {(char*)"cdrUnmarshal",      omnipy_cdrUnmarshal,            METH_VARARGS},
     {(char*)"need_ORB_init",     omnipy_need_ORB_init,           METH_VARARGS},
+    {(char*)"shutdown",          omnipy_shutdown,           METH_VARARGS},
 
     // Wrappers for functions in CORBA::
     {(char*)"ORB_init",          omnipy_ORB_init,                METH_VARARGS},
Index: modules/pyThreadCache.cc
===================================================================
RCS file: /trema/cvs/fk/tools/omniorbpy/modules/pyThreadCache.cc,v
retrieving revision 1.1.1.1
retrieving revision 1.3
diff -c -u -r1.1.1.1 -r1.3
--- modules/pyThreadCache.cc	7 Oct 2003 12:07:35 -0000	1.1.1.1
+++ modules/pyThreadCache.cc	11 Oct 2003 13:06:38 -0000	1.3
@@ -62,23 +62,27 @@
 
 class omnipyThreadScavenger : public omni_thread {
 public:
-  omnipyThreadScavenger() : dying_(0), cond_(omnipyThreadCache::guard) {
+  omnipyThreadScavenger() : dying_(0), cond_(&mutex_) {
     start_undetached();
   }
   ~omnipyThreadScavenger() { }
 
   void kill() {
     {
-      omni_mutex_lock l(*omnipyThreadCache::guard);
+      omni_mutex_lock l(mutex_);
       dying_ = 1;
       cond_.signal();
     }
+
+    Py_BEGIN_ALLOW_THREADS
     join(0);
+    Py_END_ALLOW_THREADS
   }
 
   void* run_undetached(void*);
 private:
   CORBA::Boolean dying_;
+  omni_mutex mutex_;
   omni_condition cond_;
   PyThreadState* threadState_;
   PyObject*      workerThread_;
@@ -244,8 +248,6 @@
 
   omniORB::logs(15, "Python thread state scavenger start.");
 
-  omni_mutex_lock l(*omnipyThreadCache::guard);
-
   // Create a thread state for the scavenger thread itself
   PyThreadState* oldState;
   PyEval_AcquireLock();
@@ -257,15 +259,19 @@
   PyEval_ReleaseLock();
 
   // Main loop
+  mutex_.lock();
   while (!dying_) {
     omni_thread::get_time(&abs_sec,&abs_nsec);
     abs_sec += omnipyThreadCache::scanPeriod;
     cond_.timedwait(abs_sec, abs_nsec);
 
     if (dying_) break;
+    mutex_.unlock();
 
     omniORB::logs(15, "Scanning Python thread states.");
     
+    omni_mutex_lock l(*omnipyThreadCache::guard);
+
     for (i=0; i < omnipyThreadCache::tableSize; i++) {
       cn = omnipyThreadCache::table[i];
 
@@ -318,11 +324,15 @@
 	cn = cn->next;
       }
     }
+    mutex_.lock();
   }
+  mutex_.unlock();
 
   // Delete all table entries
   PyEval_AcquireLock();
   oldState = PyThreadState_Swap(threadState_);
+
+  omni_mutex_lock l(*omnipyThreadCache::guard);
 
   for (i=0; i < omnipyThreadCache::tableSize; i++) {
     cn = omnipyThreadCache::table[i];
Index: python/omniORB/__init__.py
===================================================================
RCS file: /trema/cvs/fk/tools/omniorbpy/python/omniORB/__init__.py,v
retrieving revision 1.1.1.1
retrieving revision 1.2
diff -c -u -r1.1.1.1 -r1.2
--- python/omniORB/__init__.py	7 Oct 2003 12:07:35 -0000	1.1.1.1
+++ python/omniORB/__init__.py	11 Oct 2003 12:07:02 -0000	1.2
@@ -836,3 +836,6 @@
     importIRStubs()
 
 del omniORB
+
+import atexit
+atexit.register(_omnipy.shutdown)