refactor finding EG

reliforp · Sep 26, 2021 · 59c4207 · 59c4207
1 parent 5cbecec
commit 59c4207
Show file tree

Hide file tree

Showing 6 changed files with 93 additions and 51 deletions.
diff --git a/src/Command/Inspector/GetCurrentFunctionNameCommand.php b/src/Command/Inspector/GetCurrentFunctionNameCommand.php
@@ -13,6 +13,7 @@
 
 namespace PhpProfiler\Command\Inspector;
 
+use PhpProfiler\Inspector\RetryingLoopProvider;
 use PhpProfiler\Inspector\Settings\InspectorSettingsException;
 use PhpProfiler\Inspector\Settings\TargetPhpSettings\TargetPhpSettingsFromConsoleInput;
 use PhpProfiler\Inspector\Settings\TargetProcessSettings\TargetProcessSettingsFromConsoleInput;
@@ -39,6 +40,7 @@ public function __construct(
         private TargetProcessSettingsFromConsoleInput $target_process_settings_from_console_input,
         private TraceLoopSettingsFromConsoleInput $trace_loop_settings_from_console_input,
         private TargetProcessResolver $target_process_resolver,
+        private RetryingLoopProvider $retrying_loop_provider,
     ) {
         parent::__construct();
     }
@@ -68,25 +70,20 @@ public function execute(InputInterface $input, OutputInterface $output): int
 
         $process_specifier = $this->target_process_resolver->resolve($target_process_settings);
 
