Team2890/allwpilib
4
0
Fork 0
mirror of https://github.com/wpilibsuite/allwpilib synced 2026-06-27 02:01:42 +00:00
Code Issues Packages Projects Releases Wiki Activity

Storage: Make setters globally atomic.

Browse Source 
Previously, setters were locally but not globally atomic because they
used GetEntry() (globally atomic) in conjunction with locally atomic
gets/sets to the StorageEntry.  To support synchronizing network handshakes
they need to be globally atomic.

GetEntry() has been removed due to this issue, so a helper was added to
StorageTest instead.
This commit is contained in:
Peter Johnson
2015-07-26 09:27:23 -07:00
parent 3c7cb363ba
commit 18659257d3
3 changed files with 34 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
src/Storage.cpp
View File

@@ -29,13 +29,6 @@ std::shared_ptr<StorageEntry> Storage::FindEntry(StringRef name) const {
return i == m_entries.end() ? nullptr : i->getValue();
}
std::shared_ptr<StorageEntry> Storage::GetEntry(StringRef name) {
std::lock_guard<std::mutex> lock(m_mutex);
auto& entry = m_entries[name];
if (!entry) entry = std::make_shared<StorageEntry>();
return entry;
}
std::shared_ptr<Value> Storage::GetEntryValue(StringRef name) const {
auto entry = FindEntry(name);
if (!entry) return nullptr;
@@ -45,7 +38,9 @@ std::shared_ptr<Value> Storage::GetEntryValue(StringRef name) const {
bool Storage::SetEntryValue(StringRef name, std::shared_ptr<Value> value) {
if (name.empty()) return true;
if (!value) return true;
auto entry = GetEntry(name);
std::lock_guard<std::mutex> lock(m_mutex);
auto& entry = m_entries[name];
if (!entry) entry = std::make_shared<StorageEntry>();
auto old_value = entry->value();
if (old_value && old_value->type() != value->type())
return false; // error on type mismatch
@@ -61,7 +56,9 @@ bool Storage::SetEntryValue(StringRef name, std::shared_ptr<Value> value) {
void Storage::SetEntryTypeValue(StringRef name, std::shared_ptr<Value> value) {
if (name.empty()) return;
if (!value) return;
auto entry = GetEntry(name);
std::lock_guard<std::mutex> lock(m_mutex);
auto& entry = m_entries[name];
if (!entry) entry = std::make_shared<StorageEntry>();
auto old_value = entry->value();
entry->set_value(value);
if (!old_value || *old_value != *value) {
@@ -75,8 +72,10 @@ void Storage::SetEntryTypeValue(StringRef name, std::shared_ptr<Value> value) {
void Storage::SetEntryFlags(StringRef name, unsigned int flags) {
if (name.empty()) return;
auto entry = FindEntry(name);
if (!entry) return;
std::lock_guard<std::mutex> lock(m_mutex);
auto i = m_entries.find(name);
if (i == m_entries.end()) return;
auto entry = i->getValue();
if (entry->flags() != flags) {
entry->set_flags(flags);
AddUpdate(entry, Update::kFlagsUpdate); // put on update queue

1
src/Storage.h
View File

@@ -92,7 +92,6 @@ class Storage {
// Finds, but does not create entry. Returns nullptr if not found.
std::shared_ptr<StorageEntry> FindEntry(StringRef name) const;
std::shared_ptr<StorageEntry> GetEntry(StringRef name);
// Accessors required by Dispatcher.
void EnableUpdates() { m_updates_enabled = true; }

43
test/unit/StorageTest.cpp
View File

@@ -21,6 +21,11 @@ class StorageTest : public ::testing::Test {
}
Storage::EntriesMap& entries() { return storage.m_entries; }
Storage::UpdateQueue& updates() { return storage.updates(); }
std::shared_ptr<StorageEntry> GetEntry(StringRef name) {
auto& entry = storage.m_entries[name];
if (!entry) entry = std::make_shared<StorageEntry>();
return entry;
}
Storage storage;
};
@@ -102,15 +107,15 @@ TEST_F(StorageTest, FindEntryNotExist) {
}
TEST_F(StorageTest, FindEntryExist) {
auto entry1 = storage.GetEntry("foo");
auto entry1 = GetEntry("foo");
auto entry = storage.FindEntry("foo");
ASSERT_TRUE(bool(entry));
ASSERT_EQ(entry1, entry);
ASSERT_FALSE(storage.FindEntry("bar"));
}
TEST_F(StorageTest, GetEntryNotExist) {
auto entry = storage.GetEntry("foo");
TEST_F(StorageTest, StorageEntryInit) {
auto entry = GetEntry("foo");
ASSERT_TRUE(bool(entry));
ASSERT_EQ(1u, entries().size());
@@ -137,7 +142,7 @@ TEST_F(StorageTest, SetEntryTypeValueAssignNew) {
// brand new entry
auto value = Value::MakeBoolean(true);
storage.SetEntryTypeValue("foo", value);
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(value, entry->value());
ASSERT_EQ(1u, updates().size());
auto update = updates().pop();
@@ -149,7 +154,7 @@ TEST_F(StorageTestPopulateOne, SetEntryTypeValueAssignTypeChange) {
// update with different type results in assignment message
auto value = Value::MakeDouble(0.0);
storage.SetEntryTypeValue("foo", value);
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(value, entry->value());
ASSERT_EQ(1u, updates().size());
auto update = updates().pop();
@@ -162,7 +167,7 @@ TEST_F(StorageTestPopulateOne, SetEntryTypeValueEqualValue) {
// message is issued (minimizing bandwidth usage)
auto value = Value::MakeBoolean(true);
storage.SetEntryTypeValue("foo", value);
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(value, entry->value());
ASSERT_TRUE(updates().empty());
}
@@ -171,7 +176,7 @@ TEST_F(StorageTestPopulateOne, SetEntryTypeValueDifferentValue) {
// update with same type and different value results in value update message
auto value = Value::MakeBoolean(false);
storage.SetEntryTypeValue("foo", value);
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(value, entry->value());
ASSERT_EQ(1u, updates().size());
auto update = updates().pop();
@@ -196,7 +201,7 @@ TEST_F(StorageTest, SetEntryValueAssignNew) {
// brand new entry
auto value = Value::MakeBoolean(true);
EXPECT_TRUE(storage.SetEntryValue("foo", value));
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(value, entry->value());
ASSERT_EQ(1u, updates().size());
auto update = updates().pop();
@@ -208,7 +213,7 @@ TEST_F(StorageTestPopulateOne, SetEntryValueAssignTypeChange) {
// update with different type results in error and no message
auto value = Value::MakeDouble(0.0);
EXPECT_FALSE(storage.SetEntryValue("foo", value));
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_NE(value, entry->value());
ASSERT_TRUE(updates().empty());
}
@@ -218,7 +223,7 @@ TEST_F(StorageTestPopulateOne, SetEntryValueEqualValue) {
// message is issued (minimizing bandwidth usage)
auto value = Value::MakeBoolean(true);
EXPECT_TRUE(storage.SetEntryValue("foo", value));
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(value, entry->value());
ASSERT_TRUE(updates().empty());
}
@@ -227,7 +232,7 @@ TEST_F(StorageTestPopulateOne, SetEntryValueDifferentValue) {
// update with same type and different value results in value update message
auto value = Value::MakeBoolean(false);
EXPECT_TRUE(storage.SetEntryValue("foo", value));
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(value, entry->value());
ASSERT_EQ(1u, updates().size());
auto update = updates().pop();
@@ -259,7 +264,7 @@ TEST_F(StorageTestPopulateOne, SetEntryFlagsEqualValue) {
// update with same value: no update message is issued (minimizing bandwidth
// usage)
storage.SetEntryFlags("foo", 0u);
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(0u, entry->flags());
ASSERT_TRUE(updates().empty());
}
@@ -267,7 +272,7 @@ TEST_F(StorageTestPopulateOne, SetEntryFlagsEqualValue) {
TEST_F(StorageTestPopulateOne, SetEntryFlagsDifferentValue) {
// update with different value results in flags update message
storage.SetEntryFlags("foo", 1u);
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(1u, entry->flags());
ASSERT_EQ(1u, updates().size());
auto update = updates().pop();
@@ -298,7 +303,7 @@ TEST_F(StorageTest, DeleteEntryNotExist) {
}
TEST_F(StorageTestPopulateOne, DeleteEntryExist) {
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
storage.DeleteEntry("foo");
ASSERT_TRUE(entries().empty());
ASSERT_EQ(1u, updates().size());
@@ -472,7 +477,7 @@ TEST_F(StorageTest, LoadPersistentAssign) {
std::istringstream iss(
"[NetworkTables Storage 3.0]\nboolean \"foo\"=true\n");
EXPECT_TRUE(storage.LoadPersistent(iss, warn_func));
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(*Value::MakeBoolean(true), *entry->value());
EXPECT_EQ(NT_PERSISTENT, entry->flags());
ASSERT_EQ(1u, updates().size());
@@ -489,7 +494,7 @@ TEST_F(StorageTestPopulateOne, LoadPersistentUpdateFlags) {
std::istringstream iss(
"[NetworkTables Storage 3.0]\nboolean \"foo\"=true\n");
EXPECT_TRUE(storage.LoadPersistent(iss, warn_func));
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(*Value::MakeBoolean(true), *entry->value());
EXPECT_EQ(NT_PERSISTENT, entry->flags());
ASSERT_EQ(1u, updates().size());
@@ -503,13 +508,13 @@ TEST_F(StorageTestPopulateOne, LoadPersistentUpdateValue) {
auto warn_func =
[&](std::size_t line, const char* msg) { warn.Warn(line, msg); };
storage.GetEntry("foo")->set_flags(NT_PERSISTENT);
GetEntry("foo")->set_flags(NT_PERSISTENT);
while (!updates().empty()) updates().pop();
std::istringstream iss(
"[NetworkTables Storage 3.0]\nboolean \"foo\"=false\n");
EXPECT_TRUE(storage.LoadPersistent(iss, warn_func));
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(*Value::MakeBoolean(false), *entry->value());
EXPECT_EQ(NT_PERSISTENT, entry->flags());
ASSERT_EQ(1u, updates().size());
@@ -526,7 +531,7 @@ TEST_F(StorageTestPopulateOne, LoadPersistentUpdateValueFlags) {
std::istringstream iss(
"[NetworkTables Storage 3.0]\nboolean \"foo\"=false\n");
EXPECT_TRUE(storage.LoadPersistent(iss, warn_func));
auto entry = storage.GetEntry("foo");
auto entry = GetEntry("foo");
EXPECT_EQ(*Value::MakeBoolean(false), *entry->value());
EXPECT_EQ(NT_PERSISTENT, entry->flags());
ASSERT_EQ(2u, updates().size());