From dece386cae8a27df9ede4d9f1c177cf6af3ab7c4 Mon Sep 17 00:00:00 2001 From: Gustavo Lopes Date: Fri, 17 Nov 2023 15:18:00 +0000 Subject: [PATCH] Exec integration: remove usage of dynamic property --- ext/integrations/exec_integration.c | 16 +------------- .../Integrations/Exec/ExecIntegration.php | 21 ++++++++++++++----- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/ext/integrations/exec_integration.c b/ext/integrations/exec_integration.c index 8e53ef36c96..39eb9dfd245 100644 --- 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 index b2aecb2838a..4547f75dc46 100644 --- 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); } } );