From 8c574c5b7bc53d3c75f952bc61f26b6c8e9960ff Mon Sep 17 00:00:00 2001 From: levi Date: Thu, 31 Mar 2022 09:58:40 +0200 Subject: [PATCH 1/4] bumping version --- version.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.txt b/version.txt index ee94dd83..ac39a106 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.8.3 +0.9.0 From 68e59cca2d5ce383a0a4c7613f4d1f6a4c954bfa Mon Sep 17 00:00:00 2001 From: levi Date: Thu, 31 Mar 2022 10:08:10 +0200 Subject: [PATCH 2/4] removing cosim::utility::shared_mutex --- src/cosim/utility/concurrency.cpp | 75 +++----------------------- src/cosim/utility/concurrency.hpp | 49 +---------------- tests/utility_concurrency_unittest.cpp | 13 ----- 3 files changed, 8 insertions(+), 129 deletions(-) diff --git a/src/cosim/utility/concurrency.cpp b/src/cosim/utility/concurrency.cpp index 68ee4c21..37d1a581 100644 --- a/src/cosim/utility/concurrency.cpp +++ b/src/cosim/utility/concurrency.cpp @@ -22,69 +22,6 @@ namespace cosim namespace utility { - -// ============================================================================= -// shared_mutex -// ============================================================================= - - -void shared_mutex::lock() -{ - std::unique_lock lock(mutex_); - condition_.wait(lock, [&] { return sharedCount_ == 0; }); - - // Release the mutex from the unique_lock, so it doesn't get automatically - // unlocked when the function exits. - lock.release(); -} - - -bool shared_mutex::try_lock() -{ - std::unique_lock lock(mutex_, std::try_to_lock); - if (!lock.owns_lock()) return false; - if (sharedCount_ > 0) return false; - - // Release the mutex from the unique_lock, so it doesn't get automatically - // unlocked when the function exits. - lock.release(); - return true; -} - - -void shared_mutex::unlock() -{ - mutex_.unlock(); - condition_.notify_one(); -} - - -void shared_mutex::lock_shared() -{ - std::lock_guard lock(mutex_); - ++sharedCount_; -} - - -bool shared_mutex::try_lock_shared() -{ - std::unique_lock lock(mutex_, std::try_to_lock); - if (!lock.owns_lock()) return false; - ++sharedCount_; - return true; -} - - -void shared_mutex::unlock_shared() -{ - std::unique_lock lock(mutex_); - --sharedCount_; - if (sharedCount_ == 0) { - lock.unlock(); - condition_.notify_one(); - } -} - // ============================================================================= // file_lock // ============================================================================= @@ -112,7 +49,7 @@ void file_lock::lock() // gives us a chance to yield to the other fiber if the operation would // otherwise block. An additional reason is that it is unspecified // to which extent boost::interprocess::file_lock is thread safe. - std::unique_lock mutexLock(fileMutex_->mutex); + std::unique_lock mutexLock(fileMutex_->mutex); std::unique_lock fileLock(fileMutex_->file); mutexLock_ = std::move(mutexLock); fileLock_ = std::move(fileLock); @@ -122,7 +59,7 @@ void file_lock::lock() bool file_lock::try_lock() { // See note on locking order in lock() above. - std::unique_lock mutexLock(fileMutex_->mutex, std::try_to_lock); + std::unique_lock mutexLock(fileMutex_->mutex, std::try_to_lock); if (!mutexLock.owns_lock()) return false; std::unique_lock fileLock(fileMutex_->file, std::try_to_lock); if (!fileLock.owns_lock()) return false; @@ -135,14 +72,14 @@ bool file_lock::try_lock() void file_lock::unlock() { std::get>(fileLock_).unlock(); - std::get>(mutexLock_).unlock(); + std::get>(mutexLock_).unlock(); } void file_lock::lock_shared() { // See note on locking order in lock() above. - std::shared_lock mutexLock(fileMutex_->mutex); + std::shared_lock mutexLock(fileMutex_->mutex); std::shared_lock fileLock(fileMutex_->file); mutexLock_ = std::move(mutexLock); fileLock_ = std::move(fileLock); @@ -152,7 +89,7 @@ void file_lock::lock_shared() bool file_lock::try_lock_shared() { // See note on locking order in lock() above. - std::shared_lock mutexLock(fileMutex_->mutex, std::try_to_lock); + std::shared_lock mutexLock(fileMutex_->mutex, std::try_to_lock); if (!mutexLock.owns_lock()) return false; std::shared_lock fileLock(fileMutex_->file, std::try_to_lock); if (!fileLock.owns_lock()) return false; @@ -165,7 +102,7 @@ bool file_lock::try_lock_shared() void file_lock::unlock_shared() { std::get>(fileLock_).unlock(); - std::get>(mutexLock_).unlock(); + std::get>(mutexLock_).unlock(); } diff --git a/src/cosim/utility/concurrency.hpp b/src/cosim/utility/concurrency.hpp index 0019eb62..5719a85c 100644 --- a/src/cosim/utility/concurrency.hpp +++ b/src/cosim/utility/concurrency.hpp @@ -27,51 +27,6 @@ namespace cosim namespace utility { -/** - * A shared mutex à la `std::shared_mutex`, but with support for fibers. - * - * This class works in the same way as `std::shared_mutex`, but as it is - * implemented in terms of Boost.Fiber primitives, "blocking" operations - * are really "yielding" operations. - * - * The class meets the - * [SharedMutex](https://en.cppreference.com/w/cpp/named_req/SharedMutex) - * requirements. - */ -class shared_mutex -{ -public: - /// Locks the mutex, blocks if the mutex is not available. - void lock(); - - /// Tries to lock the mutex and returns immediately whether it succeeded. - bool try_lock(); - - /// Unlocks the mutex. - void unlock(); - - /** - * Locks the mutex for shared ownership, blocks if the mutex is not - * available. - */ - void lock_shared(); - - /** - * Tries to lock the mutex for shared ownership, returns immediately - * whether it succeeded. - */ - bool try_lock_shared(); - - /// Unlocks the mutex from shared ownership. - void unlock_shared(); - -private: - std::mutex mutex_; - std::condition_variable condition_; - int sharedCount_ = 0; -}; - - /// Whether and how a `file_lock` should acquire a lock on construction. enum class file_lock_initial_state { @@ -229,7 +184,7 @@ class file_lock struct file_mutex { file_mutex(const cosim::filesystem::path& path); - shared_mutex mutex; + std::shared_mutex mutex; boost_wrapper file; }; @@ -241,7 +196,7 @@ class file_lock std::shared_ptr fileMutex_; // The locks we hold on the mutex and the file lock. - std::variant, std::shared_lock> mutexLock_; + std::variant, std::shared_lock> mutexLock_; std::variant, std::shared_lock> fileLock_; }; diff --git a/tests/utility_concurrency_unittest.cpp b/tests/utility_concurrency_unittest.cpp index 13497506..9ce26fbc 100644 --- a/tests/utility_concurrency_unittest.cpp +++ b/tests/utility_concurrency_unittest.cpp @@ -62,19 +62,6 @@ void test_locking(F1&& getMutex1, F2&& getMutex2, F3&& getMutex3) mutex3.unlock(); } - -BOOST_AUTO_TEST_CASE(shared_mutex) -{ - cosim::utility::shared_mutex mutex1; - cosim::utility::shared_mutex mutex2; - cosim::utility::shared_mutex mutex3; - test_locking( - [&]() -> cosim::utility::shared_mutex& { return mutex1; }, - [&]() -> cosim::utility::shared_mutex& { return mutex2; }, - [&]() -> cosim::utility::shared_mutex& { return mutex3; }); -} - - BOOST_AUTO_TEST_CASE(file_lock) { const auto workDir = cosim::utility::temp_dir(); From 3a2ce7e6b5f558374979c97cdf4d86b15200e8a4 Mon Sep 17 00:00:00 2001 From: "Lars T. Kyllingstad" Date: Mon, 4 Apr 2022 10:02:19 +0200 Subject: [PATCH 3/4] Fix concurrency docs after fiber removal --- include/cosim/file_cache.hpp | 2 +- src/cosim/utility/concurrency.cpp | 8 ++----- src/cosim/utility/concurrency.hpp | 38 +++++++++++++------------------ 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/include/cosim/file_cache.hpp b/include/cosim/file_cache.hpp index 42e460e9..681f7dfd 100644 --- a/include/cosim/file_cache.hpp +++ b/include/cosim/file_cache.hpp @@ -118,7 +118,7 @@ class temporary_file_cache : public file_cache /** * A persistent file cache which can be safely accessed by multiple - * processes, threads and fibers concurrently. + * processes and threads concurrently. */ class persistent_file_cache : public file_cache { diff --git a/src/cosim/utility/concurrency.cpp b/src/cosim/utility/concurrency.cpp index 37d1a581..c55d14ac 100644 --- a/src/cosim/utility/concurrency.cpp +++ b/src/cosim/utility/concurrency.cpp @@ -43,12 +43,8 @@ file_lock::file_lock( void file_lock::lock() { // NOTE: The reason we can't use std::lock() here is that we must make - // sure that the mutex gets locked before the file. Otherwise, the - // code might block on the file lock, when the lock is in fact held by - // a different fiber in the same process. Trying to lock the mutex first - // gives us a chance to yield to the other fiber if the operation would - // otherwise block. An additional reason is that it is unspecified - // to which extent boost::interprocess::file_lock is thread safe. + // sure that the mutex gets locked before the file, since + // boost::interprocess::file_lock isn't thread safe. std::unique_lock mutexLock(fileMutex_->mutex); std::unique_lock fileLock(fileMutex_->file); mutexLock_ = std::move(mutexLock); diff --git a/src/cosim/utility/concurrency.hpp b/src/cosim/utility/concurrency.hpp index 5719a85c..ba0d7a56 100644 --- a/src/cosim/utility/concurrency.hpp +++ b/src/cosim/utility/concurrency.hpp @@ -46,24 +46,18 @@ enum class file_lock_initial_state * * This class provides interprocess synchronisation based on * `boost::interprocess::file_lock`, augmenting it with support for - * inter-fiber and inter-thread synchronisation. This is achieved by - * combining the file lock with a lock on a global `shared_mutex` object - * associated with the file. + * inter-thread synchronisation. This is achieved by combining the file lock + * with a lock on a global `std::shared_mutex` object associated with the + * file. * - * Note that a single `file_lock` object may only be used by one fiber at a - * time. That is, if it is locked by one fiber, it must be unlocked by the - * same fiber. Other fibers may not attempt to call its locking or unlocking - * functions in the meantime. + * Note that `file_lock` objects should not be shared among threads. The + * inter-thread synchronisation is handled internally via the global + * per-file mutex. * - * Furthermore, once a fiber has locked a file, the same fiber may not attempt - * to use a different `file_lock` object to lock the same file, as this would - * cause a deadlock. - * - * Therefore, to synchronise between fibers (including those running in - * separate threads), it is recommended to create one and only one `file_lock` - * object associated with the same file in each fiber. If, for some reason, - * a `file_lock` *must* be transferred between fibers, do so only when it is - * in the unlocked state. + * Furthermore, once a thread has locked a file, the same thread may not + * attempt to use a different `file_lock` object to lock the same file, as + * this would cause a deadlock. (This is also because they would share the + * same global mutex.) * * The lock automatically gets unlocked on destruction. * @@ -103,7 +97,7 @@ class file_lock * \pre * This `file_lock` object is not already locked. * The file is not locked by a different `file_lock` object in the - * same fiber. + * same thread. */ void lock(); @@ -114,7 +108,7 @@ class file_lock * \pre * This `file_lock` object is not already locked. * The file is not locked by a different `file_lock` object in the - * same fiber. + * same thread. */ bool try_lock(); @@ -122,7 +116,7 @@ class file_lock * Unlocks the file. * * \pre - * This `file_lock` object has been locked in the current fiber. + * This `file_lock` object has been locked in the current thread. */ void unlock(); @@ -132,7 +126,7 @@ class file_lock * \pre * This `file_lock` object is not already locked. * The file is not locked by a different `file_lock` object in the - * same fiber. + * same thread. */ void lock_shared(); @@ -143,7 +137,7 @@ class file_lock * \pre * This `file_lock` object is not already locked. * The file is not locked by a different `file_lock` object in the - * same fiber. + * same thread. */ bool try_lock_shared(); @@ -152,7 +146,7 @@ class file_lock * * \pre * This `file_lock` object has been locked for shared ownership in - * the current fiber. + * the current thread. */ void unlock_shared(); From 5320ad739ecab87154a727d6fbb85144be8b5521 Mon Sep 17 00:00:00 2001 From: "Lars T. Kyllingstad" Date: Mon, 4 Apr 2022 10:03:05 +0200 Subject: [PATCH 4/4] Remove last traces of fibers --- cmake/project-config.cmake.in | 2 +- include/cosim/execution.hpp | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/cmake/project-config.cmake.in b/cmake/project-config.cmake.in index 06228c8c..daaa699a 100644 --- a/cmake/project-config.cmake.in +++ b/cmake/project-config.cmake.in @@ -6,7 +6,7 @@ include(CMakeFindDependencyMacro) list(APPEND CMAKE_MODULE_PATH "${PACKAGE_PREFIX_DIR}/@LIBCOSIM_CMAKE_INSTALL_DIR@") find_dependency(MS_GSL REQUIRED) -find_dependency(Boost REQUIRED COMPONENTS date_time fiber log) +find_dependency(Boost REQUIRED COMPONENTS date_time log) set(FMILibrary_USE_SHARED_LIB @FMILibrary_USE_SHARED_LIB@) find_dependency(FMILibrary REQUIRED) find_dependency(LIBZIP REQUIRED) diff --git a/include/cosim/execution.hpp b/include/cosim/execution.hpp index da8568d7..a7d2102a 100644 --- a/include/cosim/execution.hpp +++ b/include/cosim/execution.hpp @@ -244,12 +244,6 @@ class execution /** * Advance the co-simulation forward to the given logical time. * - * This function returns immediately, and its actions will be performed - * asynchronously. As it is not possible to perform more than one - * asynchronous operation at a time per `execution` object, client code - * must verify that the operation completed before calling the function - * again (e.g. by calling `boost::fibers::future::get()` on the result). - * * \param targetTime * The logical time at which the co-simulation should pause (optional). * If specified, this must always be greater than the value of