Make all global data-structures thread-safe

[GoogleCodeExporter]

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

1. Debug::allMsgs() returns a reference to a static var

2. DistanceGraph.hh:344
static Int markGlobal

3. EquivalenceClassCollection:129 
static int s_nextCycle;
static int s_nextGraph;

4. Error.hh:531
static std::ostream *s_os;

5. FlawHandler.hh:203
static TiXmlElement* s_element; /*!< Temporary holder for copied elements */

6. Interpreter.hh:338
static unsigned int s_counter;

7. StringErrorStream.hh:39
static std::stringstream stringStream;

8. Variable.hh:157
static DomainType* sl_emptyDomain = 0;

9. DomainComparator.hh:94
static DomainComparator* s_instance; 

10. NddlRules.cc:19
static std::list<edouble> sl_values;

11. ModulePlanDatabase.cc
  static bool & planDatabaseInitialized() {
    static bool sl_alreadyDone(false);
    return sl_alreadyDone;
  }

12. Filters.cc:163
    IntervalIntDomain& HorizonFilter::getHorizon() {
      static IntervalIntDomain sl_instance;
      return sl_instance;
    }

13. 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.

14. IdTable.cc 
15. LabelStr.cc

16. Pdlfcn.cc : 147
static char buffer[256];

Original issue reported on code.google.com by javier.barreiro@gmail.com on 13 May 2011 at 12:26

[GoogleCodeExporter]

Original comment by javier.barreiro@gmail.com on 13 May 2011 at 12:28

• Added labels: Milestone-EUROPA-2.6, OpSys-All, Priority-Critical
• Removed labels: Priority-Medium

[GoogleCodeExporter]

Original comment by javier.barreiro@gmail.com on 13 May 2011 at 12:31

[GoogleCodeExporter]

Original comment by javier.barreiro@gmail.com on 30 May 2012 at 5:26

• Added labels: Milestone-EUROPA-2.7
• Removed labels: Milestone-EUROPA-2.6

[GoogleCodeExporter]

For 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.

1. Used the above pattern.
2. Unfortunately markGlobal is used in a lot of weird places.  Need to think 
about this some more.
3. Made these statics into members of EquivalenceClassCollection.
4. 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.
5. 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.
6. 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.
7. Added a simple mutex.
8. 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.
9. 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.
10. 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.
11. planDatabaseInitialized appears to do nothing.  Removed.
12. This is issue 101.
13. Used the static-wrapped-in-a-class pattern.
14. Looks good.  I'm hoping to get rid of Id and IdTable.
15. Looks good.  Again, kinda hoping to get rid of LabelStr.
16. 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.

Original comment by miata...@gmail.com on 19 Aug 2014 at 10:23

• Changed state: Fixed