Fix json bindings on apache reload

Signed-off-by: Bob Weinand <[email protected]>
DataDog · Feb 1, 2024 · 4537681 · 4537681
1 parent 610fb2b
commit 4537681
Show file tree

Hide file tree

Showing 7 changed files with 82 additions and 0 deletions.
diff --git a/ext/ddtrace.c b/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();
 

diff --git a/tests/Common/WebFrameworkTestCase.php b/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.
      *

diff --git a/tests/Integrations/Custom/NotAutoloaded/ApacheReloadTest.php b/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'),
+        ]);
+    }
+}
diff --git a/tests/Sapi/PhpApache/PhpApache.php b/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.

diff --git a/tests/WebServer.php b/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.
      */

diff --git a/zend_abstract_interface/json/json.c b/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)

diff --git a/zend_abstract_interface/json/json.h b/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