[loader] Fix zombie creation (#3683)

* Add a test to assert the loader doesn't create zombies

* Reap child in thread

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>

* Avoid PHP 7.3 syntax

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>

---------

Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
Co-authored-by: Bob Weinand <bob.weinand@datadoghq.com>

Author: iamluc

loader/dd_library_loader.c

@@ -9,6 +9,8 @@
 #include <php_ini.h>
 #include <stdbool.h>
 #include <errno.h>
+#include <pthread.h>
+#include <sys/wait.h>
 #include <main/SAPI.h>
 #include <ext/standard/basic_functions.h>
 
@@ -311,6 +313,12 @@ void ddloader_logf(injected_ext *config, log_level level, const char *format, ..
     va_end(va);
 }
 
+static void *ddloader_reap_child(void *arg) {
+    pid_t pid = (pid_t)(intptr_t)arg;
+    waitpid(pid, NULL, 0);
+    return NULL;
+}
+
 /**
  * @param error The c-string this is pointing to must not exceed 150 bytes
  */
@@ -405,6 +413,13 @@ static void ddloader_telemetryf(telemetry_reason reason, injected_ext *config, c
         return;
     }
     if (pid > 0) {
+        // reap the child in a background thread to avoid leaking it
+        pthread_t reaper;
+        pthread_attr_t attr;
+        pthread_attr_init(&attr);
+        pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
+        pthread_create(&reaper, &attr, ddloader_reap_child, (void *)(intptr_t)pid);
+        pthread_attr_destroy(&attr);
         return;  // parent
     }
 

loader/tests/functional/includes/autoload.php

@@ -4,13 +4,13 @@
 
 set_exception_handler(function ($ex) {
     $trace = $ex->getTrace();
-    $file = $trace[0]['file'] ?: '';
-    $line = $trace[0]['line'] ?: '';
+    $file = isset($trace[0]['file']) ? $trace[0]['file'] : '';
+    $line = isset($trace[0]['line']) ? $trace[0]['line'] : '';
     $stackTrace = basename($file).':'.$line;
 
     if (basename($file) === 'assert.php') {
-        $file2 = $trace[1]['file'] ?: '';
-        $line2 = $trace[1]['line'] ?: '';
+        $file2 = isset($trace[1]['file']) ? $trace[1]['file'] : '';
+        $line2 = isset($trace[1]['line']) ? $trace[1]['line'] : '';
         $stackTrace = basename($file2).':'.$line2.' > '.$stackTrace;
     }
 

loader/tests/functional/test_no_zombie.php

@@ -0,0 +1,73 @@
+<?php
+
+require_once __DIR__."/includes/autoload.php";
+skip_if_php5();
+
+$telemetryLogPath = tempnam(sys_get_temp_dir(), 'test_loader_');
+
+// Build the command to run PHP with the loader
+$cmd = sprintf(
+    'FAKE_FORWARDER_LOG_PATH=%s DD_TELEMETRY_FORWARDER_PATH=%s php -n -dzend_extension=%s -r "sleep(1);"',
+    escapeshellarg($telemetryLogPath),
+    escapeshellarg(__DIR__.'/../../bin/fake_forwarder.sh'),
+    escapeshellarg(getLoaderAbsolutePath())
+);
+
+try {
+    // Start the PHP process in background and get its PID
+    $descriptors = [
+        0 => ['pipe', 'r'],
+        1 => ['pipe', 'w'],
+        2 => ['pipe', 'w'],
+    ];
+
+    $process = proc_open($cmd . ' & echo $!', $descriptors, $pipes);
+    if (!is_resource($process)) {
+        throw new \Exception("Failed to start PHP process");
+    }
+
+    // Get the real PHP PID from the output
+    $firstLine = fgets($pipes[1]);
+    $phpPid = (int)trim($firstLine);
+
+    if ($phpPid <= 0) {
+        throw new \Exception("Failed to get PHP PID");
+    }
+
+    if (debug()) {
+        echo "[debug] PHP PID: $phpPid\n";
+    }
+
+    // Wait for the telemetry fork to happen and complete
+    usleep(300000); // 300ms
+
+    // Check for zombie processes that are children of the PHP process
+    $zombieCheckCmd = sprintf('ps --ppid %d -o pid,state,comm --no-headers 2>/dev/null || echo "NO_CHILDREN"', $phpPid);
+    $zombieOutput = shell_exec($zombieCheckCmd);
+
+    if (debug()) {
+        echo "[debug] Children processes:\n" . $zombieOutput . "\n";
+    }
+
+    $zombieCount = substr_count($zombieOutput, ' Z ');
+
+    // Wait for the PHP process to finish
+    $waitCmd = sprintf('wait %d 2>/dev/null; echo $?', $phpPid);
+    $phpExitCode = (int)trim(shell_exec($waitCmd));
+
+    // Read the remaining output and close pipes
+    $output = stream_get_contents($pipes[1]);
+    $errors = stream_get_contents($pipes[2]);
+    fclose($pipes[0]);
+    fclose($pipes[1]);
+    fclose($pipes[2]);
+    proc_close($process);
+
+    if ($zombieCount > 0) {
+        throw new \Exception("FAILED: Zombie process detected after telemetry fork! Found $zombieCount zombie(s)");
+    }
+
+    echo "OK: No zombie processes detected\n";
+} finally {
+    @unlink($telemetryLogPath);
+}