From: Fred Suter Date: Thu, 26 Oct 2023 21:24:51 +0000 (-0400) Subject: Merge branch 'master' into mq X-Git-Tag: v3.35~89^2~28^2~1 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/eb680c004b00a59218e7cdaa77fc2c96066c3548?hp=29b2e4ac1cf7f206e82da4d1a2e100428ea4c4a0 Merge branch 'master' into mq --- diff --git a/BuildSimGrid.sh b/BuildSimGrid.sh index 66b309eb8b..410d1ddb70 100755 --- a/BuildSimGrid.sh +++ b/BuildSimGrid.sh @@ -17,6 +17,7 @@ fi target=examples ncores=$(grep -c processor /proc/cpuinfo) +halfcores=$(expr $ncores / 2 + 1) install_path=$(sed -n 's/^CMAKE_INSTALL_PREFIX:PATH=//p' CMakeCache.txt) if [ -e ${install_path} ] && [ -d ${install_path} ] && [ -x ${install_path} ] && [ -w ${install_path} ] ; then @@ -32,7 +33,8 @@ fi ( echo "install_path: ${install_path}" echo "Target: ${target}" - echo "Cores: ${ncores}" - (nice ${builder} -j${ncores} ${target} tests || make ${target} tests) && nice ctest -j${ncores} --output-on-failure ; date + echo "Cores to build: ${ncores}" + echo "Cores to test: ${halfcores}" + (nice ${builder} -j${ncores} ${target} tests || ${builder} ${target} tests) && nice ctest -j${halfcores} --output-on-failure ; date ) 2>&1 | tee BuildSimGrid.sh.log diff --git a/CMakeLists.txt b/CMakeLists.txt index 03b4b09a68..ec4f39bbd1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -968,4 +968,3 @@ execute_process(COMMAND ${CMAKE_COMMAND} -E make_directory ${PROJECT_BINARY_DIR} file(WRITE ${PROJECT_BINARY_DIR}/Testing/Notes/Build "GIT version : ${GIT_VERSION}\n") file(APPEND ${PROJECT_BINARY_DIR}/Testing/Notes/Build "Release : simgrid-${release_version}\n") -INCLUDE(Dart) diff --git a/examples/sthread/CMakeLists.txt b/examples/sthread/CMakeLists.txt index 98811c23ad..7d9d7b5bca 100644 --- a/examples/sthread/CMakeLists.txt +++ b/examples/sthread/CMakeLists.txt @@ -5,7 +5,7 @@ find_package(Threads REQUIRED) ######################################################################### foreach(x - mutex-simple + mutex-simple mutex-recursive producer-consumer) if("${CMAKE_SYSTEM}" MATCHES "Linux") diff --git a/examples/sthread/pthread-mc-mutex-recursive.tesh b/examples/sthread/pthread-mc-mutex-recursive.tesh new file mode 100644 index 0000000000..984a4b56d1 --- /dev/null +++ b/examples/sthread/pthread-mc-mutex-recursive.tesh @@ -0,0 +1,15 @@ +# We ignore the LD_PRELOAD lines from the expected output because they contain the build path +! ignore .*LD_PRELOAD.* + +$ $VALGRIND_NO_TRACE_CHILDREN ${bindir:=.}/../../bin/simgrid-mc --cfg=model-check/setenv:LD_PRELOAD=${libdir:=.}/libsgmalloc.so:${libdir:=.}/libsthread.so ${bindir:=.}/pthread-mutex-recursive +> sthread is intercepting the execution of ./pthread-mutex-recursive +> [0.000000] [mc_dfs/INFO] Start a DFS exploration. Reduction is: dpor. +> Got the lock on the default mutex. +> Failed to relock the default mutex. +> Got the lock on the recursive mutex. +> Got the lock again on the recursive mutex. +> Got the lock on the default mutex. +> Failed to relock the default mutex. +> Got the lock on the recursive mutex. +> Got the lock again on the recursive mutex. +> [0.000000] [mc_dfs/INFO] DFS exploration ended. 17 unique states visited; 1 backtracks (3 transition replays, 21 states visited overall) diff --git a/examples/sthread/pthread-mc-mutex-simple.tesh b/examples/sthread/pthread-mc-mutex-simple.tesh index 55ecc955dc..9e38ceadf3 100644 --- a/examples/sthread/pthread-mc-mutex-simple.tesh +++ b/examples/sthread/pthread-mc-mutex-simple.tesh @@ -3,7 +3,7 @@ ! ignore .*LD_PRELOAD.* $ $VALGRIND_NO_TRACE_CHILDREN ${bindir:=.}/../../bin/simgrid-mc --cfg=model-check/setenv:LD_PRELOAD=${libdir:=.}/libsgmalloc.so:${libdir:=.}/libsthread.so ${bindir:=.}/pthread-mutex-simple -> [0.000000] [sthread/INFO] Starting the simulation. +> sthread is intercepting the execution of ./pthread-mutex-simple > All threads are started. > [0.000000] [mc_dfs/INFO] Start a DFS exploration. Reduction is: dpor. > The thread 0 is terminating. diff --git a/examples/sthread/pthread-mc-mutex-simpledeadlock.tesh b/examples/sthread/pthread-mc-mutex-simpledeadlock.tesh index 2de4e42603..e843a409b9 100644 --- a/examples/sthread/pthread-mc-mutex-simpledeadlock.tesh +++ b/examples/sthread/pthread-mc-mutex-simpledeadlock.tesh @@ -6,7 +6,7 @@ ! ignore .*LD_PRELOAD.* $ $VALGRIND_NO_TRACE_CHILDREN ${bindir:=.}/../../bin/simgrid-mc --cfg=model-check/setenv:LD_PRELOAD=${libdir:=.}/libsgmalloc.so:${libdir:=.}/libsthread.so ${bindir:=.}/pthread-mutex-simpledeadlock -> [0.000000] [sthread/INFO] Starting the simulation. +> sthread is intercepting the execution of ./pthread-mutex-simpledeadlock > All threads are started. > [0.000000] [mc_dfs/INFO] Start a DFS exploration. Reduction is: dpor. > The thread 0 is terminating. diff --git a/examples/sthread/pthread-mc-producer-consumer.tesh b/examples/sthread/pthread-mc-producer-consumer.tesh index a9f5a723d7..584b7a5857 100644 --- a/examples/sthread/pthread-mc-producer-consumer.tesh +++ b/examples/sthread/pthread-mc-producer-consumer.tesh @@ -2,18 +2,18 @@ ! ignore .*LD_PRELOAD.* $ $VALGRIND_NO_TRACE_CHILDREN ${bindir:=.}/../../bin/simgrid-mc --cfg=model-check/setenv:LD_PRELOAD=${libdir:=.}/libsgmalloc.so:${libdir:=.}/libsthread.so ${bindir:=.}/pthread-producer-consumer -q -C 1 -P 1 -> [0.000000] [sthread/INFO] Starting the simulation. +> sthread is intercepting the execution of ./pthread-producer-consumer > [0.000000] [mc_dfs/INFO] Start a DFS exploration. Reduction is: dpor. > [0.000000] [mc_dfs/INFO] DFS exploration ended. 786 unique states visited; 97 backtracks (2049 transition replays, 2932 states visited overall) $ $VALGRIND_NO_TRACE_CHILDREN ${bindir:=.}/../../bin/simgrid-mc --cfg=model-check/reduction:sdpor --cfg=model-check/setenv:LD_PRELOAD=${libdir:=.}/libsgmalloc.so:${libdir:=.}/libsthread.so ${bindir:=.}/pthread-producer-consumer -q -C 1 -P 1 > [0.000000] [xbt_cfg/INFO] Configuration change: Set 'model-check/reduction' to 'sdpor' -> [0.000000] [sthread/INFO] Starting the simulation. +> sthread is intercepting the execution of ./pthread-producer-consumer > [0.000000] [mc_dfs/INFO] Start a DFS exploration. Reduction is: sdpor. > [0.000000] [mc_dfs/INFO] DFS exploration ended. 1186 unique states visited; 157 backtracks (3403 transition replays, 4746 states visited overall) $ $VALGRIND_NO_TRACE_CHILDREN ${bindir:=.}/../../bin/simgrid-mc --cfg=model-check/reduction:odpor --cfg=model-check/setenv:LD_PRELOAD=${libdir:=.}/libsgmalloc.so:${libdir:=.}/libsthread.so ${bindir:=.}/pthread-producer-consumer -q -C 1 -P 1 > [0.000000] [xbt_cfg/INFO] Configuration change: Set 'model-check/reduction' to 'odpor' -> [0.000000] [sthread/INFO] Starting the simulation. +> sthread is intercepting the execution of ./pthread-producer-consumer > [0.000000] [mc_dfs/INFO] Start a DFS exploration. Reduction is: odpor. > [0.000000] [mc_dfs/INFO] DFS exploration ended. 39 unique states visited; 0 backtracks (0 transition replays, 39 states visited overall) diff --git a/examples/sthread/pthread-mutex-recursive.tesh b/examples/sthread/pthread-mutex-recursive.tesh new file mode 100644 index 0000000000..ca535da834 --- /dev/null +++ b/examples/sthread/pthread-mutex-recursive.tesh @@ -0,0 +1,7 @@ +$ env ASAN_OPTIONS=verify_asan_link_order=0:$ASAN_OPTIONS LD_PRELOAD=${libdir:=.}/libsthread.so ./pthread-mutex-recursive +> sthread is intercepting the execution of ./pthread-mutex-recursive +> Got the lock on the default mutex. +> Failed to relock the default mutex. +> Got the lock on the recursive mutex. +> Got the lock again on the recursive mutex. +> [0.000000] [sthread/INFO] All threads exited. Terminating the simulation. diff --git a/examples/sthread/pthread-mutex-simple.c b/examples/sthread/pthread-mutex-simple.c index 1d391418ba..a7dea6bd6b 100644 --- a/examples/sthread/pthread-mutex-simple.c +++ b/examples/sthread/pthread-mutex-simple.c @@ -1,3 +1,8 @@ +/* Copyright (c) 2002-2023. The SimGrid Team. All rights reserved. */ + +/* This program is free software; you can redistribute it and/or modify it + * under the terms of the license (GNU LGPL) which comes with this package. */ + /* Simple test code with no bug */ #include diff --git a/examples/sthread/pthread-mutex-simple.tesh b/examples/sthread/pthread-mutex-simple.tesh index 29d66a92df..2e12ec1204 100644 --- a/examples/sthread/pthread-mutex-simple.tesh +++ b/examples/sthread/pthread-mutex-simple.tesh @@ -1,5 +1,5 @@ $ env ASAN_OPTIONS=verify_asan_link_order=0:$ASAN_OPTIONS LD_PRELOAD=${libdir:=.}/libsthread.so ./pthread-mutex-simple -> [0.000000] [sthread/INFO] Starting the simulation. +> sthread is intercepting the execution of ./pthread-mutex-simple > All threads are started. > The thread 0 is terminating. > The thread 1 is terminating. diff --git a/examples/sthread/pthread-mutex-simpledeadlock.c b/examples/sthread/pthread-mutex-simpledeadlock.c index 09be6c11ea..92a04a1ace 100644 --- a/examples/sthread/pthread-mutex-simpledeadlock.c +++ b/examples/sthread/pthread-mutex-simpledeadlock.c @@ -1,3 +1,8 @@ +/* Copyright (c) 2002-2023. The SimGrid Team. All rights reserved. */ + +/* This program is free software; you can redistribute it and/or modify it + * under the terms of the license (GNU LGPL) which comes with this package. */ + /* Simple test code that may deadlock: Thread 1 locks mutex1 then mutex2 while thread 2 locks in reverse order. diff --git a/examples/sthread/pthread-producer-consumer.tesh b/examples/sthread/pthread-producer-consumer.tesh index a54bed01af..d8bf22cf52 100644 --- a/examples/sthread/pthread-producer-consumer.tesh +++ b/examples/sthread/pthread-producer-consumer.tesh @@ -1,5 +1,5 @@ $ env ASAN_OPTIONS=verify_asan_link_order=0:$ASAN_OPTIONS LD_PRELOAD=${libdir:=.}/libsthread.so ./pthread-producer-consumer -> [0.000000] [sthread/INFO] Starting the simulation. +> sthread is intercepting the execution of ./pthread-producer-consumer > Producer 1: Insert Item 0 at 0 > Producer 2: Insert Item 0 at 1 > Consumer 1: Remove Item 0 from 0 @@ -15,7 +15,7 @@ $ env ASAN_OPTIONS=verify_asan_link_order=0:$ASAN_OPTIONS LD_PRELOAD=${libdir:=. > [0.000000] [sthread/INFO] All threads exited. Terminating the simulation. $ env ASAN_OPTIONS=verify_asan_link_order=0:$ASAN_OPTIONS LD_PRELOAD=${libdir:=.}/libsthread.so ./pthread-producer-consumer -c 2 -C 1 -p 2 -P 1 -> [0.000000] [sthread/INFO] Starting the simulation. +> sthread is intercepting the execution of ./pthread-producer-consumer > Producer 1: Insert Item 0 at 0 > Consumer 1: Remove Item 0 from 0 > Producer 1: Insert Item 1 at 1 diff --git a/examples/sthread/stdobject/stdobject.tesh b/examples/sthread/stdobject/stdobject.tesh index 457238ced8..bb8e1b7beb 100644 --- a/examples/sthread/stdobject/stdobject.tesh +++ b/examples/sthread/stdobject/stdobject.tesh @@ -5,7 +5,7 @@ ! ignore .*LD_PRELOAD.* $ $VALGRIND_NO_TRACE_CHILDREN ${bindir:=.}/../../../bin/simgrid-mc --cfg=model-check/setenv:LD_PRELOAD=${libdir:=.}/libsthread.so ${bindir:=.}/stdobject "--log=root.fmt:[%11.6r]%e(%a@%h)%e%m%n" --log=no_loc -> [ 0.000000] (maestro@) Starting the simulation. +> sthread is intercepting the execution of ./stdobject > starting two helpers... > waiting for helpers to finish... > [ 0.000000] (maestro@) Start a DFS exploration. Reduction is: dpor. @@ -16,7 +16,7 @@ $ $VALGRIND_NO_TRACE_CHILDREN ${bindir:=.}/../../../bin/simgrid-mc --cfg=model-c > v = { 1, 2, 3, 5, 8, 13, 21, 21, }; > [ 0.000000] (maestro@) thread 1 takes &v > [ 0.000000] (maestro@) thread 2 takes &v -> [ 0.000000] (maestro@) Unprotected concurent access to &v: thread 1 vs thread 2 (locations hidden because of --log=no_loc). +> [ 0.000000] (maestro@) Unprotected concurent access to &v: thread 1 from 1 location vs thread 2 (locations hidden because of --log=no_loc). > [ 0.000000] (maestro@) ************************** > [ 0.000000] (maestro@) *** PROPERTY NOT VALID *** > [ 0.000000] (maestro@) ************************** diff --git a/examples/sthread/sthread-mutex-simple.tesh b/examples/sthread/sthread-mutex-simple.tesh index ffa04c1dc4..c44bd9794c 100644 --- a/examples/sthread/sthread-mutex-simple.tesh +++ b/examples/sthread/sthread-mutex-simple.tesh @@ -1,5 +1,5 @@ $ ./sthread-mutex-simple -> [0.000000] [sthread/INFO] Starting the simulation. +> sthread is intercepting the execution of ./sthread-mutex-simple > All threads are started. > The thread 0 is terminating. > The thread 1 is terminating. diff --git a/include/simgrid/forward.h b/include/simgrid/forward.h index ff408d4765..ec54ec4158 100644 --- a/include/simgrid/forward.h +++ b/include/simgrid/forward.h @@ -131,6 +131,7 @@ using ActorCodeFactory = std::function args)> class Simcall; class SimcallObserver; +class MutexObserver; class ObjectAccessSimcallObserver; class ObjectAccessSimcallItem; } // namespace actor diff --git a/include/simgrid/s4u/Mutex.hpp b/include/simgrid/s4u/Mutex.hpp index 7791cd7e23..06f04c2aa5 100644 --- a/include/simgrid/s4u/Mutex.hpp +++ b/include/simgrid/s4u/Mutex.hpp @@ -12,6 +12,8 @@ namespace simgrid::s4u { /** @brief A classical mutex, but blocking in the simulation world. + * + * S4U mutexes are not recursive. If an actor tries to lock the same object twice, it deadlocks with itself. * * @beginrst * It is strictly impossible to use a real mutex, such as @@ -48,7 +50,8 @@ class XBT_PUBLIC Mutex { public: /** \static Constructs a new mutex */ - static MutexPtr create(); + static MutexPtr create(bool recursive = false); + void lock(); void unlock(); bool try_lock(); diff --git a/src/bindings/python/simgrid_python.cpp b/src/bindings/python/simgrid_python.cpp index 2f2ef70bec..53065f0a72 100644 --- a/src/bindings/python/simgrid_python.cpp +++ b/src/bindings/python/simgrid_python.cpp @@ -750,7 +750,8 @@ PYBIND11_MODULE(simgrid, m) py::class_(m, "Mutex", "A classical mutex, but blocking in the simulation world." "See the C++ documentation for details.") - .def(py::init<>(&Mutex::create), py::call_guard(), "Mutex constructor.") + .def(py::init<>(&Mutex::create), py::call_guard(), + "Mutex constructor (pass True as a parameter to get a recursive Mutex).", py::arg("recursive") = false) .def("lock", &Mutex::lock, py::call_guard(), "Block until the mutex is acquired.") .def("try_lock", &Mutex::try_lock, py::call_guard(), "Try to acquire the mutex. Return true if the mutex was acquired, false otherwise.") diff --git a/src/kernel/activity/MutexImpl.cpp b/src/kernel/activity/MutexImpl.cpp index 43a511b9d3..c981c3ee76 100644 --- a/src/kernel/activity/MutexImpl.cpp +++ b/src/kernel/activity/MutexImpl.cpp @@ -47,13 +47,41 @@ unsigned MutexImpl::next_id_ = 0; MutexAcquisitionImplPtr MutexImpl::lock_async(actor::ActorImpl* issuer) { - auto res = MutexAcquisitionImplPtr(new kernel::activity::MutexAcquisitionImpl(issuer, this), true); - - if (owner_ != nullptr) { - /* Somebody is using the mutex; register the acquisition */ + /* If the mutex is recursive */ + if (is_recursive_) { + if (owner_ == issuer) { + recursive_depth++; + auto res = MutexAcquisitionImplPtr(new kernel::activity::MutexAcquisitionImpl(issuer, this), true); + res->grant(); + return res; + } else if (owner_ == nullptr) { // Free + owner_ = issuer; + recursive_depth = 1; + auto res = MutexAcquisitionImplPtr(new kernel::activity::MutexAcquisitionImpl(issuer, this), true); + res->grant(); + return res; + } + + for (auto acq : ongoing_acquisitions_) + if (acq->get_issuer() == issuer) { + acq->recursive_depth_++; + return acq; + } + + // Not yet in the ongoing acquisition list. Get in there + auto res = MutexAcquisitionImplPtr(new kernel::activity::MutexAcquisitionImpl(issuer, this), true); ongoing_acquisitions_.push_back(res); - } else { + return res; + } + + // None-recursive mutex + auto res = MutexAcquisitionImplPtr(new kernel::activity::MutexAcquisitionImpl(issuer, this), true); + if (owner_ == nullptr) { // Lock is free, take it owner_ = issuer; + recursive_depth = 1; + res->grant(); + } else { // Somebody is using the mutex; register the acquisition + ongoing_acquisitions_.push_back(res); } return res; } @@ -65,14 +93,14 @@ MutexAcquisitionImplPtr MutexImpl::lock_async(actor::ActorImpl* issuer) */ bool MutexImpl::try_lock(actor::ActorImpl* issuer) { - XBT_IN("(%p, %p)", this, issuer); - if (owner_ != nullptr) { - XBT_OUT(); - return false; + if (owner_ == issuer && is_recursive_) { + recursive_depth++; + return true; } + if (owner_ != nullptr) + return false; - owner_ = issuer; - XBT_OUT(); + owner_ = issuer; return true; } @@ -88,12 +116,20 @@ void MutexImpl::unlock(actor::ActorImpl* issuer) xbt_assert(issuer == owner_, "Cannot release that mutex: you're not the owner. %s is (pid:%ld).", owner_ != nullptr ? owner_->get_cname() : "(nobody)", owner_ != nullptr ? owner_->get_pid() : -1); + if (is_recursive_) { + recursive_depth--; + if (recursive_depth > 0) // Still owning the lock + return; + } + if (not ongoing_acquisitions_.empty()) { /* Give the ownership to the first waiting actor */ auto acq = ongoing_acquisitions_.front(); ongoing_acquisitions_.pop_front(); owner_ = acq->get_issuer(); + acq->grant(); + recursive_depth = acq->recursive_depth_; if (acq == owner_->waiting_synchro_) acq->finish(); // else, the issuer is not blocked on this acquisition so no need to release it diff --git a/src/kernel/activity/MutexImpl.hpp b/src/kernel/activity/MutexImpl.hpp index 3eebec2b12..9d9242b4a5 100644 --- a/src/kernel/activity/MutexImpl.hpp +++ b/src/kernel/activity/MutexImpl.hpp @@ -42,11 +42,18 @@ namespace simgrid::kernel::activity { class XBT_PUBLIC MutexAcquisitionImpl : public ActivityImpl_T { actor::ActorImpl* issuer_ = nullptr; MutexImpl* mutex_ = nullptr; + int recursive_depth_ = 1; + // TODO: use granted_ this instead of owner_ == self to test(). + // This is mandatory to get double-lock on non-recursive locks to properly deadlock + bool granted_ = false; + + friend MutexImpl; public: MutexAcquisitionImpl(actor::ActorImpl* issuer, MutexImpl* mutex) : issuer_(issuer), mutex_(mutex) {} MutexImplPtr get_mutex() { return mutex_; } actor::ActorImpl* get_issuer() { return issuer_; } + void grant() { granted_ = true; } bool test(actor::ActorImpl* issuer = nullptr) override; void wait_for(actor::ActorImpl* issuer, double timeout) override; @@ -63,11 +70,13 @@ class XBT_PUBLIC MutexImpl { std::deque ongoing_acquisitions_; static unsigned next_id_; unsigned id_ = next_id_++; + bool is_recursive_ = false; + int recursive_depth = 0; friend MutexAcquisitionImpl; public: - MutexImpl() : piface_(this) {} + MutexImpl(bool recursive = false) : piface_(this), is_recursive_(recursive) {} MutexImpl(MutexImpl const&) = delete; MutexImpl& operator=(MutexImpl const&) = delete; diff --git a/src/s4u/s4u_Mutex.cpp b/src/s4u/s4u_Mutex.cpp index 051f11c153..12fa915d5b 100644 --- a/src/s4u/s4u_Mutex.cpp +++ b/src/s4u/s4u_Mutex.cpp @@ -55,9 +55,9 @@ bool Mutex::try_lock() * * See @ref s4u_raii. */ -MutexPtr Mutex::create() +MutexPtr Mutex::create(bool recursive) { - auto* mutex = new kernel::activity::MutexImpl(); + auto* mutex = new kernel::activity::MutexImpl(recursive); return MutexPtr(&mutex->mutex(), false); } diff --git a/src/simgrid/sg_config.hpp b/src/simgrid/sg_config.hpp index 10dbe7baa7..25e4a168b2 100644 --- a/src/simgrid/sg_config.hpp +++ b/src/simgrid/sg_config.hpp @@ -10,7 +10,7 @@ /** Config Globals */ -XBT_PUBLIC_DATA int _sg_cfg_init_status; +XBT_PUBLIC_DATA int _sg_cfg_init_status; /* 0: not inited; 1: config module inited; 2: root zone of platform created */ XBT_PUBLIC void sg_config_init(int* argc, char** argv); XBT_PUBLIC void sg_config_finalize(); diff --git a/src/sthread/ObjectAccess.cpp b/src/sthread/ObjectAccess.cpp index ea12a53ab3..25e9b52c32 100644 --- a/src/sthread/ObjectAccess.cpp +++ b/src/sthread/ObjectAccess.cpp @@ -51,6 +51,7 @@ struct ObjectOwner { simgrid::kernel::actor::ActorImpl* owner = nullptr; const char* file = nullptr; int line = -1; + int recursive_depth = 0; explicit ObjectOwner(simgrid::kernel::actor::ActorImpl* o) : owner(o) {} }; @@ -83,10 +84,23 @@ int sthread_access_begin(void* objaddr, const char* objname, const char* file, i [self, objaddr, objname, file, line]() -> bool { XBT_INFO("%s takes %s", self->get_cname(), objname); auto* ownership = get_owner(objaddr); - if (ownership->owner != nullptr) { + if (ownership->owner == self) { + ownership->recursive_depth++; + return true; + } else if (ownership->owner != nullptr) { auto msg = std::string("Unprotected concurent access to ") + objname + ": " + ownership->owner->get_name(); - if (not xbt_log_no_loc) + if (not xbt_log_no_loc) { msg += simgrid::xbt::string_printf(" at %s:%d", ownership->file, ownership->line); + if (ownership->recursive_depth > 1) { + msg += simgrid::xbt::string_printf(" (and %d other locations)", ownership->recursive_depth - 1); + if (ownership->recursive_depth != 2) + msg += "s"; + } + } else { + msg += simgrid::xbt::string_printf(" from %d location", ownership->recursive_depth); + if (ownership->recursive_depth != 1) + msg += "s"; + } msg += " vs " + self->get_name(); if (xbt_log_no_loc) msg += std::string(" (locations hidden because of --log=no_loc)."); @@ -98,6 +112,7 @@ int sthread_access_begin(void* objaddr, const char* objname, const char* file, i ownership->owner = self; ownership->file = file; ownership->line = line; + ownership->recursive_depth = 1; return true; }, &observer); @@ -116,9 +131,12 @@ void sthread_access_end(void* objaddr, const char* objname, const char* file, in [self, objaddr, objname]() -> void { XBT_INFO("%s releases %s", self->get_cname(), objname); auto* ownership = get_owner(objaddr); - xbt_assert(ownership->owner == self, "safety check failed: %s is not owner of the object it's releasing.", - self->get_cname()); - ownership->owner = nullptr; + xbt_assert(ownership->owner == self, + "safety check failed: %s is not owner of the object it's releasing. That object owned by %s.", + self->get_cname(), (ownership->owner == nullptr ? "nobody" : ownership->owner->get_cname())); + ownership->recursive_depth--; + if (ownership->recursive_depth == 0) + ownership->owner = nullptr; }, &observer); sthread_enable(); diff --git a/src/sthread/sthread.c b/src/sthread/sthread.c index ee9e0847ea..2005f297ad 100644 --- a/src/sthread/sthread.c +++ b/src/sthread/sthread.c @@ -28,6 +28,12 @@ static int (*raw_mutex_trylock)(pthread_mutex_t*); static int (*raw_mutex_unlock)(pthread_mutex_t*); static int (*raw_mutex_destroy)(pthread_mutex_t*); +static int (*raw_pthread_mutexattr_init)(pthread_mutexattr_t*); +static int (*raw_pthread_mutexattr_settype)(pthread_mutexattr_t*, int); +static int (*raw_pthread_mutexattr_gettype)(const pthread_mutexattr_t* restrict, int* restrict); +static int (*raw_pthread_mutexattr_getrobust)(const pthread_mutexattr_t*, int*); +static int (*raw_pthread_mutexattr_setrobust)(pthread_mutexattr_t*, int); + static unsigned int (*raw_sleep)(unsigned int); static int (*raw_usleep)(useconds_t); static int (*raw_gettimeofday)(struct timeval*, void*); @@ -50,6 +56,12 @@ static void intercepter_init() raw_mutex_unlock = dlsym(RTLD_NEXT, "pthread_mutex_unlock"); raw_mutex_destroy = dlsym(RTLD_NEXT, "pthread_mutex_destroy"); + raw_pthread_mutexattr_init = dlsym(RTLD_NEXT, "pthread_mutexattr_init"); + raw_pthread_mutexattr_settype = dlsym(RTLD_NEXT, "pthread_mutexattr_settype"); + raw_pthread_mutexattr_gettype = dlsym(RTLD_NEXT, "pthread_mutexattr_gettype"); + raw_pthread_mutexattr_getrobust = dlsym(RTLD_NEXT, "pthread_mutexattr_getrobust"); + raw_pthread_mutexattr_setrobust = dlsym(RTLD_NEXT, "pthread_mutexattr_setrobust"); + raw_sleep = dlsym(RTLD_NEXT, "sleep"); raw_usleep = dlsym(RTLD_NEXT, "usleep"); raw_gettimeofday = dlsym(RTLD_NEXT, "gettimeofday"); @@ -85,6 +97,32 @@ int pthread_create(pthread_t* thread, const pthread_attr_t* attr, void* (*start_ sthread_enable(); return res; } + +#define _STHREAD_CONCAT(a, b) a##b +#define intercepted_call(name, raw_params, call_params, sim_params) \ + int _STHREAD_CONCAT(pthread_, name) raw_params \ + { \ + if (_STHREAD_CONCAT(raw_pthread_, name) == NULL) \ + intercepter_init(); \ + if (sthread_inside_simgrid) \ + return _STHREAD_CONCAT(raw_pthread_, name) call_params; \ + \ + sthread_disable(); \ + int res = _STHREAD_CONCAT(sthread_, name) sim_params; \ + sthread_enable(); \ + return res; \ + } + +intercepted_call(mutexattr_init, (pthread_mutexattr_t * attr), (attr), ((sthread_mutexattr_t*)attr)); +intercepted_call(mutexattr_settype, (pthread_mutexattr_t * attr, int type), (attr, type), + ((sthread_mutexattr_t*)attr, type)); +intercepted_call(mutexattr_gettype, (const pthread_mutexattr_t* restrict attr, int* type), (attr, type), + ((sthread_mutexattr_t*)attr, type)); +intercepted_call(mutexattr_setrobust, (pthread_mutexattr_t* restrict attr, int robustness), (attr, robustness), + ((sthread_mutexattr_t*)attr, robustness)); +intercepted_call(mutexattr_getrobust, (const pthread_mutexattr_t* restrict attr, int* restrict robustness), + (attr, robustness), ((sthread_mutexattr_t*)attr, robustness)); + int pthread_join(pthread_t thread, void** retval) { if (raw_pthread_join == NULL) @@ -107,7 +145,7 @@ int pthread_mutex_init(pthread_mutex_t* mutex, const pthread_mutexattr_t* attr) return raw_mutex_init(mutex, attr); sthread_disable(); - int res = sthread_mutex_init((sthread_mutex_t*)mutex, attr); + int res = sthread_mutex_init((sthread_mutex_t*)mutex, (sthread_mutexattr_t*)attr); sthread_enable(); return res; } diff --git a/src/sthread/sthread.h b/src/sthread/sthread.h index ba4252e2d0..f0a9117db1 100644 --- a/src/sthread/sthread.h +++ b/src/sthread/sthread.h @@ -31,10 +31,22 @@ typedef unsigned long int sthread_t; int sthread_create(sthread_t* thread, const /*pthread_attr_t*/ void* attr, void* (*start_routine)(void*), void* arg); int sthread_join(sthread_t thread, void** retval); +typedef struct { + unsigned recursive : 1; + unsigned errorcheck : 1; + unsigned robust : 1; +} sthread_mutexattr_t; + +int sthread_mutexattr_init(sthread_mutexattr_t* attr); +int sthread_mutexattr_settype(sthread_mutexattr_t* attr, int type); +int sthread_mutexattr_gettype(const sthread_mutexattr_t* attr, int* type); +int sthread_mutexattr_getrobust(const sthread_mutexattr_t* attr, int* robustness); +int sthread_mutexattr_setrobust(sthread_mutexattr_t* attr, int robustness); + typedef struct { void* mutex; } sthread_mutex_t; -int sthread_mutex_init(sthread_mutex_t* mutex, const /*pthread_mutexattr_t*/ void* attr); +int sthread_mutex_init(sthread_mutex_t* mutex, const sthread_mutexattr_t* attr); int sthread_mutex_lock(sthread_mutex_t* mutex); int sthread_mutex_trylock(sthread_mutex_t* mutex); int sthread_mutex_unlock(sthread_mutex_t* mutex); diff --git a/src/sthread/sthread_impl.cpp b/src/sthread/sthread_impl.cpp index 45ba730029..de2667b0e1 100644 --- a/src/sthread/sthread_impl.cpp +++ b/src/sthread/sthread_impl.cpp @@ -6,6 +6,9 @@ /* SimGrid's pthread interposer. Actual implementation of the symbols (see the comment in sthread.h) */ #include "smpi/smpi.h" +#include "xbt/asserts.h" +#include "xbt/ex.h" +#include "xbt/log.h" #include "xbt/string.hpp" #include #include @@ -38,14 +41,22 @@ int sthread_main(int argc, char** argv, char** envp, int (*raw_main)(int, char** { /* Do not intercept the main when run from SMPI: it will initialize the simulation properly */ for (int i = 0; envp[i] != nullptr; i++) - if (std::string_view(envp[i]).rfind("SMPI_GLOBAL_SIZE", 0) == 0) + if (std::string_view(envp[i]).rfind("SMPI_GLOBAL_SIZE", 0) == 0) { + printf("sthread refuses to intercept the SMPI application %s directly, as its interception is done otherwise.\n", + argv[0]); return raw_main(argc, argv, envp); + } - /* If not in SMPI, the old main becomes an actor in a newly created simulation */ - std::ostringstream id; - id << std::this_thread::get_id(); + /* Do not intercept valgrind step 1 */ + if (not strcmp(argv[0], "/usr/bin/valgrind.bin") || not strcmp(argv[0], "/bin/sh")) { + printf("sthread refuses to intercept the execution of %s. Running the application unmodified.\n", argv[0]); + fflush(stdout); + return raw_main(argc, argv, envp); + } - XBT_DEBUG("sthread main() is starting in thread %s", id.str().c_str()); + /* If not in SMPI, the old main becomes an actor in a newly created simulation */ + printf("sthread is intercepting the execution of %s\n", argv[0]); + fflush(stdout); sg4::Engine e(&argc, argv); auto* zone = sg4::create_full_zone("world"); @@ -56,7 +67,6 @@ int sthread_main(int argc, char** argv, char** envp, int (*raw_main)(int, char** sthread_enable(); sg4::ActorPtr main_actor = sg4::Actor::create("main thread", lilibeth, raw_main, argc, argv, envp); - XBT_INFO("Starting the simulation."); sg4::Engine::get_instance()->run(); sthread_disable(); XBT_INFO("All threads exited. Terminating the simulation."); @@ -108,9 +118,57 @@ int sthread_join(sthread_t thread, void** /*retval*/) return 0; } -int sthread_mutex_init(sthread_mutex_t* mutex, const void* /*pthread_mutexattr_t* attr*/) +int sthread_mutexattr_init(sthread_mutexattr_t* attr) +{ + memset(attr, 0, sizeof(*attr)); + return 0; +} +int sthread_mutexattr_settype(sthread_mutexattr_t* attr, int type) { - auto m = sg4::Mutex::create(); + switch (type) { + case PTHREAD_MUTEX_NORMAL: + xbt_assert(not attr->recursive, "S4U does not allow to remove the recursivness of a mutex."); + attr->recursive = 0; + break; + case PTHREAD_MUTEX_RECURSIVE: + attr->recursive = 1; + attr->errorcheck = 0; // reset + break; + case PTHREAD_MUTEX_ERRORCHECK: + attr->errorcheck = 1; + THROW_UNIMPLEMENTED; + break; + default: + THROW_IMPOSSIBLE; + } + return 0; +} +int sthread_mutexattr_gettype(const sthread_mutexattr_t* attr, int* type) +{ + if (attr->recursive) + *type = PTHREAD_MUTEX_RECURSIVE; + else if (attr->errorcheck) + *type = PTHREAD_MUTEX_ERRORCHECK; + else + *type = PTHREAD_MUTEX_NORMAL; + return 0; +} +int sthread_mutexattr_getrobust(const sthread_mutexattr_t* attr, int* robustness) +{ + *robustness = attr->robust; + return 0; +} +int sthread_mutexattr_setrobust(sthread_mutexattr_t* attr, int robustness) +{ + attr->robust = robustness; + if (robustness) + THROW_UNIMPLEMENTED; + return 0; +} + +int sthread_mutex_init(sthread_mutex_t* mutex, const sthread_mutexattr_t* attr) +{ + auto m = sg4::Mutex::create(attr != nullptr && attr->recursive); intrusive_ptr_add_ref(m.get()); mutex->mutex = m.get(); @@ -123,6 +181,7 @@ int sthread_mutex_lock(sthread_mutex_t* mutex) if (mutex->mutex == nullptr) sthread_mutex_init(mutex, nullptr); + XBT_DEBUG("%s(%p)", __FUNCTION__, mutex); static_cast(mutex->mutex)->lock(); return 0; } @@ -133,7 +192,10 @@ int sthread_mutex_trylock(sthread_mutex_t* mutex) if (mutex->mutex == nullptr) sthread_mutex_init(mutex, nullptr); - return static_cast(mutex->mutex)->try_lock(); + XBT_DEBUG("%s(%p)", __FUNCTION__, mutex); + if (static_cast(mutex->mutex)->try_lock()) + return 0; + return EBUSY; } int sthread_mutex_unlock(sthread_mutex_t* mutex) @@ -142,6 +204,7 @@ int sthread_mutex_unlock(sthread_mutex_t* mutex) if (mutex->mutex == nullptr) sthread_mutex_init(mutex, nullptr); + XBT_DEBUG("%s(%p)", __FUNCTION__, mutex); static_cast(mutex->mutex)->unlock(); return 0; } @@ -151,6 +214,7 @@ int sthread_mutex_destroy(sthread_mutex_t* mutex) if (mutex->mutex == nullptr) sthread_mutex_init(mutex, nullptr); + XBT_DEBUG("%s(%p)", __FUNCTION__, mutex); intrusive_ptr_release(static_cast(mutex->mutex)); return 0; }