europa doesn't provide built-in multi-thread support. what we offer is a guarantee that a PSEngine is self-contained, if a europa user wants to embed europa in a multi-threaded application, she only needs to worry about ensuring thread-safe access to each individual PSEngine instance.
we need to make sure that guarantee is good. There is still seems to be a number of global (within a process), mutable variables that would make that guarantee void (multiple-threads causing those variables to be modified would lead to unpredictable results).
a search for the keyword "static" on all the .hh and .cc files yields the following suspects :
the first ones we should investigate in my opinion are : Entity,IdTable,LabelStr,DomainComparator
Debug::allMsgs() returns a reference to a static var
DistanceGraph.hh:344 static Int markGlobal
EquivalenceClassCollection:129 static int s_nextCycle; static int s_nextGraph;
Error.hh:531 static std::ostream *s_os;
FlawHandler.hh:203 static TiXmlElement* s_element; /*!< Temporary holder for copied elements */
Interpreter.hh:338 static unsigned int s_counter;
StringErrorStream.hh:39 static std::stringstream stringStream;
Variable.hh:157 static DomainType* sl_emptyDomain = 0;
DomainComparator.hh:94 static DomainComparator* s_instance;
NddlRules.cc:19 static std::list<edouble> sl_values;
ModulePlanDatabase.cc static bool & planDatabaseInitialized() { static bool sl_alreadyDone(false); return sl_alreadyDone; }
Filters.cc:163 IntervalIntDomain& HorizonFilter::getHorizon() { static IntervalIntDomain sl_instance; return sl_instance; }
Entity.cc:several entries, just look for "static" in the file one that looks specially worrysome : line 117 std::map<eint, unsigned long int>& Entity::entitiesByKey(){ static std::map<eint, unsigned long int> sl_entitiesByKey; return sl_entitiesByKey; }
IdTable and LableStr were the only 2 classes where I explicitly added mutexes a long time ago, they could use a review and some automated testing though.
- IdTable.cc
LabelStr.cc
Pdlfcn.cc : 147 static char buffer[256];
Comment #1
Posted on May 13, 2011 by Happy Ox(No comment was entered for this change.)
Comment #2
Posted on May 13, 2011 by Happy Ox(No comment was entered for this change.)
Comment #3
Posted on May 30, 2012 by Happy Ox(No comment was entered for this change.)
Comment #4
Posted on Aug 19, 2014 by Swift HippoFor cases where static data needs preservation, I'm using a pattern where static data is encapsulated in a class, there's one static instance of that class, and access to it is achieved by a function that returns a pair of a MutexGrabber and the instance, so that a lock is guaranteed to persist as long as reference to the instance is possible. This encapsulation should help future moves away from static data entirely.
- Used the above pattern.
- Unfortunately markGlobal is used in a lot of weird places. Need to think about this some more.
- Made these statics into members of EquivalenceClassCollection.
- The static flags I'm okay with being unprotected, since they control very small pieces of code and changes mid-way won't have much effect. Added a mutex for outputting to the stream, however.
- s_element is being used to store a munged copy of the incoming XML config so that it can be used in other initializers in the FlawHandler constructor after delegating to the MatchingRule constructor. However, the MatchingRule doesn't use any of the munging done by FlawHandler::makeConfigData, so it really needn't be done that way. I'm removing s_element and calling makeConfigData only in the initialization of m_configData.
- This is a counter used to create internal variable names. The worst that could happen here is if two CExprs got constructed in lock-step and ended up with the same value for m_counter, resulting in later calls to createVariableName() returning the same name, which seems not awful to me.
- Added a simple mutex.
- In this case, a static version of each domain type is created, emptied, and returned in place of the actual derived domain if the CE is proven inconsistent. I don't think always returning empty is the right thing to do, so that needs examination, but in the short-term, I've addressed this issue by getting rid of the static data and instead emptying the derived domain and returning that when the CE is proven inconsistent.
- The DomainComparator is there to provide enhanced comparison semantics, but I think it's taking the wrong approach and so I've removed the static data there entirely. Who defines comparability and where it's checkable needs thought.
- This code seems to antedate the Named Return Value Optimization, so it keeps a static local variable and returns a reference to it to "avoid copying". I've removed the staticness and it now returns a list.
- planDatabaseInitialized appears to do nothing. Removed.
- This is issue 101.
- Used the static-wrapped-in-a-class pattern.
- Looks good. I'm hoping to get rid of Id and IdTable.
- Looks good. Again, kinda hoping to get rid of LabelStr.
- The static data here is function-local buffers for error strings in the dl* functions. I'm okay with these being un-protected, since dlopen isn't something that gets done a whole lot in threads. Also, it's not clear that this needs to persist, since both OS X and MinGW have progressed and may have viable dl* implementations.
Addressed in r6742.
Status: Fixed
Labels:
Type-Defect
Priority-Critical
OpSys-All
Milestone-EUROPA-2.7