refactor(userspace/falco): add an ad-hoc concurrent object for signal handlers

Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>

userspace/falco/CMakeLists.txt

@@ -16,7 +16,6 @@ configure_file(config_falco.h.in config_falco.h)
 set(
   FALCO_SOURCES
   app/app.cpp
-  app/signals.cpp
   app/options.cpp
   app/actions/helpers_generic.cpp
   app/actions/helpers_inspector.cpp

userspace/falco/app/app.cpp

@@ -19,6 +19,10 @@ limitations under the License.
 #include "signals.h"
 #include "actions/actions.h"
 
+falco::atomic_signal_handler falco::app::g_terminate_signal;
+falco::atomic_signal_handler falco::app::g_restart_signal;
+falco::atomic_signal_handler falco::app::g_reopen_outputs_signal;
+
 using app_action = std::function<falco::app::run_result(falco::app::state&)>;
 
 bool falco::app::run(int argc, char** argv, bool& restart, std::string& errstr)
@@ -99,7 +103,7 @@ bool falco::app::run(int argc, char** argv, bool& restart, std::string& errstr)
 		errstr = res.errstr;
 	}
 
-	restart = falco::app::should_restart();
+	restart = falco::app::g_restart_signal.triggered();
 
 	return res.success;
 }

userspace/falco/app/signals.cpp

