Fix a race condition in SwappedCtx parallel exec

The protection in set_maestro was failing to detect between the real
maestro and the working threads of the parmal. This is now fixed
according to helgrind.

Also, refactor things a bit by moving this set_maestro call directly
into the SwappedCtx constructor. Much less of a shotgun surgery :)

diff --git a/src/kernel/context/ContextBoost.cpp b/src/kernel/context/ContextBoost.cpp
index 3fb9768..ac88b68 100644 (file)
--- a/src/kernel/context/ContextBoost.cpp
+++ b/src/kernel/context/ContextBoost.cpp
@@ -45,7 +45,6 @@ BoostContext::BoostContext(std::function<void()> code, void_pfn_smxprocess_t cle
 #endif
 
   } else {
 #endif
 
   } else {

-    set_maestro(this); // save maestro for run_all()

 #if BOOST_VERSION < 105600
     this->fc_ = new boost::context::fcontext_t();
 #endif
 #if BOOST_VERSION < 105600
     this->fc_ = new boost::context::fcontext_t();
 #endif

diff --git a/src/kernel/context/ContextRaw.cpp b/src/kernel/context/ContextRaw.cpp
index d8dab5a..3b29062 100644 (file)
--- a/src/kernel/context/ContextRaw.cpp
+++ b/src/kernel/context/ContextRaw.cpp
@@ -206,7 +206,6 @@ RawContext::RawContext(std::function<void()> code, void_pfn_smxprocess_t cleanup
 #endif
      this->stack_top_ = raw_makecontext(get_stack(), smx_context_usable_stack_size, RawContext::wrapper, this);
    } else {
 #endif
      this->stack_top_ = raw_makecontext(get_stack(), smx_context_usable_stack_size, RawContext::wrapper, this);
    } else {

-     set_maestro(this); // save maestro for run_all()

      if (MC_is_active())
        MC_ignore_heap(&stack_top_, sizeof(stack_top_));
    }
      if (MC_is_active())
        MC_ignore_heap(&stack_top_, sizeof(stack_top_));
    }

diff --git a/src/kernel/context/ContextSwapped.cpp b/src/kernel/context/ContextSwapped.cpp
index b550205..1d18cc5 100644 (file)
--- a/src/kernel/context/ContextSwapped.cpp
+++ b/src/kernel/context/ContextSwapped.cpp
@@ -55,6 +55,10 @@ SwappedContext::SwappedContext(std::function<void()> code, void_pfn_smxprocess_t
                                SwappedContextFactory* factory)
     : Context(std::move(code), cleanup_func, process), factory_(factory)
 {
                                SwappedContextFactory* factory)
     : Context(std::move(code), cleanup_func, process), factory_(factory)
 {

+  // Save maestro (=context created first) in preparation for run_all
+  if (factory_->workers_context_[0] == nullptr)
+    factory_->workers_context_[0] = this;
+

   if (has_code()) {
     if (smx_context_guard_size > 0 && not MC_is_active()) {
 
   if (has_code()) {
     if (smx_context_guard_size > 0 && not MC_is_active()) {
 


@@ -134,12 +138,6 @@ SwappedContext::~SwappedContext()
   xbt_free(stack_);
 }
 
   xbt_free(stack_);
 }
 

-void SwappedContext::set_maestro(SwappedContext* ctx)
-{
-  if (factory_->threads_working_ == 0) // Don't save the soul of minions, only the one of maestro
-    factory_->workers_context_[0] = ctx;
-}
-

 void* SwappedContext::get_stack()
 {
   return stack_;
 void* SwappedContext::get_stack()
 {
   return stack_;

diff --git a/src/kernel/context/ContextSwapped.hpp b/src/kernel/context/ContextSwapped.hpp
index 8ff75d5..77bbbf1 100644 (file)
--- a/src/kernel/context/ContextSwapped.hpp
+++ b/src/kernel/context/ContextSwapped.hpp
@@ -45,7 +45,6 @@ public:
 
   virtual void swap_into(SwappedContext* to) = 0; // Defined in Raw, Boost and UContext subclasses
 
 
   virtual void swap_into(SwappedContext* to) = 0; // Defined in Raw, Boost and UContext subclasses
 

-  void set_maestro(SwappedContext* ctx);

   void* get_stack();
 
   // FIXME: Killme
   void* get_stack();
 
   // FIXME: Killme

diff --git a/src/kernel/context/ContextUnix.cpp b/src/kernel/context/ContextUnix.cpp
index e2906a5..1b0398e 100644 (file)
--- a/src/kernel/context/ContextUnix.cpp
+++ b/src/kernel/context/ContextUnix.cpp
@@ -54,8 +54,6 @@ UContext::UContext(std::function<void()> code, void_pfn_smxprocess_t cleanup_fun
     ASAN_ONLY(this->asan_stack_ = get_stack());
 #endif
     UContext::make_ctx(&this->uc_, UContext::smx_ctx_sysv_wrapper, this);
     ASAN_ONLY(this->asan_stack_ = get_stack());
 #endif
     UContext::make_ctx(&this->uc_, UContext::smx_ctx_sysv_wrapper, this);

-  } else {
-    set_maestro(this); // save maestro for run_all()

   }
 
 #if SIMGRID_HAVE_MC
   }
 
 #if SIMGRID_HAVE_MC