From d7fb52aa1f18cd70a51a6d8913c1a4513fea7ce7 Mon Sep 17 00:00:00 2001
From: Martin Quinson <martin.quinson@ens-rennes.fr>
Date: Sat, 19 Jan 2019 23:18:07 +0100
Subject: [PATCH] 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 :)
---
 src/kernel/context/ContextBoost.cpp   |  1 -
 src/kernel/context/ContextRaw.cpp     |  1 -
 src/kernel/context/ContextSwapped.cpp | 10 ++++------
 src/kernel/context/ContextSwapped.hpp |  1 -
 src/kernel/context/ContextUnix.cpp    |  2 --
 5 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/src/kernel/context/ContextBoost.cpp b/src/kernel/context/ContextBoost.cpp
index 3fb97680d7..ac88b6833a 100644
--- 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 {
-    set_maestro(this); // save maestro for run_all()
 #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 d8dab5ac34..3b2906237b 100644
--- 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 {
-     set_maestro(this); // save maestro for run_all()
      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 b550205e99..1d18cc5916 100644
--- 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)
 {
+  // 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()) {
 
@@ -134,12 +138,6 @@ SwappedContext::~SwappedContext()
   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_;
diff --git a/src/kernel/context/ContextSwapped.hpp b/src/kernel/context/ContextSwapped.hpp
index 8ff75d5626..77bbbf17c5 100644
--- 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
 
-  void set_maestro(SwappedContext* ctx);
   void* get_stack();
 
   // FIXME: Killme
diff --git a/src/kernel/context/ContextUnix.cpp b/src/kernel/context/ContextUnix.cpp
index e2906a5d55..1b0398ee56 100644
--- 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);
-  } else {
-    set_maestro(this); // save maestro for run_all()
   }
 
 #if SIMGRID_HAVE_MC
-- 
2.20.1