dnsdist: Fix a race condition with custom Lua web handlers

Custom web handlers written in Lua modify the global Lua context,
but until now they did not take the lock protecting it so a data
race condition was possible.
Reported by TSAN while running our unit tests.

diff --git a/pdns/dnsdistdist/dnsdist-lua-web.cc b/pdns/dnsdistdist/dnsdist-lua-web.cc
index 0498aed4fcf9ebe010cdd501d8b87e201fa39ae6..35cf0d0f09630224bd17a2d17f932982640c17a8 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-lua-web.cc
+++ b/pdns/dnsdistdist/dnsdist-lua-web.cc
@@ -25,14 +25,14 @@
 #include "dnsdist-lua.hh"
 #include "dnsdist-web.hh"
 
-void registerWebHandler(const std::string& endpoint, std::function<void(const YaHTTP::Request&, YaHTTP::Response&)> handler);
+void registerWebHandler(const std::string& endpoint, std::function<void(const YaHTTP::Request&, YaHTTP::Response&)> handler, bool isLua);
 
 void setupLuaWeb(LuaContext& luaCtx)
 {
 #ifndef DISABLE_LUA_WEB_HANDLERS
   luaCtx.writeFunction("registerWebHandler", [](const std::string& path, std::function<void(const YaHTTP::Request*, YaHTTP::Response*)> handler) {
     /* LuaWrapper does a copy for objects passed by reference, so we pass a pointer */
-    registerWebHandler(path, [handler](const YaHTTP::Request& req, YaHTTP::Response& resp) { handler(&req, &resp); });
+    registerWebHandler(path, [handler](const YaHTTP::Request& req, YaHTTP::Response& resp) { handler(&req, &resp); }, true);
   });
 
   luaCtx.registerMember<std::string(YaHTTP::Request::*)>("path", [](const YaHTTP::Request& req) -> std::string { return req.url.path; }, [](YaHTTP::Request& req, const std::string& path) { (void) path; });
@@ -78,4 +78,3 @@ void setupLuaWeb(LuaContext& luaCtx)
   });
 #endif /* DISABLE_LUA_WEB_HANDLERS */
 }
-

diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc
index 7fe717b1d30ae5c590297904cd81635a1442d58d..4e18a637529b9ed79b7a5c5f5881d0494fc36ba7 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-web.cc
+++ b/pdns/dnsdistdist/dnsdist-web.cc
@@ -1717,14 +1717,20 @@ static void handleRings(const YaHTTP::Request& req, YaHTTP::Response& resp)
 }
 
 using WebHandler = std::function<void(const YaHTTP::Request&, YaHTTP::Response&)>;
-static SharedLockGuarded<std::unordered_map<std::string, WebHandler>> s_webHandlers;
+struct WebHandlerContext
+{
+  WebHandler d_handler;
+  bool d_isLua;
+};
+
+static SharedLockGuarded<std::unordered_map<std::string, WebHandlerContext>> s_webHandlers;
 
-void registerWebHandler(const std::string& endpoint, WebHandler handler);
+void registerWebHandler(const std::string& endpoint, WebHandler handler, bool isLua = false);
 
-void registerWebHandler(const std::string& endpoint, WebHandler handler)
+void registerWebHandler(const std::string& endpoint, WebHandler handler, bool isLua)
 {
   auto handlers = s_webHandlers.write_lock();
-  (*handlers)[endpoint] = std::move(handler);
+  (*handlers).emplace(std::make_pair(endpoint, WebHandlerContext{std::move(handler), isLua}));
 }
 
 void clearWebHandlers()
@@ -1868,17 +1874,23 @@ static void connectionThread(WebClientConnection&& conn)
       resp.status = 405;
     }
     else {
-      WebHandler handler;
+      std::optional<WebHandlerContext> handlerCtx{std::nullopt};
       {
         auto handlers = s_webHandlers.read_lock();
         const auto webHandlersIt = handlers->find(req.url.path);
         if (webHandlersIt != handlers->end()) {
-          handler = webHandlersIt->second;
+          handlerCtx = webHandlersIt->second;
         }
       }
 
-      if (handler) {
-        handler(req, resp);
+      if (handlerCtx) {
+        if (handlerCtx->d_isLua) {
+          auto lua = g_lua.lock();
+          handlerCtx->d_handler(req, resp);
+        }
+        else {
+          handlerCtx->d_handler(req, resp);
+        }
       }
       else {
         resp.status = 404;