From 731dc1c70e398d8b83e8af1898f21c4b36ca3fb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Wed, 22 Apr 2020 12:21:37 -0400
Subject: [PATCH] jamiaccount: check account state, lock mutex for public ops
Change-Id: Idc8c36466c9563d6ed9fbd1c0f7ec401807ba382
---
src/jamidht/contact_list.cpp | 8 ++---
src/jamidht/contact_list.h | 2 +-
src/jamidht/jamiaccount.cpp | 68 +++++++++++++++++++++++-------------
3 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/src/jamidht/contact_list.cpp b/src/jamidht/contact_list.cpp
index d3ab295c1e..1ceb92c992 100644
--- a/src/jamidht/contact_list.cpp
+++ b/src/jamidht/contact_list.cpp
@@ -380,7 +380,7 @@ ContactList::foundAccountDevice(const dht::InfoHash& device,
if (it.second) {
JAMI_DBG("[Contacts] Found account device: %s %s", name.c_str(), device.toString().c_str());
saveKnownDevices();
- callbacks_.devicesChanged();
+ callbacks_.devicesChanged(knownDevices_);
} else {
// update device name
if (not name.empty() and it.first->second.name != name) {
@@ -389,7 +389,7 @@ ContactList::foundAccountDevice(const dht::InfoHash& device,
device.toString().c_str());
it.first->second.name = name;
saveKnownDevices();
- callbacks_.devicesChanged();
+ callbacks_.devicesChanged(knownDevices_);
}
}
}
@@ -419,7 +419,7 @@ ContactList::foundAccountDevice(const std::shared_ptr<dht::crypto::Certificate>&
crt->getId().toString().c_str());
tls::CertificateStore::instance().pinCertificate(crt);
saveKnownDevices();
- callbacks_.devicesChanged();
+ callbacks_.devicesChanged(knownDevices_);
} else {
// update device name
if (not name.empty() and it.first->second.name != name) {
@@ -428,7 +428,7 @@ ContactList::foundAccountDevice(const std::shared_ptr<dht::crypto::Certificate>&
crt->getId().toString().c_str());
it.first->second.name = name;
saveKnownDevices();
- callbacks_.devicesChanged();
+ callbacks_.devicesChanged(knownDevices_);
}
}
return true;
diff --git a/src/jamidht/contact_list.h b/src/jamidht/contact_list.h
index 542078f86e..f892d06f4e 100644
--- a/src/jamidht/contact_list.h
+++ b/src/jamidht/contact_list.h
@@ -42,7 +42,7 @@ public:
using OnContactRemoved = std::function<void(const std::string&, bool)>;
using OnIncomingTrustRequest
= std::function<void(const std::string&, const std::vector<uint8_t>&, time_t)>;
- using OnDevicesChanged = std::function<void()>;
+ using OnDevicesChanged = std::function<void(const std::map<dht::InfoHash, KnownDevice>&)>;
struct OnChangeCallback
{
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index 8f668586f9..9e329bdc26 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -1167,11 +1167,17 @@ JamiAccount::loadAccount(const std::string& archive_password,
received);
});
},
- [this]() {
- dht::ThreadPool::computation().run([id = getAccountID(), devices = getKnownDevices()] {
+ [this](const std::map<dht::InfoHash, KnownDevice>& devices) {
+ std::map<std::string, std::string> ids;
+ for (auto& d : devices) {
+ auto id = d.first.toString();
+ auto label = d.second.name.empty() ? id.substr(0, 8) : d.second.name;
+ ids.emplace(std::move(id), std::move(label));
+ }
+ dht::ThreadPool::computation().run([id=getAccountID(), devices=std::move(ids)] {
emitSignal<DRing::ConfigurationSignal::KnownDevicesChanged>(id, devices);
});
- },
+ }
};
try {
@@ -1497,6 +1503,7 @@ JamiAccount::getVolatileAccountDetails() const
void
JamiAccount::lookupName(const std::string& name)
{
+ std::lock_guard<std::mutex> lock(configurationMutex_);
if (accountManager_)
accountManager_->lookupUri(name,
nameServer_,
@@ -1510,6 +1517,7 @@ JamiAccount::lookupName(const std::string& name)
void
JamiAccount::lookupAddress(const std::string& addr)
{
+ std::lock_guard<std::mutex> lock(configurationMutex_);
auto acc = getAccountID();
if (accountManager_)
accountManager_->lookupAddress(
@@ -1524,6 +1532,7 @@ JamiAccount::lookupAddress(const std::string& addr)
void
JamiAccount::registerName(const std::string& password, const std::string& name)
{
+ std::lock_guard<std::mutex> lock(configurationMutex_);
if (accountManager_)
accountManager_->registerName(
password,
@@ -2695,6 +2704,7 @@ JamiAccount::isMessageTreated(const std::string& id)
std::map<std::string, std::string>
JamiAccount::getKnownDevices() const
{
+ std::lock_guard<std::mutex> lock(configurationMutex_);
if (not accountManager_ or not accountManager_->getInfo())
return {};
std::map<std::string, std::string> ids;
@@ -2935,6 +2945,7 @@ JamiAccount::getContactHeader(pjsip_transport* t)
void
JamiAccount::addContact(const std::string& uri, bool confirmed)
{
+ std::lock_guard<std::mutex> lock(configurationMutex_);
if (accountManager_)
accountManager_->addContact(uri, confirmed);
else
@@ -2944,33 +2955,34 @@ JamiAccount::addContact(const std::string& uri, bool confirmed)
void
JamiAccount::removeContact(const std::string& uri, bool ban)
{
- if (accountManager_)
- accountManager_->removeContact(uri, ban);
- else
- JAMI_WARN("[Account %s] removeContact: account not loaded", getAccountID().c_str());
+ {
+ std::lock_guard<std::mutex> lock(configurationMutex_);
+ if (accountManager_)
+ accountManager_->removeContact(uri, ban);
+ else
+ JAMI_WARN("[Account %s] removeContact: account not loaded", getAccountID().c_str());
+ }
// Remove current connections with contact
- dht::InfoHash peer_account(uri);
-
- std::unique_lock<std::mutex> lk(sipConnectionsMtx_);
std::set<std::string> devices;
- for (const auto& deviceConn : sipConnections_[uri]) {
- devices.emplace(deviceConn.first);
- }
- sipConnections_.erase(uri);
+ {
+ std::unique_lock<std::mutex> lk(sipConnectionsMtx_);
+ for (const auto& deviceConn: sipConnections_[uri]) {
+ devices.emplace(deviceConn.first);
+ }
+ sipConnections_.erase(uri);
- for (auto pendingIt = pendingSipConnections_.begin();
- pendingIt != pendingSipConnections_.end();) {
- if (uri == pendingIt->first) {
- devices.emplace(pendingIt->second);
- pendingIt = pendingSipConnections_.erase(pendingIt);
- } else {
- ++pendingIt;
+ for (auto pendingIt = pendingSipConnections_.begin(); pendingIt != pendingSipConnections_.end();) {
+ if (uri == pendingIt->first) {
+ devices.emplace(pendingIt->second);
+ pendingIt = pendingSipConnections_.erase(pendingIt);
+ } else {
+ ++pendingIt;
+ }
}
}
- lk.unlock();
- for (const auto& device : devices) {
+ for (const auto& device: devices) {
if (connectionManager_)
connectionManager_->closeConnectionsWith(device);
}
@@ -2979,6 +2991,7 @@ JamiAccount::removeContact(const std::string& uri, bool ban)
std::map<std::string, std::string>
JamiAccount::getContactDetails(const std::string& uri) const
{
+ std::lock_guard<std::mutex> lock(configurationMutex_);
return (accountManager_ and accountManager_->getInfo())
? accountManager_->getContactDetails(uri)
: std::map<std::string, std::string> {};
@@ -2987,6 +3000,7 @@ JamiAccount::getContactDetails(const std::string& uri) const
std::vector<std::map<std::string, std::string>>
JamiAccount::getContacts() const
{
+ std::lock_guard<std::mutex> lock(configurationMutex_);
if (not accountManager_)
return {};
return accountManager_->getContacts();
@@ -2997,13 +3011,14 @@ JamiAccount::getContacts() const
std::vector<std::map<std::string, std::string>>
JamiAccount::getTrustRequests() const
{
- return accountManager_ ? accountManager_->getTrustRequests()
- : std::vector<std::map<std::string, std::string>> {};
+ std::lock_guard<std::mutex> lock(configurationMutex_);
+ return accountManager_ ? accountManager_->getTrustRequests() : std::vector<std::map<std::string, std::string>>{};
}
bool
JamiAccount::acceptTrustRequest(const std::string& from)
{
+ std::lock_guard<std::mutex> lock(configurationMutex_);
if (accountManager_)
return accountManager_->acceptTrustRequest(from);
JAMI_WARN("[Account %s] acceptTrustRequest: account not loaded", getAccountID().c_str());
@@ -3013,6 +3028,7 @@ JamiAccount::acceptTrustRequest(const std::string& from)
bool
JamiAccount::discardTrustRequest(const std::string& from)
{
+ std::lock_guard<std::mutex> lock(configurationMutex_);
if (accountManager_)
return accountManager_->discardTrustRequest(from);
JAMI_WARN("[Account %s] discardTrustRequest: account not loaded", getAccountID().c_str());
@@ -3022,6 +3038,7 @@ JamiAccount::discardTrustRequest(const std::string& from)
void
JamiAccount::sendTrustRequest(const std::string& to, const std::vector<uint8_t>& payload)
{
+ std::lock_guard<std::mutex> lock(configurationMutex_);
if (accountManager_)
accountManager_->sendTrustRequest(to, payload);
else
@@ -3031,6 +3048,7 @@ JamiAccount::sendTrustRequest(const std::string& to, const std::vector<uint8_t>&
void
JamiAccount::sendTrustRequestConfirm(const std::string& to)
{
+ std::lock_guard<std::mutex> lock(configurationMutex_);
if (accountManager_)
accountManager_->sendTrustRequestConfirm(dht::InfoHash(to));
else
--
GitLab