@@ -1,86 +0,0 @@
-/*
-Copyright (C) 2023 The Falco Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-#include "signals.h"
-#include "../logger.h"
-#include "../falco_outputs.h"
-
-std::atomic<int> falco::app::g_terminate(APP_SIGNAL_NOT_SET);
-std::atomic<int> falco::app::g_restart(APP_SIGNAL_NOT_SET);
-std::atomic<int> falco::app::g_reopen_outputs(APP_SIGNAL_NOT_SET);
-
-static inline bool should_take_action_to_signal(std::atomic<int>& v)
-{
-	// we expected the signal to be received, and we try to set action-taken flag
-	int value = APP_SIGNAL_SET;
-	while (!v.compare_exchange_weak(
-			value,
-			APP_SIGNAL_ACTION_TAKEN,
-			std::memory_order_seq_cst,
-			std::memory_order_seq_cst))
-	{
-		// application already took action, there's no need to do it twice
-		if (value == APP_SIGNAL_ACTION_TAKEN)
-		{
-			return false;
-		}
-
-		// signal did was not really received, so we "fake" receiving it
-		if (value == APP_SIGNAL_NOT_SET)
-		{
-			v.store(APP_SIGNAL_SET, std::memory_order_seq_cst);
-		}
-
-		// reset "expected" CAS variable and keep looping until we succeed
-		value = APP_SIGNAL_SET;
-	}
-	return true;
-}
-
-void falco::app::terminate(bool verbose)
-{
-	if (should_take_action_to_signal(falco::app::g_terminate))
-	{
-		if (verbose)
-		{
-			falco_logger::log(LOG_INFO, "SIGINT received, exiting...\n");
-		}
-	}
-}
-
-void falco::app::reopen_outputs(std::function<void()> on_reopen, bool verbose)
-{
-	if (should_take_action_to_signal(falco::app::g_reopen_outputs))
-	{
-		if (verbose)
-		{
-			falco_logger::log(LOG_INFO, "SIGUSR1 received, reopening outputs...\n");
-		}
-        on_reopen();
-		falco::app::g_reopen_outputs.store(APP_SIGNAL_NOT_SET);
-	}
-}
-
-void falco::app::restart(bool verbose)
-{
-	if (should_take_action_to_signal(falco::app::g_restart))
-	{
-		if (verbose)
-		{
-			falco_logger::log(LOG_INFO, "SIGHUP received, restarting...\n");
-		}
-	}
-}

userspace/falco/app/signals.h

@@ -16,41 +16,14 @@ limitations under the License.
 
 #pragma once
 
-#include <atomic>
-#include <functional>
-
-#define APP_SIGNAL_NOT_SET          0   // The signal flag is not set
-#define APP_SIGNAL_SET              1   // The signal flag has been set
-#define APP_SIGNAL_ACTION_TAKEN     2   // The signal flag has been set and the application took action
+#include "../atomic_signal_handler.h"
 
 namespace falco {
 namespace app {
 
-// todo(jasondellaluce): hide this into a class
-extern std::atomic<int> g_terminate;
-extern std::atomic<int> g_restart;
-extern std::atomic<int> g_reopen_outputs;
-
-void terminate(bool verbose=true);
-
-void restart(bool verbose=true);
-
-void reopen_outputs(std::function<void()> on_reopen, bool verbose=true);
-
-inline bool should_terminate()
-{
-    return g_terminate.load(std::memory_order_seq_cst) != APP_SIGNAL_NOT_SET;
-}
-
-inline bool should_restart()
-{
-    return g_restart.load(std::memory_order_seq_cst) != APP_SIGNAL_NOT_SET;
-}
-
-inline bool should_reopen_outputs()
-{
-    return g_reopen_outputs.load(std::memory_order_seq_cst) != APP_SIGNAL_NOT_SET;
-}
+extern atomic_signal_handler g_terminate_signal;
+extern atomic_signal_handler g_restart_signal;
+extern atomic_signal_handler g_reopen_outputs_signal;
 
 }; // namespace app
 }; // namespace falco

userspace/falco/atomic_signal_handler.h

@@ -0,0 +1,137 @@
+/*
+Copyright (C) 2023 The Falco Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+#include <mutex>
+#include <atomic>
+#include <functional>
+
+namespace falco
+{
+    /**
+     * @brief A concurrent object that helps properly handling
+     * system signals from multiple threads.
+     */
+    class atomic_signal_handler
+    {
+    public:
+        atomic_signal_handler(): m_triggered(false), m_handled(false) { }
+        atomic_signal_handler(atomic_signal_handler&&) = default;
+        atomic_signal_handler& operator = (atomic_signal_handler&&) = default;
+        atomic_signal_handler(const atomic_signal_handler&) = delete;
+        atomic_signal_handler& operator = (const atomic_signal_handler&) = delete;
+        ~atomic_signal_handler() = default;
+        
+        /**
+         * @brief Returns true if the underlying atomic implementation
+         * is lock-free as per C++ standard semantics.
+         */
+        inline bool is_lock_free() const
+        {
+            return m_handled.is_lock_free() && m_triggered.is_lock_free();
+        }
+
+        /**
+         * @brief Resets the handler to its initial state, which is
+         * non-triggered and non-handled.
+         */
+        inline void reset()
+        {
+            m_handled.store(false, std::memory_order_seq_cst);
+            m_triggered.store(false, std::memory_order_seq_cst);
+        }
+
+        /**
+         * @brief Returns true if the signal has been triggered.
+         */
+        inline bool triggered() const
+        {
+            return m_triggered.load(std::memory_order_seq_cst);
+        }
+
+        /**
+         * @brief Returns true if the signal has been handled.
+         */
+        inline bool handled() const
+        {
+            return m_handled.load(std::memory_order_seq_cst);
+        }
+
+        /**
+         * @brief Triggers the signal. Must generally be invoked from
+         * within an actual signal handler (created with the `signal`
+         * system call). Can eventually be invoked for "faking"
+         * the triggering of a signal programmatically.
+         */
+        inline void trigger()
+        {
+            m_triggered.store(true, std::memory_order_seq_cst);
+            m_handled.store(false, std::memory_order_seq_cst);
+        }
+
+        /**
+         * @brief If a signal is triggered, performs an handler action.
+         * The action function will be invoked exactly once among all the
+         * simultaneus calls. The action will not be performed if the
+         * signal is not triggered, or if the triggered has already been
+         * handled. When an action is being performed, all the simultaneus
+         * callers will wait and be blocked up until its execution is finished.
+         * If the handler action throws an exception, it will be considered
+         * performed. After the first handler has been performed, every
+         * other invocation of handle() will be skipped and return false
+         * up until the next invocation of reset().
+         * 
+         * @param f The action to perform.
+         * @return true If the action has been performed.
+         * @return false If the action has not been performed.
+         */
+        inline bool handle(std::function<void()> f)
+        {
+            if (triggered() && !handled())
+            {
+                std::unique_lock<std::mutex> lock(m_mtx);
+                if (!handled())
+                {
+                    try
+                    {
+                        f();
+                        // note: the action may have forcely resetted
+                        // the signal handler, so we don't want to create
+                        // an inconsistent state
+                        if (triggered())
+                        {
+                            m_handled.store(true, std::memory_order_seq_cst);
+                        }
+                    }
+                    catch (std::exception& e)
+                    {
+                        if (triggered())
+                        {
+                            m_handled.store(true, std::memory_order_seq_cst);
+                        }
+                        throw e;
+                    }
+                    return true;
+                }
+            }
+            return false;
+        }
+
+    private:
+        std::mutex m_mtx;
+        std::atomic<bool> m_triggered;
+        std::atomic<bool> m_handled;
+    };
+};