From 34b53ccd86e2a99c44710f31412959514572f595 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
 <sebastien.blin@savoirfairelinux.com>
Date: Wed, 17 Feb 2021 17:11:48 -0500
Subject: [PATCH] namedirectory: fix double free on requests_

the map should be protected to avoid erasing requests during
the shutdown and in the callback

Change-Id: I003b868239209c7ccd4f22274cbc2c949fefba2a
GitLab: #440
---
 src/jamidht/namedirectory.cpp | 31 ++++++++++++++++++++++---------
 src/jamidht/namedirectory.h   |  2 ++
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/jamidht/namedirectory.cpp b/src/jamidht/namedirectory.cpp
index 192855d76e..7edc9d1855 100644
--- a/src/jamidht/namedirectory.cpp
+++ b/src/jamidht/namedirectory.cpp
@@ -77,9 +77,7 @@ NameDirectory::instance()
 }
 
 void
-NameDirectory::lookupUri(std::string_view uri,
-                         const std::string& default_server,
-                         LookupCallback cb)
+NameDirectory::lookupUri(std::string_view uri, const std::string& default_server, LookupCallback cb)
 {
     const std::string& default_ns = default_server.empty() ? DEFAULT_SERVER_HOST : default_server;
     std::svmatch pieces_match;
@@ -92,7 +90,7 @@ NameDirectory::lookupUri(std::string_view uri,
             return;
         }
     }
-    JAMI_ERR("Can't parse URI: %.*s", (int)uri.size(), uri.data());
+    JAMI_ERR("Can't parse URI: %.*s", (int) uri.size(), uri.data());
     cb("", Response::invalidResponse);
 }
 
@@ -108,6 +106,12 @@ NameDirectory::NameDirectory(const std::string& serverUrl, std::shared_ptr<dht::
                  + DIR_SEPARATOR_STR + resolver_->get_url().host;
 }
 
+NameDirectory::~NameDirectory()
+{
+    std::lock_guard<std::mutex> lk(requestsMtx_);
+    requests_.clear();
+}
+
 void
 NameDirectory::load()
 {
@@ -172,12 +176,12 @@ NameDirectory::lookupAddress(const std::string& addr, LookupCallback cb)
                         Json::CharReaderBuilder rbuilder;
                         auto reader = std::unique_ptr<Json::CharReader>(rbuilder.newCharReader());
                         if (!reader->parse(response.body.data(),
-                                        response.body.data() + response.body.size(),
-                                        &json,
-                                        &err)) {
+                                           response.body.data() + response.body.size(),
+                                           &json,
+                                           &err)) {
                             JAMI_DBG("Address lookup for %s: can't parse server response: %s",
-                                    addr.c_str(),
-                                    response.body.c_str());
+                                     addr.c_str(),
+                                     response.body.c_str());
                             cb("", Response::error);
                             return;
                         }
@@ -199,12 +203,15 @@ NameDirectory::lookupAddress(const std::string& addr, LookupCallback cb)
                         cb("", Response::error);
                     }
                 }
+                std::lock_guard<std::mutex> lk(requestsMtx_);
                 requests_.erase(reqid);
             });
         request->send();
+        std::lock_guard<std::mutex> lk(requestsMtx_);
         requests_[reqid] = request;
     } catch (const std::exception& e) {
         JAMI_ERR("Error when performing address lookup: %s", e.what());
+        std::lock_guard<std::mutex> lk(requestsMtx_);
         requests_.erase(reqid);
     }
 }
@@ -296,12 +303,15 @@ NameDirectory::lookupName(const std::string& n, LookupCallback cb)
                     cb("", Response::error);
                 }
             }
+            std::lock_guard<std::mutex> lk(requestsMtx_);
             requests_.erase(reqid);
         });
         request->send();
+        std::lock_guard<std::mutex> lk(requestsMtx_);
         requests_[reqid] = request;
     } catch (const std::exception& e) {
         JAMI_ERR("Name lookup for %s failed: %s", name.c_str(), e.what());
+        std::lock_guard<std::mutex> lk(requestsMtx_);
         requests_.erase(reqid);
     }
 }
@@ -399,13 +409,16 @@ NameDirectory::registerName(const std::string& addr,
                     }
                     cb(success ? RegistrationResponse::success : RegistrationResponse::error);
                 }
+                std::lock_guard<std::mutex> lk(requestsMtx_);
                 requests_.erase(reqid);
             });
         request->send();
+        std::lock_guard<std::mutex> lk(requestsMtx_);
         requests_[reqid] = request;
     } catch (const std::exception& e) {
         JAMI_ERR("Error when performing name registration: %s", e.what());
         cb(RegistrationResponse::error);
+        std::lock_guard<std::mutex> lk(requestsMtx_);
         requests_.erase(reqid);
     }
 }
diff --git a/src/jamidht/namedirectory.h b/src/jamidht/namedirectory.h
index 7e96464df0..0ae8ecb6b0 100644
--- a/src/jamidht/namedirectory.h
+++ b/src/jamidht/namedirectory.h
@@ -67,6 +67,7 @@ public:
     using RegistrationCallback = std::function<void(RegistrationResponse response)>;
 
     NameDirectory(const std::string& serverUrl, std::shared_ptr<dht::Logger> l = {});
+    ~NameDirectory();
     void load();
 
     static NameDirectory& instance(const std::string& serverUrl,
@@ -108,6 +109,7 @@ private:
      */
     std::shared_ptr<asio::io_context> httpContext_;
     std::shared_ptr<dht::http::Resolver> resolver_;
+    std::mutex requestsMtx_ {};
     std::map<unsigned, std::shared_ptr<dht::http::Request>> requests_;
 
     std::map<std::string, std::string> nameCache_ {};
-- 
GitLab