Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing cosim::utility::shared_mutex #692

Merged
merged 4 commits into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 6 additions & 69 deletions src/cosim/utility/concurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,69 +22,6 @@ namespace cosim
namespace utility
{


// =============================================================================
// shared_mutex
// =============================================================================


void shared_mutex::lock()
{
std::unique_lock<std::mutex> 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<std::mutex> 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<std::mutex> lock(mutex_);
++sharedCount_;
}


bool shared_mutex::try_lock_shared()
{
std::unique_lock<std::mutex> lock(mutex_, std::try_to_lock);
if (!lock.owns_lock()) return false;
++sharedCount_;
return true;
}


void shared_mutex::unlock_shared()
{
std::unique_lock<std::mutex> lock(mutex_);
--sharedCount_;
if (sharedCount_ == 0) {
lock.unlock();
condition_.notify_one();
}
}

// =============================================================================
// file_lock
// =============================================================================
Expand Down Expand Up @@ -112,7 +49,7 @@ void file_lock::lock()
// gives us a chance to yield to the other fiber if the operation would
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment talks about fibers

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what the correct note for lock() should be. Maybe we can use std::lock now?

There are mentions about fibers in almost every doc string in concurrenty.cpp and concurrency.hpp. @kyllingstad, shall we just remove all fiber references in the doc (remove all \pre sections) ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most cases, including this one, "fiber" can just be replaced by "thread". But if you want, I can take care of updating the docs in this PR.

// otherwise block. An additional reason is that it is unspecified
// to which extent boost::interprocess::file_lock is thread safe.
std::unique_lock<shared_mutex> mutexLock(fileMutex_->mutex);
std::unique_lock<std::shared_mutex> mutexLock(fileMutex_->mutex);
std::unique_lock<boost_wrapper> fileLock(fileMutex_->file);
mutexLock_ = std::move(mutexLock);
fileLock_ = std::move(fileLock);
Expand All @@ -122,7 +59,7 @@ void file_lock::lock()
bool file_lock::try_lock()
{
// See note on locking order in lock() above.
std::unique_lock<shared_mutex> mutexLock(fileMutex_->mutex, std::try_to_lock);
std::unique_lock<std::shared_mutex> mutexLock(fileMutex_->mutex, std::try_to_lock);
if (!mutexLock.owns_lock()) return false;
std::unique_lock<boost_wrapper> fileLock(fileMutex_->file, std::try_to_lock);
if (!fileLock.owns_lock()) return false;
Expand All @@ -135,14 +72,14 @@ bool file_lock::try_lock()
void file_lock::unlock()
{
std::get<std::unique_lock<boost_wrapper>>(fileLock_).unlock();
std::get<std::unique_lock<shared_mutex>>(mutexLock_).unlock();
std::get<std::unique_lock<std::shared_mutex>>(mutexLock_).unlock();
}


void file_lock::lock_shared()
{
// See note on locking order in lock() above.
std::shared_lock<shared_mutex> mutexLock(fileMutex_->mutex);
std::shared_lock<std::shared_mutex> mutexLock(fileMutex_->mutex);
std::shared_lock<boost_wrapper> fileLock(fileMutex_->file);
mutexLock_ = std::move(mutexLock);
fileLock_ = std::move(fileLock);
Expand All @@ -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<shared_mutex> mutexLock(fileMutex_->mutex, std::try_to_lock);
std::shared_lock<std::shared_mutex> mutexLock(fileMutex_->mutex, std::try_to_lock);
if (!mutexLock.owns_lock()) return false;
std::shared_lock<boost_wrapper> fileLock(fileMutex_->file, std::try_to_lock);
if (!fileLock.owns_lock()) return false;
Expand All @@ -165,7 +102,7 @@ bool file_lock::try_lock_shared()
void file_lock::unlock_shared()
{
std::get<std::shared_lock<boost_wrapper>>(fileLock_).unlock();
std::get<std::shared_lock<shared_mutex>>(mutexLock_).unlock();
std::get<std::shared_lock<std::shared_mutex>>(mutexLock_).unlock();
}


Expand Down
49 changes: 2 additions & 47 deletions src/cosim/utility/concurrency.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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;
};

Expand All @@ -241,7 +196,7 @@ class file_lock
std::shared_ptr<file_mutex> fileMutex_;

// The locks we hold on the mutex and the file lock.
std::variant<std::unique_lock<shared_mutex>, std::shared_lock<shared_mutex>> mutexLock_;
std::variant<std::unique_lock<std::shared_mutex>, std::shared_lock<std::shared_mutex>> mutexLock_;
std::variant<std::unique_lock<boost_wrapper>, std::shared_lock<boost_wrapper>> fileLock_;
};

Expand Down
13 changes: 0 additions & 13 deletions tests/utility_concurrency_unittest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.8.3
0.9.0