From accfa6fec5c2b3f25cd375eb136afd1aa4f2354a Mon Sep 17 00:00:00 2001 From: Anthony Geay Date: Fri, 6 Jan 2017 08:13:34 +0100 Subject: [PATCH] Bug correction revealed by stress test. The CORBA invokation inside CORBA invokation breaked the MONOTHREAD property. Why ? --- idl/SALOME_SDS.idl | 2 +- src/SALOMESDS/SALOMESDS_DataScopeServer.cxx | 8 +++++--- src/SALOMESDS/SALOMESDS_KeyWaiter.cxx | 19 +------------------ src/SALOMESDS/SALOMESDS_KeyWaiter.hxx | 2 +- src/SALOMESDS/SALOME_DataScopeServer.cxx | 4 +++- src/SALOMESDS/TestSalomeSDS.py | 15 ++++++++++----- src/SALOMESDS/TestSalomeSDSHelper0.py | 3 ++- 7 files changed, 23 insertions(+), 30 deletions(-) diff --git a/idl/SALOME_SDS.idl b/idl/SALOME_SDS.idl index e56de99f3..c5d7e288d 100644 --- a/idl/SALOME_SDS.idl +++ b/idl/SALOME_SDS.idl @@ -109,7 +109,7 @@ module SALOME interface KeyWaiter { - ByteVec waitFor() raises (SALOME::SALOME_Exception); + void waitFor() raises (SALOME::SALOME_Exception); }; interface DataScopeServerTransaction : DataScopeServerBase diff --git a/src/SALOMESDS/SALOMESDS_DataScopeServer.cxx b/src/SALOMESDS/SALOMESDS_DataScopeServer.cxx index 23b809b87..92dae7794 100644 --- a/src/SALOMESDS/SALOMESDS_DataScopeServer.cxx +++ b/src/SALOMESDS/SALOMESDS_DataScopeServer.cxx @@ -759,14 +759,16 @@ SALOME::ByteVec *DataScopeServerTransaction::waitForMonoThrRev(SALOME::KeyWaiter PortableServer::ServantBase *ret(0); try { - ret=_poa_for_key_waiter->reference_to_servant(kw); + ret=_poa_for_key_waiter->reference_to_servant(kw);// Warning ref cnt of ret has been incremented ! } catch(...) { ret=0; } KeyWaiter *retc(dynamic_cast(ret)); if(!retc) throw Exception("DataScopeServerTransaction::invokeMonoThr : internal error 1 !"); - retc->_remove_ref(); - return retc->waitForMonoThr(); + retc->_remove_ref();// restore the counter afer _poa_for_key_waiter->reference_to_servant(kw) + SALOME::ByteVec *zeRet(retc->waitForMonoThr()); + retc->enforcedRelease(); + return zeRet; } void DataScopeServerTransaction::atomicApply(const SALOME::ListOfTransaction& transactions) diff --git a/src/SALOMESDS/SALOMESDS_KeyWaiter.cxx b/src/SALOMESDS/SALOMESDS_KeyWaiter.cxx index 7f18d8569..ca951bcda 100644 --- a/src/SALOMESDS/SALOMESDS_KeyWaiter.cxx +++ b/src/SALOMESDS/SALOMESDS_KeyWaiter.cxx @@ -78,28 +78,11 @@ PortableServer::POA_var KeyWaiter::getPOA() const * WARNING : here it is the single method that can be invoked in non SINGLE_THREAD POA. * So take care to do avoid collapses (especially in python). */ -SALOME::ByteVec *KeyWaiter::waitFor() +void KeyWaiter::waitFor() { sem_wait(&_sem); if(!_ze_value) throw Exception("KeyWaiter::waitFor : internal error 1 !"); - SALOME::ByteVec *ret(0); - {// this section is to guarantee that no concurrent threads are doing python stuff at the same time - // Here a pickelization is needed so to guarantee to be alone doing python action the idea is to invoke using monothread POA. - DataScopeServerTransaction *dss(getDSS()); - PortableServer::POA_var poa(dss->getPOA()); - CORBA::Object_var dssPtr(poa->servant_to_reference(dss)); - SALOME::DataScopeServerTransaction_var dssPtr2(SALOME::DataScopeServerTransaction::_narrow(dssPtr)); - if(CORBA::is_nil(dssPtr2)) - throw Exception("KeyWaiter::waitFor : internal error 2 !"); - CORBA::Object_var thisPtr(getPOA()->servant_to_reference(this)); - SALOME::KeyWaiter_var thisPtr2(SALOME::KeyWaiter::_narrow(thisPtr)); - if(CORBA::is_nil(thisPtr2)) - throw Exception("KeyWaiter::waitFor : internal error 3 !"); - ret=dssPtr2->waitForMonoThrRev(thisPtr2);//<- this invokation through SINGLE_THREAD POA here will guarantee thread safety - } - enforcedRelease(); - return ret; } /*! diff --git a/src/SALOMESDS/SALOMESDS_KeyWaiter.hxx b/src/SALOMESDS/SALOMESDS_KeyWaiter.hxx index 0676c16ae..7f3eee1b0 100644 --- a/src/SALOMESDS/SALOMESDS_KeyWaiter.hxx +++ b/src/SALOMESDS/SALOMESDS_KeyWaiter.hxx @@ -44,7 +44,7 @@ namespace SALOMESDS PyObject *getKeyPyObj() const { return _ze_key; } virtual ~KeyWaiter(); PortableServer::POA_var getPOA() const; - SALOME::ByteVec *waitFor(); + void waitFor(); void valueJustCome(PyObject *val); void go(); std::string getVarName() const { return _var->getVarNameCpp(); } diff --git a/src/SALOMESDS/SALOME_DataScopeServer.cxx b/src/SALOMESDS/SALOME_DataScopeServer.cxx index f0bda405b..f7daac616 100644 --- a/src/SALOMESDS/SALOME_DataScopeServer.cxx +++ b/src/SALOMESDS/SALOME_DataScopeServer.cxx @@ -52,9 +52,11 @@ int main(int argc, char *argv[]) server=new SALOMESDS::DataScopeServerTransaction(orb,killerObj,scopeName); // CORBA::PolicyList policies; - policies.length(1); + policies.length(3); PortableServer::ThreadPolicy_var threadPol(poa->create_thread_policy(PortableServer::SINGLE_THREAD_MODEL)); policies[0]=PortableServer::ThreadPolicy::_duplicate(threadPol); + policies[1]=poa->create_implicit_activation_policy(PortableServer::NO_IMPLICIT_ACTIVATION); + policies[2]=poa->create_id_uniqueness_policy(PortableServer::UNIQUE_ID); PortableServer::POA_var poa2(poa->create_POA("SingleThPOA4SDS",mgr,policies)); threadPol->destroy(); server->initializePython(argc,argv);// agy : Very important ! invoke this method BEFORE activation ! diff --git a/src/SALOMESDS/TestSalomeSDS.py b/src/SALOMESDS/TestSalomeSDS.py index ceef5e77d..bdbdc6761 100644 --- a/src/SALOMESDS/TestSalomeSDS.py +++ b/src/SALOMESDS/TestSalomeSDS.py @@ -123,7 +123,8 @@ class SalomeSDSTest(unittest.TestCase): # self.assertEqual(str2Obj(dss.fetchSerializedContent(varName)),{'ab':[4,5,6],'cd':[7,8,9,10]}) wk=dss.waitForKeyInVar(varName,obj2Str("cd")) - self.assertEqual(str2Obj(wk.waitFor()),[7,8,9,10]) + wk.waitFor() + self.assertEqual(str2Obj(dss.waitForMonoThrRev(wk)),[7,8,9,10]) # nbProc=8 pool=mp.Pool(processes=nbProc) @@ -152,7 +153,8 @@ class SalomeSDSTest(unittest.TestCase): # self.assertEqual(str2Obj(dss.fetchSerializedContent(varName)),{'ab':[4,5,6],'cd':[7,8,9,10]}) wk=dss.waitForKeyInVar(varName,obj2Str("cd")) - self.assertEqual(str2Obj(wk.waitFor()),[7,8,9,10]) + wk.waitFor() + self.assertEqual(str2Obj(dss.waitForMonoThrRev(wk)),[7,8,9,10]) def testTransaction3(self): scopeName="Scope1" @@ -195,7 +197,8 @@ class SalomeSDSTest(unittest.TestCase): self.assertEqual(str2Obj(dss.fetchSerializedContent(varName)),{'ab':[4,5,6],'cd':[7,8,9,10]}) wk,t2=dss.waitForKeyInVarAndKillIt(varName,obj2Str("cd")) self.assertEqual(str2Obj(dss.fetchSerializedContent(varName)),{'ab':[4,5,6],'cd':[7,8,9,10]}) - self.assertEqual(str2Obj(wk.waitFor()),[7,8,9,10]) + wk.waitFor() + self.assertEqual(str2Obj(dss.waitForMonoThrRev(wk)),[7,8,9,10]) dss.atomicApply([t2]) self.assertEqual(str2Obj(dss.fetchSerializedContent(varName)),{'ab':[4,5,6]}) @@ -220,14 +223,16 @@ class SalomeSDSTest(unittest.TestCase): self.assertEqual(str2Obj(dss.fetchSerializedContent(varName)),{'ab':[4,5,6]}) wk=dss.waitForKeyInVar(varName,obj2Str("cd")) t1.addKeyValueInVarErrorIfAlreadyExistingNow(obj2Str("cd"),obj2Str([7,8,9,10])) - self.assertEqual(str2Obj(wk.waitFor()),[7,8,9,10]) + wk.waitFor() + self.assertEqual(str2Obj(dss.waitForMonoThrRev(wk)),[7,8,9,10]) self.assertEqual(str2Obj(dss.fetchSerializedContent(varName)),{'ab':[4,5,6]})# it is not a bug ! commit of t1 not done ! dss.atomicApply([t1]) self.assertEqual(dss.getAccessOfVar(varName),"RdExt") # self.assertEqual(str2Obj(dss.fetchSerializedContent(varName)),{'ab':[4,5,6],'cd':[7,8,9,10]}) wk=dss.waitForKeyInVar(varName,obj2Str("cd")) - self.assertEqual(str2Obj(wk.waitFor()),[7,8,9,10]) + wk.waitFor() + self.assertEqual(str2Obj(dss.waitForMonoThrRev(wk)),[7,8,9,10]) keys=[str2Obj(elt) for elt in dss.getAllKeysOfVarWithTypeDict(varName)] self.assertEqual(keys,['ab','cd']) diff --git a/src/SALOMESDS/TestSalomeSDSHelper0.py b/src/SALOMESDS/TestSalomeSDSHelper0.py index fc3fd0f3d..dd0c66ac5 100644 --- a/src/SALOMESDS/TestSalomeSDSHelper0.py +++ b/src/SALOMESDS/TestSalomeSDSHelper0.py @@ -19,7 +19,8 @@ def waitKey(): dss,isCreated=dsm.giveADataScopeTransactionCalled(scopeName) assert(not isCreated) wk=dss.waitForKeyInVar(varName,obj2Str("ef")) - return str2Obj(wk.waitFor())==[11,14,100] + wk.waitFor() + return str2Obj(dss.waitForMonoThrRev(wk))==[11,14,100] if __name__=="__main__": sys.exit(not int(waitKey())) -- 2.39.2