From aec4aed93f71f2ba757ecc7f69caff90ab4d54c9 Mon Sep 17 00:00:00 2001
From: Gustavo Lopes <mail@geleia.net>
Date: Thu, 17 Oct 2024 11:38:54 +0100
Subject: [PATCH] Fix abort() call on appsec helper unload

runner was living too long (until shared library unload) due to a static
shared pointer used for RC notifications. Plus, the destructor would
have a call to shared_for_this(), which would try to revive the shared
pointer being destroyed, which raise an exception due to there being no
shared pointer anymore. We would catch this and abort().

Instead, destroy the runner earlier (when its own thread finishes).
Reset the static shared pointer just before that.
---
 appsec/src/helper/main.cpp   |  2 ++
 appsec/src/helper/runner.cpp | 12 ++++++++----
 appsec/src/helper/runner.hpp |  4 +++-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/appsec/src/helper/main.cpp b/appsec/src/helper/main.cpp
index 68a8f69166..36f74360c1 100644
--- a/appsec/src/helper/main.cpp
+++ b/appsec/src/helper/main.cpp
@@ -125,6 +125,8 @@ int appsec_helper_main_impl()
 
         runner->run();
 
+        runner->unregister_for_rc_notifications();
+
         finished.store(true, std::memory_order_release);
     }};
     thread_id = thr.native_handle();
diff --git a/appsec/src/helper/runner.cpp b/appsec/src/helper/runner.cpp
index dee62304fd..8567db2a11 100644
--- a/appsec/src/helper/runner.cpp
+++ b/appsec/src/helper/runner.cpp
@@ -123,7 +123,7 @@ void runner::register_for_rc_notifications()
                 std::atomic_load(&RUNNER_FOR_NOTIFICATIONS);
             if (!runner) {
                 // NOLINTNEXTLINE(bugprone-lambda-function-name)
-                SPDLOG_ERROR("No runner to notify of remote config updates");
+                SPDLOG_WARN("No runner to notify of remote config updates");
                 ddog_remote_config_path_free(path);
                 return;
             }
@@ -136,15 +136,19 @@ void runner::register_for_rc_notifications()
         });
 }
 
-runner::~runner() noexcept
+void runner::unregister_for_rc_notifications()
 {
+    SPDLOG_INFO("Unregister runner for RC update callback");
     try {
         std::shared_ptr<runner> expected = shared_from_this();
         std::atomic_compare_exchange_strong(&RUNNER_FOR_NOTIFICATIONS,
             &expected, std::shared_ptr<runner>(nullptr));
     } catch (...) {
-        // can only happened if there is no shared_ptr for the runner
-        // in this case a std::bad_weak_ptr is thrown
+        // can only happen if there is no shared_ptr for the runner
+        // in this case a std::bad_weak_ptr is thrown.
+        // But we only expose runner through a shared pointer, so this would
+        // require extraordinary actions to destroy the shared pointer but not
+        // the object.
         std::abort();
     }
 }
diff --git a/appsec/src/helper/runner.hpp b/appsec/src/helper/runner.hpp
index 2c495b3910..b11b217a9f 100644
--- a/appsec/src/helper/runner.hpp
+++ b/appsec/src/helper/runner.hpp
@@ -26,7 +26,7 @@ class runner : public std::enable_shared_from_this<runner> {
     runner &operator=(const runner &) = delete;
     runner(runner &&) = delete;
     runner &operator=(runner &&) = delete;
-    ~runner() noexcept;
+    ~runner() = default;
 
     static void resolve_symbols();
 
@@ -34,6 +34,8 @@ class runner : public std::enable_shared_from_this<runner> {
 
     void register_for_rc_notifications();
 
+    void unregister_for_rc_notifications();
+
     [[nodiscard]] bool interrupted() const
     {
         return interrupted_.load(std::memory_order_acquire);