From 6d660b475f940e04284174a0a68d5fadc1ce47ba Mon Sep 17 00:00:00 2001
From: Anthony Geay <anthony.geay@edf.fr>
Date: Wed, 25 Sep 2019 11:21:22 +0200
Subject: [PATCH] Key point. OK for GUI persalys + python persalys + Test C++.
 Tests follow new explicit rule

---
 AdaoExchangeLayer.cxx | 30 +++++++++++++++++++++++++-----
 README.txt            |  4 ++++
 TestAdaoExchange.cxx  | 20 ++++++++++++++++----
 3 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/AdaoExchangeLayer.cxx b/AdaoExchangeLayer.cxx
index 09e396f..c5c15b8 100644
--- a/AdaoExchangeLayer.cxx
+++ b/AdaoExchangeLayer.cxx
@@ -168,7 +168,8 @@ class AdaoExchangeLayer::Internal
 {
 public:
   Internal():_context(PyObjectRAII::FromNew(PyDict_New()))
-  { 
+  {
+    AutoGIL agil;
     PyObject *mainmod(PyImport_AddModule("__main__"));
     PyObject *globals(PyModule_GetDict(mainmod));
     PyObject *bltins(PyEval_GetBuiltins());
@@ -227,6 +228,17 @@ PyObject *AdaoExchangeLayer::getPythonContext() const
   return _internal->_context;
 }
 
+/*!
+ * AdaoExchangeLayer is based on multithreaded paradigm.
+ * Master thread (thread calling this method) and slave thread (thread calling ADAO algo)
+ * are calling both python interpretor. Consequence all python calls have to be surrounded with AGIL.
+ *
+ * User consequence : To avoid deadlocks this method release GIL. The downstream python calls must be with AGIL.
+ * 
+ * This method initialize python interpretor if not already the case.
+ * At the end of this method the lock is released to be ready to perform RAII on GIL
+ * easily. 
+ */
 void AdaoExchangeLayer::initPythonIfNeeded()
 {
   if (!Py_IsInitialized())
@@ -239,9 +251,17 @@ void AdaoExchangeLayer::initPythonIfNeeded()
       PySys_SetArgv(1,TABW);
       FreeWChar(1,TABW);
       PyEval_InitThreads();
+      delete _internal;
+      _internal = new Internal;
+      _internal->_tstate=PyEval_SaveThread(); // release the lock acquired in AdaoExchangeLayer::initPythonIfNeeded by PyEval_InitThreads()
+    }
+  else
+    {
+      delete _internal;
+      _internal = new Internal;
+      if( PyGILState_Check() )// is the GIL already acquired (typically by a PyEval_InitThreads) ?
+        _internal->_tstate=PyEval_SaveThread(); // release the lock acquired upstream
     }
-  delete _internal;
-  _internal = new Internal;
 }
 
 class Visitor1 : public AdaoModel::PythonLeafVisitor
@@ -335,7 +355,6 @@ void ExecuteAsync(PyObject *pyExecuteFunction, DataExchangedBetweenThreads *data
 
 void AdaoExchangeLayer::execute()
 {
-  _internal->_tstate=PyEval_SaveThread(); // release the lock acquired in AdaoExchangeLayer::initPythonIfNeeded by PyEval_InitThreads()
   _internal->_fut = std::async(std::launch::async,ExecuteAsync,_internal->_execute_func,&_internal->_data_btw_threads);
 }
 
@@ -364,7 +383,8 @@ void AdaoExchangeLayer::setResult(PyObject *outputAssociated)
 PyObject *AdaoExchangeLayer::getResult()
 {
   _internal->_fut.wait();
-  PyEval_RestoreThread(_internal->_tstate);
+  if(_internal->_tstate)
+    PyEval_RestoreThread(_internal->_tstate);
   AutoGIL gil;
   // now retrieve case.get("Analysis")[-1]
   PyObjectRAII get_func_of_adao_case(PyObjectRAII::FromNew(PyObject_GetAttrString(_internal->_adao_case,"get")));
diff --git a/README.txt b/README.txt
index a7ac090..5ca7076 100644
--- a/README.txt
+++ b/README.txt
@@ -10,6 +10,10 @@ AdaoExchangeLayer manage the GIL (using AutoGIL) in its implementation.
 
 Consequence the AdaoExchangeLayer instance should NOT be surrounded by GIL protection.
 
+Consequence : AdaoExchangeLayer::initPythonIfNeeded initializes the python interpretor (if not already done) and let the GIL released to manage all python caller threads easily
+
+Consequence : Using ADAO interface implies to protect python calls
+
 ############## user GIL management in AdaoModel::MainModel
 
 The custom MainModel overloading should be GIL protected by the user.
diff --git a/TestAdaoExchange.cxx b/TestAdaoExchange.cxx
index f0d7150..30d2949 100644
--- a/TestAdaoExchange.cxx
+++ b/TestAdaoExchange.cxx
@@ -125,7 +125,10 @@ void AdaoExchangeTest::test3DVar()
   adao.init();
   // For bounds, Background/Vector, Observation/Vector
   Visitor2 visitorPythonObj(adao.getPythonContext());
-  mm.visitPythonLeaves(&visitorPythonObj);
+  {
+    AutoGIL agil;
+    mm.visitPythonLeaves(&visitorPythonObj);
+  }
   //
   adao.loadTemplate(&mm);
   //
@@ -179,7 +182,10 @@ void AdaoExchangeTest::testBlue()
   adao.init();
   // For bounds, Background/Vector, Observation/Vector
   Visitor2 visitorPythonObj(adao.getPythonContext());
-  mm.visitPythonLeaves(&visitorPythonObj);
+  {
+    AutoGIL agil;
+    mm.visitPythonLeaves(&visitorPythonObj);
+  }
   //
   adao.loadTemplate(&mm);
   //
@@ -232,7 +238,10 @@ void AdaoExchangeTest::testNonLinearLeastSquares()
   adao.init();
   // For bounds, Background/Vector, Observation/Vector
   Visitor2 visitorPythonObj(adao.getPythonContext());
-  mm.visitPythonLeaves(&visitorPythonObj);
+  {
+    AutoGIL agil;
+    mm.visitPythonLeaves(&visitorPythonObj);
+  }
   //
   adao.loadTemplate(&mm);
   //
@@ -269,7 +278,10 @@ void AdaoExchangeTest::testCasCrue()
   adao.init();
   // For bounds, Background/Vector, Observation/Vector
   VisitorCruePython visitorPythonObj(adao.getPythonContext());
-  mm.visitPythonLeaves(&visitorPythonObj);
+  {
+    AutoGIL agil;
+    mm.visitPythonLeaves(&visitorPythonObj);
+  }
   //
   adao.loadTemplate(&mm);
   //
-- 
2.30.2