commit 3d8c9128650c9e42a5f467a7a484d6ac3073aa91
author Alex Vakulenko <avakulenko@chromium.org> Fri Mar 13 13:54:00 2015 -0700
committer ChromeOS Commit Bot <chromeos-commit-bot@chromium.org> Tue Mar 17 22:32:48 2015 +0000
tree 49d6070cd41309ae980cfdf80ac051b5b3631329
parent c168b9410ef9021364c689d49d59ed009be7ca10

webserver: Fix handler unregistration when client quits

When there are multiple protocol handlers sharing the same name,
the implementation in libwebserv library had a bug that would
attempt to unregister request handler on a wrong protocol handler.

We need to track the exact protocol handler D-Bus object proxy that
we registered a request handler, so when the client exits, we remove
the request handler from the correct protocol handler object on the
server side.

BUG=brillo:539
TEST=`FEATURES=test emerge-link webserver`
     Manually tested under debugger on both webservd and privetd
     sides starting/existing processes in various orders and made
     sure that D-Bus communications happen on the correct objects.

Change-Id: I8499052b48cb38a70b96fc3d6214490b412050c9
Reviewed-on: https://chromium-review.googlesource.com/260027
Trybot-Ready: Alex Vakulenko <avakulenko@chromium.org>
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Commit-Queue: Alex Vakulenko <avakulenko@chromium.org>

libwebserv/protocol_handler.cc

diff --git a/libwebserv/protocol_handler.cc b/libwebserv/protocol_handler.cc
index fe4d567..fe0c9b0 100644
--- a/libwebserv/protocol_handler.cc
+++ b/libwebserv/protocol_handler.cc
@@ -76,9 +76,13 @@
     const std::string& url,
     const std::string& method,
     std::unique_ptr<RequestHandlerInterface> handler) {
-  request_handlers_.emplace(++last_handler_id_,
-                            HandlerMapEntry{url, method, std::string{},
-                                            std::move(handler)});
+  request_handlers_.emplace(
+      ++last_handler_id_,
+      HandlerMapEntry{url, method,
+                      std::map<ProtocolHandlerProxy*, std::string>{},
+                      std::move(handler)});
+  // For each instance of remote protocol handler object sharing the same name,
+  // add the request handler.
   for (const auto& pair : proxies_) {
     pair.second->AddRequestHandlerAsync(
         url,
@@ -86,7 +90,8 @@
         server_->service_name_,
         base::Bind(&ProtocolHandler::AddHandlerSuccess,
                    weak_ptr_factory_.GetWeakPtr(),
-                   last_handler_id_),
+                   last_handler_id_,
+                   pair.second),
         base::Bind(&ProtocolHandler::AddHandlerError,
                    weak_ptr_factory_.GetWeakPtr(),
                    last_handler_id_));
@@ -109,9 +114,9 @@
   if (p == request_handlers_.end())
     return false;
 
-  for (const auto& pair : proxies_) {
-    pair.second->RemoveRequestHandlerAsync(
-        p->second.remote_handler_id,
+  for (const auto& pair : p->second.remote_handler_ids) {
+    pair.first->RemoveRequestHandlerAsync(
+        pair.second,
         base::Bind(&base::DoNothing),
         base::Bind(&IgnoreError));
   }
@@ -129,7 +134,8 @@
         server_->service_name_,
         base::Bind(&ProtocolHandler::AddHandlerSuccess,
                    weak_ptr_factory_.GetWeakPtr(),
-                   pair.first),
+                   pair.first,
+                   proxy),
         base::Bind(&ProtocolHandler::AddHandlerError,
                    weak_ptr_factory_.GetWeakPtr(),
                    pair.first));
@@ -140,13 +146,16 @@
   proxies_.erase(object_path);
   if (proxies_.empty())
     remote_handler_id_map_.clear();
+  for (auto& pair : request_handlers_)
+    pair.second.remote_handler_ids.clear();
 }
 
 void ProtocolHandler::AddHandlerSuccess(int handler_id,
+                                        ProtocolHandlerProxy* proxy,
                                         const std::string& remote_handler_id) {
   auto p = request_handlers_.find(handler_id);
   CHECK(p != request_handlers_.end());
-  p->second.remote_handler_id = remote_handler_id;
+  p->second.remote_handler_ids.emplace(proxy, remote_handler_id);
 
   remote_handler_id_map_.emplace(remote_handler_id, handler_id);
 }

libwebserv/protocol_handler.h

diff --git a/libwebserv/protocol_handler.h b/libwebserv/protocol_handler.h
index bd8afb2..f6543cc 100644
--- a/libwebserv/protocol_handler.h
+++ b/libwebserv/protocol_handler.h
@@ -42,7 +42,7 @@
 // information.
 class LIBWEBSERV_EXPORT ProtocolHandler final {
  public:
-  explicit ProtocolHandler(const std::string& name, Server* server);
+  ProtocolHandler(const std::string& name, Server* server);
   ~ProtocolHandler();
 
   // Returns true if the protocol handler object is connected to the web server
@@ -123,7 +123,7 @@
   struct LIBWEBSERV_PRIVATE HandlerMapEntry {
     std::string url;
     std::string method;
-    std::string remote_handler_id;
+    std::map<ProtocolHandlerProxy*, std::string> remote_handler_ids;
     std::unique_ptr<RequestHandlerInterface> handler;
   };
 
@@ -138,7 +138,9 @@
   // Asynchronous callbacks to handle successful or failed request handler
   // registration over D-Bus.
   LIBWEBSERV_PRIVATE void AddHandlerSuccess(
-      int handler_id, const std::string& remote_handler_id);
+      int handler_id,
+      ProtocolHandlerProxy* proxy,
+      const std::string& remote_handler_id);
   LIBWEBSERV_PRIVATE void AddHandlerError(int handler_id,
                                           chromeos::Error* error);