From 9b03e03e1e1617d6daccf8ca967d7942c249d21c Mon Sep 17 00:00:00 2001
From: Andreas Traczyk <andreas.traczyk@savoirfairelinux.com>
Date: Tue, 20 Feb 2018 15:38:55 -0500
Subject: [PATCH] ringaccount: fix concurrent access during setAccountDetails
- removes the locks from doRegister and doUnregister and places
one on loadValues to prevent concurrent access when upnp port mapping
is lagging and account details are set before registration completes.
- places locks in saveNodes and loadNodes.
- still not optimal, but a safer approach with less chance for deadlock.
Change-Id: I6e4feb76fe6dc4242797ffc7a466c9114313da3e
---
src/manager.cpp | 2 ++
src/ringdht/ringaccount.cpp | 8 +++++---
src/ringdht/ringaccount.h | 2 +-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/manager.cpp b/src/manager.cpp
index 2c32b1a576..d2adddece0 100644
--- a/src/manager.cpp
+++ b/src/manager.cpp
@@ -2651,6 +2651,8 @@ Manager::setAccountDetails(const std::string& accountID,
if (account->isUsable())
account->doRegister();
+ else
+ account->doUnregister();
// Update account details to the client side
emitSignal<DRing::ConfigurationSignal::AccountDetailsChanged>(accountID, details);
diff --git a/src/ringdht/ringaccount.cpp b/src/ringdht/ringaccount.cpp
index 5ebcfbec62..6158f43c63 100644
--- a/src/ringdht/ringaccount.cpp
+++ b/src/ringdht/ringaccount.cpp
@@ -1984,7 +1984,6 @@ RingAccount::onTrackedBuddyOffline(std::map<dht::InfoHash, BuddyInfo>::iterator&
void
RingAccount::doRegister_()
{
- std::lock_guard<std::mutex> lock(registerMtx_);
try {
if (not identity_.first or not identity_.second)
throw std::runtime_error("No identity configured for this account.");
@@ -2465,8 +2464,6 @@ RingAccount::replyToIncomingIceMsg(const std::shared_ptr<SIPCall>& call,
void
RingAccount::doUnregister(std::function<void(bool)> released_cb)
{
- std::lock_guard<std::mutex> lock(registerMtx_);
-
if (registrationState_ == RegistrationState::INITIALIZING
|| registrationState_ == RegistrationState::ERROR_NEED_MIGRATION) {
if (released_cb) released_cb(false);
@@ -2490,6 +2487,7 @@ RingAccount::doUnregister(std::function<void(bool)> released_cb)
saveValues(dht_.exportValues());
dht_.join();
setRegistrationState(RegistrationState::UNREGISTERED);
+
if (released_cb)
released_cb(false);
}
@@ -2682,6 +2680,7 @@ RingAccount::saveNodes(const std::vector<dht::NodeExport>& nodes) const
fileutils::check_dir(cachePath_.c_str());
std::string nodesPath = cachePath_+DIR_SEPARATOR_STR "nodes";
{
+ std::lock_guard<std::mutex> lock(fileutils::getFileLock(nodesPath));
std::ofstream file(nodesPath, std::ios::trunc | std::ios::binary);
if (!file.is_open()) {
RING_ERR("Could not save nodes to %s", nodesPath.c_str());
@@ -2695,6 +2694,7 @@ RingAccount::saveNodes(const std::vector<dht::NodeExport>& nodes) const
void
RingAccount::saveValues(const std::vector<dht::ValuesExport>& values) const
{
+ std::lock_guard<std::mutex> lock(dhtValuesMtx_);
fileutils::check_dir(dataPath_.c_str());
for (const auto& v : values) {
const std::string fname = dataPath_ + DIR_SEPARATOR_STR + v.first.toString();
@@ -2709,6 +2709,7 @@ RingAccount::loadNodes() const
std::vector<dht::NodeExport> nodes;
std::string nodesPath = cachePath_+DIR_SEPARATOR_STR "nodes";
{
+ std::lock_guard<std::mutex> lock(fileutils::getFileLock(nodesPath));
std::ifstream file(nodesPath);
if (!file.is_open()) {
RING_DBG("Could not load nodes from %s", nodesPath.c_str());
@@ -2731,6 +2732,7 @@ RingAccount::loadNodes() const
std::vector<dht::ValuesExport>
RingAccount::loadValues() const
{
+ std::lock_guard<std::mutex> lock(dhtValuesMtx_);
std::vector<dht::ValuesExport> values;
const auto dircontent(fileutils::readDirectory(dataPath_));
for (const auto& fname : dircontent) {
diff --git a/src/ringdht/ringaccount.h b/src/ringdht/ringaccount.h
index bbf200df3b..369c2fa590 100644
--- a/src/ringdht/ringaccount.h
+++ b/src/ringdht/ringaccount.h
@@ -409,7 +409,6 @@ class RingAccount : public SIPAccountBase {
*/
void onTrackedBuddyOnline(std::map<dht::InfoHash, BuddyInfo>::iterator& buddy_info_it, const dht::InfoHash& device_id);
- std::mutex registerMtx_;
void doRegister_();
void incomingCall(dht::IceCandidates&& msg, const std::shared_ptr<dht::crypto::Certificate>& from_cert, const dht::InfoHash& from);
@@ -580,6 +579,7 @@ class RingAccount : public SIPAccountBase {
dht::crypto::Identity loadIdentity(const std::string& crt_path, const std::string& key_path, const std::string& key_pwd) const;
std::vector<dht::NodeExport> loadNodes() const;
std::vector<dht::ValuesExport> loadValues() const;
+ mutable std::mutex dhtValuesMtx_;
bool dhtPublicInCalls_ {true};
--
GitLab