Make sthread_inside_simgrid static into libsthread in the (vain) hope that it'll...

but unfortunately the symptoms are exactly the same:
- default builds work well with sthread LD_PRELOAD'ed
- MC builds segfault once the second thread starts in this case
- ns-3 builds deadlock in dl_init with sthread

I'm still commiting this because I believe that breaking the hard
dependency of sthread -> simgrid goes in the right direction: If
sthread is alone in memory without simgrid (as it may happen at
init/fini phases), it's a rather bad idea for sthread to use a
variable that lives in simgrid symbols.

Maybe I should load simgrid dynamically with libdl instead of
depending on it to further untighten the dependency, but I'd prefer to
understand. Plus, it'd only help at init phase and ns-3 builds are not
the ones I'd prefer to fix here.

diff --git a/examples/sthread/sthread-mutex-simple.c b/examples/sthread/sthread-mutex-simple.c
index efbedd3..213fca8 100644 (file)
--- a/examples/sthread/sthread-mutex-simple.c
+++ b/examples/sthread/sthread-mutex-simple.c
@@ -22,7 +22,6 @@ static void* thread2_fun(void* ignore)
 
 int main(int argc, char* argv[])
 {
-  sthread_inside_simgrid = 1;
   sthread_mutex_init(&mutex, NULL);
 
   sthread_t thread1, thread2;

diff --git a/src/kernel/context/Context.cpp b/src/kernel/context/Context.cpp
index f25b980..6827ff9 100644 (file)
--- a/src/kernel/context/Context.cpp
+++ b/src/kernel/context/Context.cpp
@@ -9,7 +9,7 @@
 #include "simgrid/s4u/Host.hpp"
 #include "src/kernel/activity/CommImpl.hpp"
 #include "src/kernel/context/Context.hpp"
-#include "src/sthread/sthread.h" // sthread_inside_simgrid
+#include "src/sthread/sthread.h"
 #include "src/surf/surf_interface.hpp"
 
 #include <vector>
@@ -140,7 +140,7 @@ Context::~Context()
 void Context::stop()
 {
   this->actor_->cleanup_from_self();
-  sthread_inside_simgrid = 1;
+  sthread_disable();
   throw ForcefulKillException(); // clean RAII variables with the dedicated exception
 }
 AttachContext::~AttachContext() = default;

diff --git a/src/kernel/context/ContextSwapped.cpp b/src/kernel/context/ContextSwapped.cpp
index eb4e41b..599d52a 100644 (file)
--- a/src/kernel/context/ContextSwapped.cpp
+++ b/src/kernel/context/ContextSwapped.cpp
@@ -8,7 +8,7 @@
 #include "src/internal_config.h"
 #include "src/kernel/EngineImpl.hpp"
 #include "src/kernel/actor/ActorImpl.hpp"
-#include "src/sthread/sthread.h" // sthread_inside_simgrid
+#include "src/sthread/sthread.h"
 #include "xbt/parmap.hpp"
 
 #include "src/kernel/context/ContextSwapped.hpp"
@@ -49,15 +49,15 @@ void smx_ctx_wrapper(simgrid::kernel::context::SwappedContext* context)
   __sanitizer_finish_switch_fiber(nullptr, &context->asan_ctx_->asan_stack_, &context->asan_ctx_->asan_stack_size_);
 #endif
   try {
-    sthread_inside_simgrid = 0;
+    sthread_enable();
     (*context)();
-    sthread_inside_simgrid = 1;
+    sthread_disable();
     context->stop();
   } catch (simgrid::ForcefulKillException const&) {
-    sthread_inside_simgrid = 1;
+    sthread_disable();
     XBT_DEBUG("Caught a ForcefulKillException");
   } catch (simgrid::Exception const& e) {
-    sthread_inside_simgrid = 1;
+    sthread_disable();
     XBT_INFO("Actor killed by an uncaught exception %s", boost::core::demangle(typeid(e).name()).c_str());
     throw;
   }
@@ -249,7 +249,7 @@ void SwappedContext::resume()
     // Save my current soul (either maestro, or one of the minions) in a thread-specific area
     worker_context_ = old;
   }
-  sthread_inside_simgrid = 0;
+  sthread_enable();
   // Switch my soul and the actor's one
   Context::set_current(this);
   old->swap_into(this);
@@ -291,12 +291,12 @@ void SwappedContext::suspend()
     if (i < engine->get_actor_to_run_count()) {
       /* Actually swap into the next actor directly without transiting to maestro */
       XBT_DEBUG("Run next actor");
-      sthread_inside_simgrid = 0;
+      sthread_enable();
       next_context = static_cast<SwappedContext*>(engine->get_actor_to_run_at(i)->context_.get());
     } else {
       /* all processes were run, actually return to maestro */
       XBT_DEBUG("No more actors to run");
-      sthread_inside_simgrid = 1;
+      sthread_disable();
       next_context = factory_.maestro_context_;
     }
   }

