Skip to content
Snippets Groups Projects
Commit 1fda90fe authored by Sébastien Blin's avatar Sébastien Blin Committed by Adrien Béraud
Browse files

contact_list: make thread safe

Protect trustRequests_ with lock

GitLab: #934
Change-Id: Ic4bfd40894579040f8ed68745b76515090f797e2
parent cc5897f3
No related branches found
No related tags found
No related merge requests found
...@@ -119,6 +119,7 @@ ContactList::updateConversation(const dht::InfoHash& h, const std::string& conve ...@@ -119,6 +119,7 @@ ContactList::updateConversation(const dht::InfoHash& h, const std::string& conve
bool bool
ContactList::removeContact(const dht::InfoHash& h, bool ban) ContactList::removeContact(const dht::InfoHash& h, bool ban)
{ {
std::unique_lock<std::mutex> lk(mutex_);
JAMI_WARN("[Contacts] removeContact: %s", h.to_c_str()); JAMI_WARN("[Contacts] removeContact: %s", h.to_c_str());
auto c = contacts_.find(h); auto c = contacts_.find(h);
if (c == contacts_.end()) if (c == contacts_.end())
...@@ -133,6 +134,7 @@ ContactList::removeContact(const dht::InfoHash& h, bool ban) ...@@ -133,6 +134,7 @@ ContactList::removeContact(const dht::InfoHash& h, bool ban)
if (trustRequests_.erase(h) > 0) if (trustRequests_.erase(h) > 0)
saveTrustRequests(); saveTrustRequests();
saveContacts(); saveContacts();
lk.unlock();
#ifdef ENABLE_PLUGIN #ifdef ENABLE_PLUGIN
auto filename = path_.filename().string(); auto filename = path_.filename().string();
jami::Manager::instance() jami::Manager::instance()
...@@ -206,8 +208,11 @@ ContactList::updateContact(const dht::InfoHash& id, const Contact& contact) ...@@ -206,8 +208,11 @@ ContactList::updateContact(const dht::InfoHash& id, const Contact& contact)
stateChanged = c->second.update(contact); stateChanged = c->second.update(contact);
} }
if (stateChanged) { if (stateChanged) {
{
std::lock_guard<std::mutex> lk(mutex_);
if (trustRequests_.erase(id) > 0) if (trustRequests_.erase(id) > 0)
saveTrustRequests(); saveTrustRequests();
}
if (c->second.isActive()) { if (c->second.isActive()) {
trust_->setCertificateStatus(id.toString(), dhtnet::tls::TrustStore::PermissionStatus::ALLOWED); trust_->setCertificateStatus(id.toString(), dhtnet::tls::TrustStore::PermissionStatus::ALLOWED);
callbacks_.contactAdded(id.toString(), c->second.confirmed); callbacks_.contactAdded(id.toString(), c->second.confirmed);
...@@ -290,6 +295,7 @@ ContactList::onTrustRequest(const dht::InfoHash& peer_account, ...@@ -290,6 +295,7 @@ ContactList::onTrustRequest(const dht::InfoHash& peer_account,
{ {
bool accept = false; bool accept = false;
// Check existing contact // Check existing contact
std::unique_lock<std::mutex> lk(mutex_);
auto contact = contacts_.find(peer_account); auto contact = contacts_.find(peer_account);
bool active = false; bool active = false;
if (contact != contacts_.end()) { if (contact != contacts_.end()) {
...@@ -329,6 +335,7 @@ ContactList::onTrustRequest(const dht::InfoHash& peer_account, ...@@ -329,6 +335,7 @@ ContactList::onTrustRequest(const dht::InfoHash& peer_account,
} }
saveTrustRequests(); saveTrustRequests();
} }
lk.unlock();
// Note: call JamiAccount's callback to build ConversationRequest anyway // Note: call JamiAccount's callback to build ConversationRequest anyway
if (!confirm) if (!confirm)
callbacks_.trustRequest(peer_account.toString(), callbacks_.trustRequest(peer_account.toString(),
...@@ -349,6 +356,7 @@ ContactList::getTrustRequests() const ...@@ -349,6 +356,7 @@ ContactList::getTrustRequests() const
{ {
using Map = std::map<std::string, std::string>; using Map = std::map<std::string, std::string>;
std::vector<Map> ret; std::vector<Map> ret;
std::lock_guard<std::mutex> lk(mutex_);
ret.reserve(trustRequests_.size()); ret.reserve(trustRequests_.size());
for (const auto& r : trustRequests_) { for (const auto& r : trustRequests_) {
ret.emplace_back( ret.emplace_back(
...@@ -365,6 +373,7 @@ std::map<std::string, std::string> ...@@ -365,6 +373,7 @@ std::map<std::string, std::string>
ContactList::getTrustRequest(const dht::InfoHash& from) const ContactList::getTrustRequest(const dht::InfoHash& from) const
{ {
using Map = std::map<std::string, std::string>; using Map = std::map<std::string, std::string>;
std::lock_guard<std::mutex> lk(mutex_);
auto r = trustRequests_.find(from); auto r = trustRequests_.find(from);
if (r == trustRequests_.end()) if (r == trustRequests_.end())
return {}; return {};
...@@ -379,13 +388,16 @@ bool ...@@ -379,13 +388,16 @@ bool
ContactList::acceptTrustRequest(const dht::InfoHash& from) ContactList::acceptTrustRequest(const dht::InfoHash& from)
{ {
// The contact sent us a TR so we are in its contact list // The contact sent us a TR so we are in its contact list
std::unique_lock<std::mutex> lk(mutex_);
auto i = trustRequests_.find(from); auto i = trustRequests_.find(from);
if (i == trustRequests_.end()) if (i == trustRequests_.end())
return false; return false;
addContact(from, true, i->second.conversationId); auto convId = i->second.conversationId;
// Clear trust request // Clear trust request
trustRequests_.erase(i); trustRequests_.erase(i);
saveTrustRequests(); saveTrustRequests();
lk.unlock();
addContact(from, true, convId);
return true; return true;
} }
...@@ -399,6 +411,7 @@ ContactList::acceptConversation(const std::string& convId, const std::string& de ...@@ -399,6 +411,7 @@ ContactList::acceptConversation(const std::string& convId, const std::string& de
bool bool
ContactList::discardTrustRequest(const dht::InfoHash& from) ContactList::discardTrustRequest(const dht::InfoHash& from)
{ {
std::lock_guard<std::mutex> lk(mutex_);
if (trustRequests_.erase(from) > 0) { if (trustRequests_.erase(from) > 0) {
saveTrustRequests(); saveTrustRequests();
return true; return true;
...@@ -572,6 +585,7 @@ ContactList::getSyncData() const ...@@ -572,6 +585,7 @@ ContactList::getSyncData() const
sync_data.peers = getContacts(); sync_data.peers = getContacts();
static constexpr size_t MAX_TRUST_REQUESTS = 20; static constexpr size_t MAX_TRUST_REQUESTS = 20;
std::lock_guard<std::mutex> lk(mutex_);
if (trustRequests_.size() <= MAX_TRUST_REQUESTS) if (trustRequests_.size() <= MAX_TRUST_REQUESTS)
for (const auto& req : trustRequests_) for (const auto& req : trustRequests_)
sync_data.trust_requests.emplace(req.first, sync_data.trust_requests.emplace(req.first,
......
...@@ -147,7 +147,7 @@ public: ...@@ -147,7 +147,7 @@ public:
bool syncDevice(const dht::PkId& device, const time_point& syncDate); bool syncDevice(const dht::PkId& device, const time_point& syncDate);
private: private:
mutable std::mutex lock; mutable std::mutex mutex_;
std::map<dht::InfoHash, Contact> contacts_; std::map<dht::InfoHash, Contact> contacts_;
std::map<dht::InfoHash, TrustRequest> trustRequests_; std::map<dht::InfoHash, TrustRequest> trustRequests_;
std::map<dht::InfoHash, KnownDevice> knownDevicesLegacy_; std::map<dht::InfoHash, KnownDevice> knownDevicesLegacy_;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment