Fix json bindings on apache reload

Signed-off-by: Bob Weinand <[email protected]>

Author: bwoebi

appsec/src/extension/ddappsec.c

@@ -41,6 +41,8 @@
 #include "tags.h"
 #include "user_tracking.h"
 
+#include <json/json.h>
+
 #if ZTS
 static atomic_int _thread_count;
 #endif
@@ -171,10 +173,12 @@ static PHP_GSHUTDOWN_FUNCTION(ddappsec)
     if (prev == 1) {
         dd_log_shutdown();
         zai_config_mshutdown();
+        zai_json_shutdown_bindings();
     }
 #else
     dd_log_shutdown();
     zai_config_mshutdown();
+    zai_json_shutdown_bindings();
 #endif
 
     memset(ddappsec_globals, '\0', sizeof(*ddappsec_globals)); // NOLINT

ext/ddtrace.c

@@ -995,6 +995,7 @@ static PHP_MSHUTDOWN_FUNCTION(ddtrace) {
 
     if (DDTRACE_G(disable) == 1) {
         zai_config_mshutdown();
+        zai_json_shutdown_bindings();
         return SUCCESS;
     }
 
@@ -1021,6 +1022,7 @@ static PHP_MSHUTDOWN_FUNCTION(ddtrace) {
     ddtrace_shutdown_span_sampling_limiter();
     ddtrace_limiter_destroy();
     zai_config_mshutdown();
+    zai_json_shutdown_bindings();
 
     ddtrace_user_req_shutdown();
 

profiling/src/lib.rs

@@ -839,9 +839,10 @@ extern "C" fn shutdown(_extension: *mut ZendExtension) {
     // data race condition.
     unsafe { config::shutdown() };
 
-    // SAFETY: zai_config_mshutdown should be safe to cal in shutdown instead
+    // SAFETY: zai_config_mshutdown should be safe to call in shutdown instead
     // of mshutdown.
     unsafe { bindings::zai_config_mshutdown() };
+    unsafe { bindings::zai_json_shutdown_bindings() };
 }
 
 /// Notifies the profiler a trace has finished so it can update information

profiling/src/php_ffi.h

@@ -17,6 +17,8 @@
 
 // Profiling needs ZAI config for INI support.
 #include <config/config.h>
+// And json to cleanup json state for graceful restart
+#include <json/json.h>
 
 // Used to communicate strings from C -> Rust.
 #include <zai_string/string.h>

tests/Common/WebFrameworkTestCase.php

@@ -140,6 +140,15 @@ private static function tearDownWebServer()
         }
     }
 
+    protected function reloadAppServer()
+    {
+        if (\method_exists(self::$appServer, "reload")) {
+            self::$appServer->reload();
+        } else {
+            $this->markTestSkipped("Webserver reload not supported");
+        }
+    }
+
     /**
      * Executed a call to the test web server.
      *

tests/Integrations/Custom/NotAutoloaded/ApacheReloadTest.php

@@ -0,0 +1,43 @@
+<?php
+
+
+use DDTrace\Tests\Common\SpanAssertion;
+use DDTrace\Tests\Common\WebFrameworkTestCase;
+use DDTrace\Tests\Frameworks\Util\Request\GetSpec;
+
+final class ApacheReloadTest extends WebFrameworkTestCase
+{
+    protected static function getAppIndexScript()
+    {
+        return __DIR__ . '/../../../Frameworks/Custom/Version_Not_Autoloaded/index.php';
+    }
+
+    protected static function getEnvs()
+    {
+        return array_merge(parent::getEnvs(), [
+            'APP_NAME' => 'custom_no_autoloaded_app',
+            'DD_TRACE_NO_AUTOLOADER' => 'true',
+        ]);
+    }
+
+    public function testWebserverReload()
+    {
+        $traces = $this->tracesFromWebRequest(function () {
+            $this->call(GetSpec::create('A web request to a framework not using auto loaders', '/'));
+        });
+
+        $this->assertSpans($traces, [
+            SpanAssertion::exists('web.request'),
+        ]);
+
+        $this->reloadAppServer();
+
+        $traces = $this->tracesFromWebRequest(function () {
+            $this->call(GetSpec::create('A web request to a framework not using auto loaders', '/'));
+        });
+
+        $this->assertSpans($traces, [
+            SpanAssertion::exists('web.request'),
+        ]);
+    }
+}

tests/Sapi/PhpApache/PhpApache.php

@@ -149,6 +149,12 @@ public function start()
         $this->configChanged = false;
     }
 
+    public function reload()
+    {
+        $this->process->signal(SIGUSR1);
+        usleep(10000);
+    }
+
     public function stop()
     {
         // I do not understand why we get a SIGTERM when we try to send a SIGTERM to apache.

tests/WebServer.php

@@ -9,6 +9,7 @@
 use DDTrace\Tests\Sapi\PhpFpm\PhpFpm;
 use DDTrace\Tests\Sapi\Roadrunner\RoadrunnerServer;
 use DDTrace\Tests\Sapi\Sapi;
+use PHPUnit\Framework\Assert;
 
 /**
  * A controllable php server running in a separate process.
@@ -180,6 +181,15 @@ public function start()
         usleep(500000);
     }
 
+    public function reload()
+    {
+        if (\method_exists($this->sapi, "reload")) {
+            $this->sapi->reload();
+        } else {
+            Assert::markTestSkipped("Webserver reload not supported");
+        }
+    }
+
     /**
      * Teardown promptly.
      */

zend_abstract_interface/json/json.c

@@ -89,6 +89,8 @@ __attribute__((weak)) int php_json_encode(smart_str *buf, zval *val, int options
 __attribute__((weak)) zend_class_entry *php_json_serializable_ce;
 #endif
 
+static bool zai_json_dynamic_bindings = false;
+
 bool zai_json_setup_bindings(void) {
     if (php_json_encode && php_json_serializable_ce) {
         zai_json_encode = php_json_encode;
@@ -105,6 +107,8 @@ bool zai_json_setup_bindings(void) {
 
     if (!json_me) return false;
 
+    zai_json_dynamic_bindings = true;
+
     zai_json_encode = DL_FETCH_SYMBOL(json_me->handle, "php_json_encode");
     if (zai_json_encode == NULL) {
         zai_json_encode = DL_FETCH_SYMBOL(json_me->handle, "_php_json_encode");
@@ -138,6 +142,13 @@ bool zai_json_setup_bindings(void) {
     return zai_json_encode != NULL;
 }
 
+void zai_json_shutdown_bindings(void) {
+    if (zai_json_dynamic_bindings) {
+        zai_json_dynamic_bindings = false;
+        php_json_serializable_ce = NULL;
+    }
+}
+
 void zai_json_release_persistent_array(HashTable *ht) {
 #if PHP_VERSION_ID < 70300
     if (--GC_REFCOUNT(ht) == 0)

zend_abstract_interface/json/json.h

@@ -45,5 +45,6 @@ extern __attribute__((weak)) zend_class_entry *php_json_serializable_ce;
 void zai_json_release_persistent_array(HashTable *ht);
 void zai_json_dtor_pzval(zval *pval);
 bool zai_json_setup_bindings(void);
+void zai_json_shutdown_bindings(void);
 
 #endif  // ZAI_JSON_H