diff --git a/src/sthread/sthread.c b/src/sthread/sthread.c
index 7724bcf..ef1259c 100644 (file)
--- a/src/sthread/sthread.c
+++ b/src/sthread/sthread.c
@@ -35,6 +35,15 @@ static void intercepter_init()
   raw_sem_post = (int (*)(sem_t*))dlsym(RTLD_NEXT, "sem_post");
 }
 
+static int sthread_inside_simgrid = 1;
+void sthread_enable(void)
+{ // Start intercepting all pthread calls
+  sthread_inside_simgrid = 0;
+}
+void sthread_disable(void)
+{ // Stop intercepting all pthread calls
+  sthread_inside_simgrid = 1;
+}
 int pthread_create(pthread_t* thread, const pthread_attr_t* attr, void* (*start_routine)(void*), void* arg)
 {
   if (raw_pthread_create == NULL)

diff --git a/src/sthread/sthread.h b/src/sthread/sthread.h
index 233dbb1..9bc6c50 100644 (file)
--- a/src/sthread/sthread.h
+++ b/src/sthread/sthread.h
@@ -7,12 +7,23 @@
  *
  * The sthread_* symbols are those actual implementations, used in the pthread_* redefinitions. */
 
+#ifndef SIMGRID_STHREAD_H
+#define SIMGRID_STHREAD_H
+
+#if defined(__ELF__)
+#define XBT_PUBLIC __attribute__((visibility("default")))
+#else
+#define XBT_PUBLIC
+#endif
+
 #if defined(__cplusplus)
 extern "C" {
 #endif
-extern volatile int sthread_inside_simgrid; // Only intercept pthread calls in user code
 
+// Launch the simulation. The old main function (passed as a parameter) is launched as an actor
 int sthread_main(int argc, char** argv, char** envp, int (*raw_main)(int, char**, char**));
+XBT_PUBLIC void sthread_enable(void);  // Start intercepting all pthread calls
+XBT_PUBLIC void sthread_disable(void); // Stop intercepting all pthread calls
 
 typedef unsigned long int sthread_t;
 int sthread_create(sthread_t* thread, const /*pthread_attr_t*/ void* attr, void* (*start_routine)(void*), void* arg);
@@ -30,3 +41,5 @@ int sthread_mutex_destroy(sthread_mutex_t* mutex);
 #if defined(__cplusplus)
 }
 #endif
+
+#endif
\ No newline at end of file

diff --git a/src/sthread/sthread_impl.cpp b/src/sthread/sthread_impl.cpp
index f2afe1e..19a19b8 100644 (file)
--- a/src/sthread/sthread_impl.cpp
+++ b/src/sthread/sthread_impl.cpp
@@ -38,7 +38,7 @@ int sthread_main(int argc, char** argv, char** envp, int (*raw_main)(int, char**
   zone->seal();
 
   /* Launch the user's main() on an actor */
-  sthread_inside_simgrid   = 0;
+  sthread_enable();
   sg4::ActorPtr main_actor = sg4::Actor::create("tid 0", lilibeth, raw_main, argc, argv, envp);
 
   XBT_INFO("sthread main() is launching the simulation");
@@ -46,6 +46,7 @@ int sthread_main(int argc, char** argv, char** envp, int (*raw_main)(int, char**
 
   return 0;
 }
+
 struct sthread_mutex {
   s4u_Mutex* mutex;
 };
@@ -56,17 +57,17 @@ static void thread_create_wrapper(void* (*user_function)(void*), void* param)
   if (SMPI_is_inited())
     SMPI_thread_create();
 #endif
-  sthread_inside_simgrid = 0;
+  sthread_enable();
   user_function(param);
-  sthread_inside_simgrid = 1;
+  sthread_disable();
 }
 
 int sthread_create(unsigned long int* thread, const /*pthread_attr_t*/ void* attr, void* (*start_routine)(void*),
                    void* arg)
 {
-  static int TID = 1;
-
+  static int TID = 0;
   TID++;
+  XBT_INFO("Create thread %d", TID);
   int rank = 0;
 #if HAVE_SMPI
   if (SMPI_is_inited())

diff --git a/src/xbt/xbt_main.cpp b/src/xbt/xbt_main.cpp
index 6086dad..9443d05 100644 (file)
--- a/src/xbt/xbt_main.cpp
+++ b/src/xbt/xbt_main.cpp
@@ -41,7 +41,6 @@ std::string binary_name;          /* Name of the system process containing us (m
 std::vector<std::string> cmdline; /* all we got in argv */
 } // namespace simgrid::xbt
 
-volatile int sthread_inside_simgrid = 1; // Only intercept pthread calls in user code.
 
 int xbt_initialized = 0;
 simgrid::config::Flag<bool> cfg_dbg_clean_atexit{
@@ -57,6 +56,8 @@ int xbt_pagebits = 0;
  */
 static void xbt_preinit() XBT_ATTRIB_CONSTRUCTOR(200);
 static void xbt_postexit();
+void sthread_enable() {}  // These symbols are used from ContextSwapped in any case, but they are only useful
+void sthread_disable() {} //  when libsthread is LD_PRELOADED. In this case, sthread's implem gets used instead.
 
 #ifdef _WIN32
 #include <windows.h>