Merge LeftRight fixes from develop

This commit is contained in:
Sebastian Messmer 2019-02-02 13:03:14 -08:00
parent 1cdf530548
commit 6142485859
2 changed files with 54 additions and 20 deletions

View File

@ -31,14 +31,6 @@ private:
template <class T>
class LeftRight final {
public:
LeftRight()
: _writeMutex()
, _foregroundCounterIndex{0}
, _foregroundDataIndex{0}
, _counters{{{0}, {0}}}
, _data{{{}, {}}}
, _inDestruction(false) {}
~LeftRight() {
// from now on, no new readers/writers will be accepted (see asserts in read()/write())
_inDestruction = true;
@ -56,22 +48,25 @@ public:
template <typename F>
auto read(F&& readFunc) const {
detail::IncrementRAII _increment_counter(&_counters[_foregroundCounterIndex.load()]); // NOLINT(cppcoreguidelines-pro-bounds-constant-array-index)
if(_inDestruction.load()) {
throw std::logic_error("Issued LeftRight::read() after the destructor started running");
}
detail::IncrementRAII _increment_counter(&_counters[_foregroundCounterIndex.load()]); // NOLINT(cppcoreguidelines-pro-bounds-constant-array-index)
return readFunc(_data[_foregroundDataIndex.load()]); // NOLINT(cppcoreguidelines-pro-bounds-constant-array-index)
}
// Throwing from write would result in invalid state
// Throwing an exception in writeFunc is ok but causes the state to be either the old or the new state,
// depending on if the first or the second call to writeFunc threw.
template <typename F>
auto write(F&& writeFunc) {
std::unique_lock<std::mutex> lock(_writeMutex);
if(_inDestruction.load()) {
throw std::logic_error("Issued LeftRight::read() after the destructor started running");
}
std::unique_lock<std::mutex> lock(_writeMutex);
return _write(writeFunc);
}
@ -116,7 +111,7 @@ private:
_waitForBackgroundCounterToBeZero(localCounterIndex);
/*
*4. Switch A/B counters
* 4. Switch A/B counters
*
* Now that we know all readers on B are really gone, we can switch the counters and have new readers
* increment A's counter again, which is the correct counter since they're reading A.
@ -133,7 +128,7 @@ private:
_waitForBackgroundCounterToBeZero(localCounterIndex);
// 6. Write to B
_callWriteFuncOnBackgroundInstance(writeFunc, localDataIndex);
return _callWriteFuncOnBackgroundInstance(writeFunc, localDataIndex);
}
template<class F>
@ -155,11 +150,11 @@ private:
}
std::mutex _writeMutex;
std::atomic<uint8_t> _foregroundCounterIndex;
std::atomic<uint8_t> _foregroundDataIndex;
mutable std::array<std::atomic<int32_t>, 2> _counters;
std::array<T, 2> _data;
std::atomic<bool> _inDestruction;
std::atomic<uint8_t> _foregroundCounterIndex = {0};
std::atomic<uint8_t> _foregroundDataIndex = {0};
mutable std::array<std::atomic<int32_t>, 2> _counters = {{{0}, {0}}};
std::array<T, 2> _data = {{{}, {}}};
std::atomic<bool> _inDestruction = {false};
};
}

View File

@ -30,9 +30,17 @@ TEST(LeftRightTest, givenVector_whenWritingAndReading_thenChangesArePresent) {
EXPECT_EQ((vector<int>{5, 6}), read);
}
TEST(LeftRightTest, givenVector_whenWritingReturnsValue_thenValueIsReturned) {
LeftRight<vector<int>> obj;
auto a = obj.write([] (auto&) -> int {return 5;});
static_assert(std::is_same<int, decltype(a)>::value, "");
EXPECT_EQ(5, a);
}
TEST(LeftRightTest, readsCanBeConcurrent) {
LeftRight<int> obj;
std::atomic<int> num_running_readers{0};
std::atomic<int> num_running_readers{0};
std::thread reader1([&] () {
obj.read([&] (auto&) {
@ -157,7 +165,7 @@ TEST(LeftRightTest, whenWriteThrowsException_thenThrowsThrough) {
);
}
TEST(LeftRightTest, givenInt_whenWriteThrowsException_thenResetsToOldState) {
TEST(LeftRightTest, givenInt_whenWriteThrowsExceptionOnFirstCall_thenResetsToOldState) {
LeftRight<int> obj;
obj.write([](auto& obj) {obj = 5;});
@ -180,6 +188,37 @@ TEST(LeftRightTest, givenInt_whenWriteThrowsException_thenResetsToOldState) {
EXPECT_EQ(5, read);
}
// note: each write is executed twice, on the foreground and background copy.
// We need to test a thrown exception in either call is handled correctly.
TEST(LeftRightTest, givenInt_whenWriteThrowsExceptionOnSecondCall_thenKeepsNewState) {
LeftRight<int> obj;
obj.write([](auto& obj) {obj = 5;});
bool write_called = false;
EXPECT_THROW(
obj.write([&](auto& obj) {
obj = 6;
if (write_called) {
// this is the second time the write callback is executed
throw MyException();
} else {
write_called = true;
}
}),
MyException
);
// check reading it returns new value
int read = obj.read([] (auto& obj) {return obj;});
EXPECT_EQ(6, read);
// check changes are also present in background copy
obj.write([] (auto&) {}); // this switches to the background copy
read = obj.read([] (auto& obj) {return obj;});
EXPECT_EQ(6, read);
}
TEST(LeftRightTest, givenVector_whenWriteThrowsException_thenResetsToOldState) {
LeftRight<vector<int>> obj;