Exec integration: remove usage of dynamic property

DataDog · Nov 23, 2023 · 154e4a9 · 154e4a9
1 parent 1e3bdb2
commit 154e4a9
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 20 deletions.
diff --git a/ext/integrations/exec_integration.c b/ext/integrations/exec_integration.c
@@ -118,21 +118,7 @@ static void dd_proc_wrapper_rsrc_dtor(zend_resource *rsrc) {
     ddtrace_span_data *span_data = OBJ_SPANDATA(proc_span->span);
 
     if (span_data->duration == 0) {
-        // if we reaped, the proc handle destructor will set
-        // FG(pclose_ret) to -1, causing proc_close to return -1, so we
-        // must signal that we have to override the return value in the post
-        // hook of proc_close
-        bool did_reap = dd_waitpid(span_data, proc_span->child);
-        if (did_reap) {
-            // set a dynamic property on the span
-#if PHP_VERSION_ID < 80000
-            zval zobj;
-            ZVAL_OBJ(&zobj, proc_span->span);
-            zend_update_property_bool(ddtrace_ce_span_data, &zobj, ZEND_STRL("overrideRetval"), 1);
-#else
-            zend_update_property_bool(ddtrace_ce_span_data, proc_span->span, ZEND_STRL("overrideRetval"), 1);
-#endif
-        }
+        (void)dd_waitpid(span_data, proc_span->child);
 
         dd_trace_stop_span_time(span_data);
         ddtrace_close_span_restore_stack(span_data);

diff --git a/src/Integrations/Integrations/Exec/ExecIntegration.php b/src/Integrations/Integrations/Exec/ExecIntegration.php
@@ -167,9 +167,21 @@ static function (HookData $hook) {
                     return;
                 }
 
-                // the span must be retrieved from the resource because by the time
-                // the post hook runs, the resource has already been destroyed
-                $hook->data = proc_get_span($hook->args[0]);
+                // the span must be stored in $hook because by the time the post
+                //hook runs, the resource has already been destroyed
+                $span = proc_get_span($hook->args[0]);
+                if (!$span) {
+                    return;
+                }
+                // must match condition in dd_proc_wrapper_rsrc_dtor before
+                // calling dd_waitpid()
+                if ($span->getDuration() != 0) {
+                    return;
+                }
+                // if we get here, we will call waitpid() in the resource
+                // destructor and very likely reap the process, resulting in
+                // proc_close() returning -1
+                $hook->data = $span;
             },
             static function (HookData $hook) {
                 /** @var SpanData $span */
@@ -178,9 +190,8 @@ static function (HookData $hook) {
                     return;
                 }
 
-                if (isset($span->overrideRetval) && key_exists(Tag::EXEC_EXIT_CODE, $span->meta)) {
+                if ($hook->returned === -1 && isset($span->meta[Tag::EXEC_EXIT_CODE])) {
                     $hook->overrideReturnValue($span->meta[Tag::EXEC_EXIT_CODE]);
-                    unset($span->overrideRetval);
                 }
             }
         );