8000 used the required mutex in Store::clear to avoid races (#2957) · Hotkey/arangodb@49fa0bf · GitHub
[go: up one dir, main page]

Skip to content

Commit 49fa0bf

Browse files
jsteemannfceller
authored andcommitted
used the required mutex in Store::clear to avoid races (arangodb#2957)
also added asserts for that the mutex is actually held everywhere where it is required
1 parent 6a7b2ff commit 49fa0bf

File tree

2 files changed

+26
-3
lines changed

2 files changed

+26
-3
lines changed

CHANGELOG

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
devel
22
-----
33

4+
* fixed a multi-threading issue in the agency when callElection was called
5+
while the SuperVision was calling updateSnapshot
6+
47
* added startup option `--query.tracking-with-bindvars`
58

69
This option controls whether the list of currently running queries

arangod/Agency/Store.cpp

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ inline static bool endpointPathFromUrl(std::string const& url,
116116
Store::Store(Agent* agent, std::string const& name)
117117
: Thread(name), _agent(agent), _node(name, this) {}
118118

119-
/// Move constructor
119+
/// Move constructor. note: this is not thread-safe!
120120
Store::Store(Store&& other)
121121
: Thread(other._node.name()),
122122
_agent(std::move(other._agent)),
@@ -141,6 +141,7 @@ Store& Store::operator=(Store const& rhs) {
141141
/// Move assignment operator
142142
Store& Store::operator=(Store&& rhs) {
143143
if (&rhs != this) {
144+
MUTEX_LOCKER(otherLock, rhs._storeLock);
144145
_agent = std::move(rhs._agent);
145146
_timeTable = std::move(rhs._timeTable);
146147
_observerTable = std::move(rhs._o 8000 bserverTable);
@@ -375,6 +376,8 @@ check_ret_t Store::check(VPackSlice const& slice, CheckMode mode) const {
375376
check_ret_t ret;
376377
ret.open();
377378

379+
_storeLock.assertLockedByCurrentThread();
380+
378381
for (auto const& precond : VPackObjectIterator(slice)) { // Preconditions
379382

380383
std::string key = precond.key.copyString();
@@ -671,6 +674,8 @@ bool Store::applies(arangodb::velocypack::Slice const& transaction) {
671674

672675
sort(idx.begin(), idx.end(),
673676
[&abskeys](size_t i1, size_t i2) { return abskeys[i1] < abskeys[i2]; });
677+
678+
_storeLock.assertLockedByCurrentThread();
674679

675680
for (const auto& i : idx) {
676681
std::string const& key = keys.at(i);
@@ -689,6 +694,7 @@ bool Store::applies(arangodb::velocypack::Slice const& transaction) {
689694

690695
// Clear my data
691696
void Store::clear() {
697+
MUTEX_LOCKER(storeLocker, _storeLock);
692698
_timeTable.clear();
693699
_observerTable.clear();
694700
_observedTable.clear();
@@ -730,30 +736,42 @@ Store& Store::operator=(VPackSlice const& slice) {
730736

731737
/// Put key value store in velocypack, guarded by caller
732738
void Store::toBuilder(Builder& b, bool showHidden) const {
739+
_storeLock.assertLockedByCurrentThread();
733740
_node.toBuilder(b, showHidden); }
734741

735742
/// Time table
736-
std::multimap<TimePoint, std::string>& Store::timeTable() { return _timeTable; }
743+
std::multimap<TimePoint, std::string>& Store::timeTable() {
744+
_storeLock.assertLockedByCurrentThread();
745+
return _timeTable;
746+
}
747+
737748
/// Time table
738-
const std::multimap<TimePoint, std::string>& Store::timeTable() const {
749+
std::multimap<TimePoint, std::string> const& Store::timeTable() const {
750+
_storeLock.assertLockedByCurrentThread();
739751
return _timeTable;
740752
}
741753

742754
/// Observer table
743755
std::multimap<std::string, std::string>& Store::observerTable() {
756+
_storeLock.assertLockedByCurrentThread();
744757
return _observerTable;
745758
}
759+
746760
/// Observer table
747761
std::multimap<std::string, std::string> const& Store::observerTable() const {
762+
_storeLock.assertLockedByCurrentThread();
748763
return _observerTable;
749764
}
750765

751766
/// Observed table
752767
std::multimap<std::string, std::string>& Store::observedTable() {
768+
_storeLock.assertLockedByCurrentThread();
753769
return _observedTable;
754770
}
771+
755772
/// Observed table
756773
std::multimap<std::string, std::string> const& Store::observedTable() const {
774+
_storeLock.assertLockedByCurrentThread();
757775
return _observedTable;
758776
}
759777

@@ -771,6 +789,8 @@ bool Store::has(std::string const& path) const {
771789

772790
/// Remove ttl entry for path, guarded by caller
773791
void Store::removeTTL(std::string const& uri) {
792+
_storeLock.assertLockedByCurrentThread();
793+
774794
if (!_timeTable.empty()) {
775795
for (auto it = _timeTable.cbegin(); it != _timeTable.cend();) {
776796
if (it->second == uri) {

0 commit comments

Comments
 (0)
0