Skip to content

Commit

Permalink
Make SynchronizedValue::replace() return an accessor
Browse files Browse the repository at this point in the history
Do this to make it more usable in cases where one might want to replace
the value and retain the lock for further processing.

Do not use replace() in the constructor as it unnecessarily locks the
mutex. Instead, directly call std::make_unique().
  • Loading branch information
dartdart26 committed Sep 13, 2021
1 parent 6285cc3 commit 50a1f25
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 20 deletions.
34 changes: 17 additions & 17 deletions util/include/synchronized_value.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,6 @@ class SynchronizedValue {
using SharedLock = std::shared_lock<MutexType>;

public:
// Construct the value in-place with the given arguments.
template <typename... Args>
SynchronizedValue(Args&&... args) {
replace(std::forward<Args>(args)...);
}

// Replace the value by constructing a new one in-place with the given arguments.
//
// Precondition: the calling thread doesn't have any accessors at the time of the call. If it does, the behaviour is
// undefined.
template <typename... Args>
void replace(Args&&... args) {
auto new_val = std::make_unique<T>(std::forward<Args>(args)...);
auto lock = ExclusiveLock{mtx_};
val_ = std::move(new_val);
}

// Provides read-write access to the value.
// Locks the mutex in the constructor and unlocks it in the dtor.
class Accessor {
Expand Down Expand Up @@ -74,6 +57,23 @@ class SynchronizedValue {
SharedLock lock_;
};

public:
// Construct the value in-place with the given arguments.
template <typename... Args>
SynchronizedValue(Args&&... args) : val_{std::make_unique<T>(std::forward<Args>(args)...)} {}

// Replace the value by constructing a new one in-place with the given arguments.
//
// Precondition: the calling thread doesn't have any accessors at the time of the call. If it does, the behaviour is
// undefined.
template <typename... Args>
Accessor replace(Args&&... args) {
auto new_val = std::make_unique<T>(std::forward<Args>(args)...);
auto lock = ExclusiveLock{mtx_};
val_ = std::move(new_val);
return Accessor{*val_, std::move(lock)};
}

// Create an accessor to the value.
//
// Precondition: the calling thread doesn't have any other accessors at the time of the call. If it does, the
Expand Down
23 changes: 20 additions & 3 deletions util/test/synchronized_value_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,29 @@ TEST(synchronized_value, replace_with_arguments) {
ASSERT_EQ(42, *v.constAccess());
}

TEST(synchronized_value, replace_accessor_changes_value) {
auto v = SynchronizedValue<int>{41};

{
auto a = v.replace(42);
ASSERT_EQ(42, *a);
*a = 43;
}

auto a = v.constAccess();
ASSERT_EQ(43, *a);
}

TEST(synchronized_value, accessor_changes_value) {
auto v = SynchronizedValue<int>{42};
auto a = v.access();
ASSERT_EQ(42, *a);

*a = 43;
{
auto a = v.access();
ASSERT_EQ(42, *a);
*a = 43;
}

auto a = v.constAccess();
ASSERT_EQ(43, *a);
}

Expand Down

0 comments on commit 50a1f25

Please sign in to comment.