Fixed fork handling (#12)

Author: intuibase

prod/native/extension/code/ForkHandler.cpp

@@ -7,42 +7,34 @@
 #include "os/OsUtils.h"
 #include "LoggerInterface.h"
 #include "ModuleGlobals.h"
-#include "PeriodicTaskExecutor.h"
-#include "transport/HttpTransportAsync.h"
+#include "ForkableRegistry.h"
 
-static void callbackToLogForkBeforeInParent() {
+namespace opentelemetry::php {
+
+static void beforeFork() {
     ELOGF_NF_DEBUG(OTEL_GL(logger_), "Before process fork (i.e., in parent context); its parent (i.e., grandparent) PID: %d", static_cast<int>(opentelemetry::osutils::getParentProcessId()));
     // TODO implement forkable registry
-    if (OTEL_G(globals) && OTEL_G(globals)->periodicTaskExecutor_) {
-        OTEL_G(globals)->periodicTaskExecutor_->prefork();
-    }
-    if (OTEL_G(globals) && OTEL_G(globals)->httpTransportAsync_) {
-        OTEL_G(globals)->httpTransportAsync_->prefork();
+    if (OTEL_G(globals) && OTEL_G(globals)->forkableRegistry_) {
+        OTEL_G(globals)->forkableRegistry_->preFork();
     }
 }
 
-static void callbackToLogForkAfterInParent() {
+static void afterForkInParent() {
     ELOGF_NF_DEBUG(OTEL_GL(logger_), "After process fork (in parent context)");
-    if (OTEL_G(globals) && OTEL_G(globals)->periodicTaskExecutor_) {
-        OTEL_G(globals)->periodicTaskExecutor_->postfork(false);
-    }
-    if (OTEL_G(globals) && OTEL_G(globals)->httpTransportAsync_) {
-        OTEL_G(globals)->httpTransportAsync_->postfork(false);
+    if (OTEL_G(globals) && OTEL_G(globals)->forkableRegistry_) {
+        OTEL_G(globals)->forkableRegistry_->postFork(false);
     }
 }
 
-static void callbackToLogForkAfterInChild() {
+static void afterForkInChild() {
     ELOGF_NF_DEBUG(OTEL_GL(logger_), "After process fork (in child context); parent PID: %d", static_cast<int>(opentelemetry::osutils::getParentProcessId()));
-    if (OTEL_G(globals) && OTEL_G(globals)->periodicTaskExecutor_) {
-        OTEL_G(globals)->periodicTaskExecutor_->postfork(true);
-    }
-    if (OTEL_G(globals) && OTEL_G(globals)->httpTransportAsync_) {
-        OTEL_G(globals)->httpTransportAsync_->postfork(true);
+    if (OTEL_G(globals) && OTEL_G(globals)->forkableRegistry_) {
+        OTEL_G(globals)->forkableRegistry_->postFork(true);
     }
 }
 
-void registerCallbacksToLogFork() {
-    int retVal = pthread_atfork(callbackToLogForkBeforeInParent, callbackToLogForkAfterInParent, callbackToLogForkAfterInChild);
+void registerCallbacksToHandleFork() {
+    int retVal = pthread_atfork(beforeFork, afterForkInParent, afterForkInChild);
     if (retVal == 0) {
         ELOGF_NF_DEBUG(OTEL_GL(logger_), "Registered callbacks to log process fork");
     } else {
@@ -51,6 +43,7 @@ void registerCallbacksToLogFork() {
 }
 
 #else
-void registerCallbacksToLogFork() {
+void registerCallbacksToHandleFork() {
+}
+#endif
 }
-#endif

prod/native/extension/code/ForkHandler.h

@@ -1,3 +1,7 @@
 #pragma once
 
-void registerCallbacksToLogFork();
+namespace opentelemetry::php {
+
+void registerCallbacksToHandleFork();
+
+}

prod/native/extension/code/ModuleInit.cpp

@@ -94,6 +94,9 @@ void moduleInit(int moduleType, int moduleNumber) {
     if (php_check_open_basedir_ex(OTEL_GL(config_)->get(&opentelemetry::php::ConfigurationSnapshot::bootstrap_php_part_file).c_str(), false) != 0) {
         ELOGF_WARNING(globals->logger_, MODULE, "OpenTelemetry PHP distro bootstrap file (%s) is located outside of paths allowed by open_basedir ini setting.", OTEL_GL(config_)->get(&opentelemetry::php::ConfigurationSnapshot::bootstrap_php_part_file).c_str());
     }
+
+    // Registering fork handlers in module init to ensure that we will handle all forks properly - even those triggered before request start (e.g. in MINIT of other extensions) or when fpm/apache spawns worker processes. We cannot be sure that some of the classes implementing ForkableInterface will not start thread before request start.
+    registerCallbacksToHandleFork();
 }
 
 void moduleShutdown( int moduleType, int moduleNumber ) {

prod/native/libcommon/code/AgentGlobals.cpp

@@ -2,6 +2,7 @@
 
 #include "PhpBridgeInterface.h"
 #include "SharedMemoryState.h"
+#include "ForkableRegistry.h"
 #include "InferredSpans.h"
 #include "PeriodicTaskExecutor.h"
 #include "PeriodicTaskExecutor.h"
@@ -32,6 +33,7 @@ AgentGlobals::AgentGlobals(std::shared_ptr<LoggerInterface> logger,
         std::shared_ptr<InstrumentedFunctionHooksStorageInterface> hooksStorage,
         std::shared_ptr<InferredSpans> inferredSpans,
         ConfigurationStorage::configUpdate_t updateConfigurationSnapshot) :
+    forkableRegistry_(std::make_shared<ForkableRegistry>()),
     config_(std::make_shared<opentelemetry::php::ConfigurationStorage>(std::move(updateConfigurationSnapshot))),
     logger_(std::move(logger)),
     logSinkStdErr_(std::move(logSinkStdErr)),
@@ -52,6 +54,9 @@ AgentGlobals::AgentGlobals(std::shared_ptr<LoggerInterface> logger,
     coordinatorConfigProvider_(std::make_shared<opentelemetry::php::coordinator::CoordinatorConfigurationProvider>(logger_, opAmp_)),
     coordinatorProcess_(std::make_shared<opentelemetry::php::coordinator::CoordinatorProcess>(logger_, messagesDispatcher_, coordinatorConfigProvider_))
     {
+        forkableRegistry_->registerForkable(httpTransportAsync_);
+        forkableRegistry_->registerForkable(opAmp_);
+
         config_->addConfigUpdateWatcher([logger = logger_, stderrsink = logSinkStdErr_, syslogsink = logSinkSysLog_, filesink = logSinkFile_](ConfigurationSnapshot const &cfg) {
             stderrsink->setLevel(cfg.log_level_stderr);
             syslogsink->setLevel(cfg.log_level_syslog);
@@ -95,8 +100,10 @@ std::shared_ptr<PeriodicTaskExecutor> AgentGlobals::getPeriodicTaskExecutor() {
                 opentelemetry::utils::blockSignal(SIGPROF); // php timeout signal
             }
         );
-        return periodicTaskExecutor_;
-    }
+    forkableRegistry_->registerForkable(periodicTaskExecutor_);
+
+    return periodicTaskExecutor_;
+}
 
 
 }

prod/native/libcommon/code/AgentGlobals.h

@@ -17,6 +17,7 @@ class OpAmp;
 
 namespace opentelemetry::php {
 
+class ForkableRegistry;
 class LoggerInterface;
 class PhpBridgeInterface;
 class InferredSpans;
@@ -58,6 +59,7 @@ class AgentGlobals {
 
     std::shared_ptr<PeriodicTaskExecutor> getPeriodicTaskExecutor();
 
+    std::shared_ptr<ForkableRegistry> forkableRegistry_;
     std::shared_ptr<ConfigurationStorage> config_;
     std::shared_ptr<LoggerInterface> logger_;
     std::shared_ptr<LoggerSinkInterface> logSinkStdErr_;

prod/native/libcommon/code/ForkableRegistry.h

@@ -0,0 +1,62 @@
+#pragma once
+
+#include "ForkableInterface.h"
+
+#include <memory>
+#include <vector>
+
+namespace opentelemetry::php {
+
+/**
+ * @class ForkableRegistry
+ * @brief A registry for managing fork-aware objects in a PHP extension.
+ *
+ * This class is NOT thread-safe. Registration of forkable objects should only be performed
+ * during extension initialization or within the Zend Engine's main execution thread (the primary PHP thread).
+ * Attempting to register forkables from other threads may lead to unpredictable behavior and race conditions.
+ *
+ * The deliberate absence of mutex protection is intentional: adding synchronization primitives could
+ * introduce deadlock scenarios during process forking. Specifically, if another thread holds a mutex
+ * lock during the pre-fork phase, the child process would inherit a locked mutex that can never be
+ * unlocked (since the thread that locked it doesn't exist in the child process), resulting in a deadlock.
+ *
+ * @note Thread Safety: This class is not thread-safe by design. All operations must be performed
+ *       from the main PHP thread.
+ * @note Fork Safety: The class provides pre-fork and post-fork hooks to ensure proper state
+ *       management across process boundaries.
+ */
+class ForkableRegistry {
+public:
+    ForkableRegistry() = default;
+    ~ForkableRegistry() = default;
+
+    ForkableRegistry(const ForkableRegistry &) = delete;
+    ForkableRegistry &operator=(const ForkableRegistry &) = delete;
+    ForkableRegistry(ForkableRegistry &&) = delete;
+    ForkableRegistry &operator=(ForkableRegistry &&) = delete;
+
+    void registerForkable(std::shared_ptr<ForkableInterface> forkable) {
+        forkables_.emplace_back(std::move(forkable));
+    }
+
+    void preFork() {
+        for (const auto &forkable : forkables_) {
+            if (forkable) {
+                forkable->prefork();
+            }
+        }
+    }
+
+    void postFork(bool isChild) {
+        for (const auto &forkable : forkables_) {
+            if (forkable) {
+                forkable->postfork(isChild);
+            }
+        }
+    }
+
+private:
+    std::vector<std::shared_ptr<ForkableInterface>> forkables_;
+};
+
+}