-        $last_exception = null;
-        for ($i = 0; $i < 10; $i++) {
-            try {
+        // see the comment at GetTraceCommand::execute()
+        $eg_address = null;
+        $this->retrying_loop_provider->do(
+            try: function () use (&$eg_address, $process_specifier, $target_php_settings) {
                 $eg_address = $this->php_globals_finder->findExecutorGlobals(
                     $process_specifier,
                     $target_php_settings
                 );
-                break;
-            } catch (\Throwable $e) {
-                $last_exception = $e;
-                /** @psalm-suppress UnusedFunctionCall */
-                \time_nanosleep(0, 1000 * 1000 * 10);
-                continue;
-            }
-        }
-        if ($i === 10) {
-            assert(isset($last_exception) and $last_exception instanceof \Exception);
-            throw new \Exception('cannot find executor globals', 0, $last_exception);
-        }
+            },
+            retry_on: [\Throwable::class],
+            max_retry: 10,
+            interval_on_retry_ns: 1000 * 1000 * 10,
+        );
+        assert(is_int($eg_address));
 
         $this->loop_provider->getMainLoop(
             function () use ($process_specifier, $target_php_settings, $eg_address, $output): bool {

diff --git a/src/Command/Inspector/GetEgAddressCommand.php b/src/Command/Inspector/GetEgAddressCommand.php
@@ -13,6 +13,7 @@
 
 namespace PhpProfiler\Command\Inspector;
 
+use PhpProfiler\Inspector\RetryingLoopProvider;
 use PhpProfiler\Inspector\Settings\InspectorSettingsException;
 use PhpProfiler\Inspector\Settings\TargetPhpSettings\TargetPhpSettingsFromConsoleInput;
 use PhpProfiler\Inspector\Settings\TargetProcessSettings\TargetProcessSettingsFromConsoleInput;
@@ -36,6 +37,7 @@ public function __construct(
         private TargetPhpSettingsFromConsoleInput $target_php_settings_from_console_input,
         private TargetProcessSettingsFromConsoleInput $target_process_settings_from_console_input,
         private TargetProcessResolver $target_process_resolver,
+        private RetryingLoopProvider $retrying_loop_provider,
     ) {
         parent::__construct();
     }
@@ -63,25 +65,20 @@ public function execute(InputInterface $input, OutputInterface $output): int
 
         $process_specifier = $this->target_process_resolver->resolve($target_process_settings);
 
-        $last_exception = null;
-        for ($i = 0; $i < 10; $i++) {
-            try {
+        // see the comment at GetTraceCommand::execute()
+        $eg_address = null;
+        $this->retrying_loop_provider->do(
+            try: function () use (&$eg_address, $process_specifier, $target_php_settings) {
                 $eg_address = $this->php_globals_finder->findExecutorGlobals(
                     $process_specifier,
                     $target_php_settings
                 );
-                break;
-            } catch (\Throwable $e) {
-                $last_exception = $e;
-                /** @psalm-suppress UnusedFunctionCall */
-                \time_nanosleep(0, 1000 * 1000 * 10);
-                continue;
-            }
-        }
-        if ($i === 10) {
-            assert(isset($last_exception) and $last_exception instanceof \Exception);
-            throw new \Exception('cannot find executor globals', 0, $last_exception);
-        }
+            },
+            retry_on: [\Throwable::class],
+            max_retry: 10,
+            interval_on_retry_ns: 1000 * 1000 * 10,
+        );
+        assert(is_int($eg_address));
 
         $output->writeln(
             sprintf(

diff --git a/src/Command/Inspector/GetTraceCommand.php b/src/Command/Inspector/GetTraceCommand.php
@@ -14,6 +14,7 @@
 namespace PhpProfiler\Command\Inspector;
 
 use PhpProfiler\Inspector\Output\TraceFormatter\Templated\TemplatedTraceFormatterFactory;
+use PhpProfiler\Inspector\RetryingLoopProvider;
 use PhpProfiler\Inspector\Settings\GetTraceSettings\GetTraceSettingsFromConsoleInput;
 use PhpProfiler\Inspector\Settings\InspectorSettingsException;
 use PhpProfiler\Inspector\Settings\TargetPhpSettings\TargetPhpSettingsFromConsoleInput;
@@ -49,6 +50,7 @@ public function __construct(
         private TemplatedTraceFormatterFactory $templated_trace_formatter_factory,
         private ProcessStopper $process_stopper,
         private TargetProcessResolver $target_process_resolver,
+        private RetryingLoopProvider $retrying_loop_provider,
     ) {
         parent::__construct();
     }
@@ -83,25 +85,22 @@ public function execute(InputInterface $input, OutputInterface $output): int
 
         $process_specifier = $this->target_process_resolver->resolve($target_process_settings);
 
-        $last_exception = null;
-        for ($i = 0; $i < 10; $i++) {
-            try {
+        // On targeting ZTS, it's possible that libpthread.so of the target process isn't yet loaded
+        // at this point. In that case the TLS block can't be located, then the address of EG can't
+        // be found also. So simply retrying the whole process of finding EG here.
+        $eg_address = null;
+        $this->retrying_loop_provider->do(
+            try: function () use (&$eg_address, $process_specifier, $target_php_settings) {
                 $eg_address = $this->php_globals_finder->findExecutorGlobals(
                     $process_specifier,
                     $target_php_settings
                 );
-                break;
-            } catch (\Throwable $e) {
-                $last_exception = $e;
-                /** @psalm-suppress UnusedFunctionCall */
-                \time_nanosleep(0, 1000 * 1000 * 10);
-                continue;
-            }
-        }
-        if ($i === 10) {
-            assert(isset($last_exception) and $last_exception instanceof \Exception);
-            throw new \Exception('cannot find executor globals', 0, $last_exception);
-        }
+            },
+            retry_on: [\Throwable::class],
+            max_retry: 10,
+            interval_on_retry_ns: 1000 * 1000 * 10,
+        );
+        assert(is_int($eg_address));
 
         $this->loop_provider->getMainLoop(
             function () use (
@@ -113,7 +112,6 @@ function () use (
                 $output,
                 $formatter
             ): bool {
-
                 if ($loop_settings->stop_process and $this->process_stopper->stop($process_specifier->pid)) {
                     defer($_, fn () => $this->process_stopper->resume($process_specifier->pid));
                 }

diff --git a/src/Inspector/RetryingLoopProvider.php b/src/Inspector/RetryingLoopProvider.php
@@ -0,0 +1,48 @@
+<?php
+
+/**
+ * This file is part of the sj-i/php-profiler package.
+ *
+ * (c) sji <sji@sj-i.dev>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace PhpProfiler\Inspector;
+
+use PhpProfiler\Lib\Loop\LoopBuilder;
+use PhpProfiler\Lib\Loop\LoopMiddleware\CallableMiddleware;
+use PhpProfiler\Lib\Loop\LoopMiddleware\NanoSleepMiddleware;
+use PhpProfiler\Lib\Loop\LoopMiddleware\RetryOnExceptionMiddleware;
+
+class RetryingLoopProvider
+{
+    public function __construct(
+        private LoopBuilder $loop_builder
+    ) {
+    }
+
+    /** @param class-string<\Throwable>[] $retry_on */
+    public function do(
+        callable $try,
+        array $retry_on,
+        int $max_retry,
+        int $interval_on_retry_ns,
+    ): void {
+        // one successful execution is enough
+        $loop_canceller = function () use ($try): bool {
+            $try();
+            return false;
+        };
+
+        $this->loop_builder
+            ->addProcess(RetryOnExceptionMiddleware::class, [$max_retry, $retry_on])
+            ->addProcess(NanoSleepMiddleware::class, [$interval_on_retry_ns])
+            ->addProcess(CallableMiddleware::class, [$loop_canceller])
+            ->build()
+            ->invoke();
+    }
+}
diff --git a/src/Lib/Loop/LoopMiddleware/RetryOnExceptionMiddleware.php b/src/Lib/Loop/LoopMiddleware/RetryOnExceptionMiddleware.php
@@ -37,10 +37,12 @@ public function invoke(): bool
                 $result = $this->chain->invoke();
                 $this->current_retry_count = 0;
                 return $result;
-            } catch (Exception $e) {
-                if (in_array(get_class($e), $this->exception_names, true)) {
-                    $this->current_retry_count++;
-                    continue;
+            } catch (\Throwable $e) {
+                foreach ($this->exception_names as $exception_name) {
+                    if (is_a($e, $exception_name)) {
+                        $this->current_retry_count++;
+                        continue 2;
+                    }
                 }
                 throw $e;
             }

diff --git a/src/Lib/Process/Exec/TraceeExecutor.php b/src/Lib/Process/Exec/TraceeExecutor.php
@@ -20,7 +20,7 @@
 use PhpProfiler\Lib\Process\Exec\Internal\Pcntl;
 use PhpProfiler\Lib\System\OnShutdown;
 
-final class TraceeExecutor
+class TraceeExecutor
 {
     public function __construct(
         private Pcntl $pcntl,