Merge pull request #52 from sj-i/improve-stability

Improve stability
reliforp · Aug 12, 2021 · cefdd75 · cefdd75
2 parents e847fb5 + bfa3423
commit cefdd75
Show file tree

Hide file tree

Showing 32 changed files with 384 additions and 127 deletions.
diff --git a/composer.json b/composer.json
@@ -41,6 +41,7 @@
     "php-coveralls/php-coveralls": "^2.3"
   },
   "autoload": {
+    "files": ["src/Lib/Defer/defer.php"],
     "psr-4": {
       "PhpProfiler\\": "src"
     }

diff --git a/config/di.php b/config/di.php
@@ -26,8 +26,8 @@
 use PhpProfiler\Lib\ByteStream\IntegerByteSequence\LittleEndianReader;
 use PhpProfiler\Lib\Elf\SymbolResolver\Elf64SymbolResolverCreator;
 use PhpProfiler\Lib\Elf\SymbolResolver\SymbolResolverCreatorInterface;
-use PhpProfiler\Lib\File\CatFileReader;
 use PhpProfiler\Lib\File\FileReaderInterface;
+use PhpProfiler\Lib\File\NativeFileReader;
 use PhpProfiler\Lib\Log\StateCollector\CallerStateCollector;
 use PhpProfiler\Lib\Log\StateCollector\GroupedStateCollector;
 use PhpProfiler\Lib\Log\StateCollector\ProcessStateCollector;
@@ -46,7 +46,7 @@
         return new ZendTypeReader(ZendTypeReader::V80);
     },
     SymbolResolverCreatorInterface::class => autowire(Elf64SymbolResolverCreator::class),
-    FileReaderInterface::class => autowire(CatFileReader::class),
+    FileReaderInterface::class => autowire(NativeFileReader::class),
     IntegerByteSequenceReader::class => autowire(LittleEndianReader::class),
     ContextCreator::class => autowire()
         ->constructorParameter('di_config_file', __DIR__ . '/di.php'),
@@ -70,6 +70,8 @@
                 Logger::toMonologLevel($config->get('log.level'))
             )
         );
+        $handler->setFormatter(new JsonFormatter());
+        $logger->pushHandler($handler);
         return $logger;
     }
 ];
diff --git a/src/Command/Inspector/DaemonCommand.php b/src/Command/Inspector/DaemonCommand.php
@@ -28,6 +28,7 @@
 use PhpProfiler\Inspector\Settings\TemplatedTraceFormatterSettings\TemplateSettingsFromConsoleInput;
 use PhpProfiler\Inspector\Settings\TraceLoopSettings\TraceLoopSettingsFromConsoleInput;
 use PhpProfiler\Lib\Console\EchoBackCanceller;
+use PhpProfiler\Lib\Log\Log;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -111,8 +112,14 @@ function (string $watcher_id, $stream) {
             $promises = [];
             $promises[] = call(function () use ($searcher_context, $dispatch_table) {
                 while (1) {
+                    Log::debug('receiving pid List');
                     $update_target_message = yield $searcher_context->receivePidList();
+                    Log::debug('update targets', [
+                        'update' => $update_target_message->target_process_list->getArray(),
+                        'current' => $dispatch_table->worker_pool->debugDump(),
+                    ]);
                     $dispatch_table->updateTargets($update_target_message->target_process_list);
+                    Log::debug('target updated', [$dispatch_table->worker_pool->debugDump()]);
                 }
             });
             foreach ($worker_pool->getWorkers() as $reader) {
@@ -123,6 +130,7 @@ function () use ($reader, $dispatch_table, $output, $formatter) {
                             if ($result instanceof TraceMessage) {
                                 $this->outputTrace($output, $formatter, $result);
                             } else {
+                                Log::debug('releaseOne', [$result]);
                                 $dispatch_table->releaseOne($result->pid);
                             }
                         }

diff --git a/src/Command/Inspector/GetTraceCommand.php b/src/Command/Inspector/GetTraceCommand.php
@@ -32,6 +32,8 @@
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 
+use function PhpProfiler\Lib\Defer\defer;
+
 final class GetTraceCommand extends Command
 {
     public function __construct(
@@ -91,20 +93,18 @@ function () use (
                 $output,
                 $formatter
             ): bool {
-                $is_target_stopped = false;
-                if ($loop_settings->stop_process) {
-                    $is_target_stopped = $this->process_stopper->stop($target_process_settings->pid);
+                if ($loop_settings->stop_process and $this->process_stopper->stop($target_process_settings->pid)) {
+                    defer($_, fn () => $this->process_stopper->resume($target_process_settings->pid));
                 }
                 $call_trace = $this->executor_globals_reader->readCallTrace(
                     $target_process_settings->pid,
                     $target_php_settings->php_version,
                     $eg_address,
                     $get_trace_settings->depth
                 );
-                if ($loop_settings->stop_process and $is_target_stopped) {
-                    $this->process_stopper->resume($target_process_settings->pid);
+                if (!is_null($call_trace)) {
+                    $output->write($formatter->format($call_trace) . PHP_EOL);
                 }
-                $output->write($formatter->format($call_trace) . PHP_EOL);
                 return true;
             },
             $loop_settings

diff --git a/src/Inspector/Daemon/Dispatcher/DispatchTable.php b/src/Inspector/Daemon/Dispatcher/DispatchTable.php
@@ -59,9 +59,11 @@ public function release(TargetProcessListInterface $targets): void
 
     public function releaseOne(int $pid): void
     {
-        $worker = $this->dispatch_table[$pid];
-        $this->worker_pool->returnWorkerToPool($worker);
-        unset($this->dispatch_table[$pid]);
+        if (isset($this->dispatch_table[$pid])) {
+            $worker = $this->dispatch_table[$pid];
+            $this->worker_pool->returnWorkerToPool($worker);
+            unset($this->dispatch_table[$pid]);
+        }
         $this->assigned = $this->assigned->getDiff(new TargetProcessList($pid));
     }
 }
diff --git a/src/Inspector/Daemon/Dispatcher/WorkerPool.php b/src/Inspector/Daemon/Dispatcher/WorkerPool.php
@@ -96,4 +96,12 @@ public function returnWorkerToPool(PhpReaderControllerInterface $context_to_retu
             }
         }
     }
+
+    public function debugDump(): array
+    {
+        return [
+            'all' => array_keys($this->contexts),
+            'is_free_list' => $this->is_free_list,
+        ];
+    }
 }
diff --git a/src/Inspector/Daemon/Dispatcher/WorkerPoolInterface.php b/src/Inspector/Daemon/Dispatcher/WorkerPoolInterface.php
@@ -25,4 +25,6 @@ public function returnWorkerToPool(PhpReaderControllerInterface $context_to_retu
      * @return iterable<int, PhpReaderControllerInterface>
      */
     public function getWorkers(): iterable;
+
+    public function debugDump(): array;
 }
diff --git a/src/Inspector/Daemon/Reader/Worker/PhpReaderEntryPoint.php b/src/Inspector/Daemon/Reader/Worker/PhpReaderEntryPoint.php
@@ -19,13 +19,13 @@
 use PhpProfiler\Inspector\Daemon\Reader\Protocol\PhpReaderWorkerProtocolInterface;
 use PhpProfiler\Inspector\Settings\TargetProcessSettings\TargetProcessSettings;
 use PhpProfiler\Lib\Amphp\WorkerEntryPointInterface;
-use PhpProfiler\Lib\Process\MemoryReader\MemoryReaderException;
+use PhpProfiler\Lib\Log\Log;
 
 final class PhpReaderEntryPoint implements WorkerEntryPointInterface
 {
     public function __construct(
         private PhpReaderTraceLoopInterface $trace_loop,
-        private PhpReaderWorkerProtocolInterface $protocol
+        private PhpReaderWorkerProtocolInterface $protocol,
     ) {
     }
 
@@ -36,13 +36,15 @@ public function run(): \Generator
          * @var SetSettingsMessage $set_settings_message
          */
         $set_settings_message = yield $this->protocol->receiveSettings();
+        Log::debug('settings_message', [$set_settings_message]);
 
         while (1) {
             /**
              * @psalm-ignore-var
              * @var AttachMessage $attach_message
              */
             $attach_message = yield $this->protocol->receiveAttach();
+            Log::debug('attach_message', [$attach_message]);
 
             $target_process_settings = new TargetProcessSettings(
                 $attach_message->pid
@@ -55,16 +57,23 @@ public function run(): \Generator
                     $set_settings_message->target_php_settings,
                     $set_settings_message->get_trace_settings
                 );
+                Log::debug('start trace');
                 foreach ($loop_runner as $message) {
                     yield $this->protocol->sendTrace($message);
                 }
-            } catch (MemoryReaderException $e) {
-                // TODO: log errors
+                Log::debug('end trace');
+            } catch (\Throwable $e) {
+                Log::debug('exception thrown at reading traces', [
+                    'exception' => $e,
+                    'trace' => $e->getTrace(),
+                ]);
             }
 
+            Log::debug('detaching worker');
             yield $this->protocol->sendDetachWorker(
                 new DetachWorkerMessage($attach_message->pid)
             );
+            Log::debug('detached worker');
         }
     }
 }
diff --git a/src/Inspector/Daemon/Reader/Worker/PhpReaderTraceLoop.php b/src/Inspector/Daemon/Reader/Worker/PhpReaderTraceLoop.php
@@ -23,6 +23,8 @@
 use PhpProfiler\Lib\PhpProcessReader\PhpMemoryReader\ExecutorGlobalsReader;
 use PhpProfiler\Lib\Process\ProcessStopper\ProcessStopper;
 
+use function PhpProfiler\Lib\Defer\defer;
+
 final class PhpReaderTraceLoop implements PhpReaderTraceLoopInterface
 {
     public function __construct(
@@ -60,18 +62,17 @@ function () use (
                 $loop_settings,
                 $eg_address
             ): \Generator {
-                $is_target_stopped = false;
-                if ($loop_settings->stop_process) {
-                    $is_target_stopped = $this->process_stopper->stop($target_process_settings->pid);
+                if ($loop_settings->stop_process and $this->process_stopper->stop($target_process_settings->pid)) {
+                    defer($_, fn () => $this->process_stopper->resume($target_process_settings->pid));
                 }
                 $call_trace = $this->executor_globals_reader->readCallTrace(
                     $target_process_settings->pid,
                     $target_php_settings->php_version,
                     $eg_address,
                     $get_trace_settings->depth
                 );
-                if ($loop_settings->stop_process and $is_target_stopped) {
-                    $this->process_stopper->resume($target_process_settings->pid);
+                if (is_null($call_trace)) {
+                    return;
                 }
                 yield new TraceMessage($call_trace);
             },

diff --git a/src/Lib/Defer/ScopeGuard.php b/src/Lib/Defer/ScopeGuard.php
@@ -0,0 +1,31 @@
+<?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\Lib\Defer;
+
+final class ScopeGuard
+{
+    public function __construct(
+        private \Closure $callable,
+        private ?ScopeGuard $chain = null,
+    ) {
+    }
+
+    public function __destruct()
+    {
+        ($this->callable)();
+        if (isset($this->chain)) {
+            unset($this->chain);
+        }
+    }
+}
diff --git a/src/Lib/Defer/defer.php b/src/Lib/Defer/defer.php
@@ -0,0 +1,22 @@
+<?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\Lib\Defer;
+
+function defer(?ScopeGuard &$scope_guard, callable $function): void
+{
+    $scope_guard = new ScopeGuard(
+        \Closure::fromCallable($function),
+        $scope_guard ?? null
+    );
+}
diff --git a/src/Lib/File/NativeFileReader.php b/src/Lib/File/NativeFileReader.php
@@ -0,0 +1,52 @@
+<?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\Lib\File;
+
+use FFI;
+
+class NativeFileReader implements FileReaderInterface
+{
+    /** @var FFI\Libc\libc_file_ffi  */
+    private FFI $ffi;
+
+    public function __construct()
+    {
+        /** @var FFI\Libc\libc_file_ffi */
+        $this->ffi = FFI::cdef('
+            int open(const char *pathname, int flags);
+            ssize_t read(int fd, void *buf, size_t count);
+            int close(int fd);
+        ');
+    }
+
+    public function readAll(string $path): string
+    {
+        $buffer = $this->ffi->new("unsigned char[4096]");
+
+        $fd = $this->ffi->open($path, 0);
+        $result = "";
+        $done = false;
+        do {
+            $read_len = $this->ffi->read($fd, $buffer, 4096);
+            if ($read_len > 0) {
+                $result .= FFI::string($buffer, min($read_len, 4096));
+            } else {
+                $done = true;
+            }
+        } while (!$done);
+        $this->ffi->close($fd);
+
+        return $result;
+    }
+}
diff --git a/src/Lib/Loop/AsyncLoopMiddleware/RetryOnExceptionMiddlewareAsync.php b/src/Lib/Loop/AsyncLoopMiddleware/RetryOnExceptionMiddlewareAsync.php
@@ -14,6 +14,7 @@
 namespace PhpProfiler\Lib\Loop\AsyncLoopMiddleware;
 
 use Exception;
+use PhpProfiler\Lib\Log\Log;
 use PhpProfiler\Lib\Loop\AsyncLoopMiddlewareInterface;
 
 final class RetryOnExceptionMiddlewareAsync implements AsyncLoopMiddlewareInterface
@@ -37,6 +38,10 @@ public function invoke(): \Generator
             try {
                 yield from $this->chain->invoke();
             } catch (Exception $e) {
+                Log::debug($e->getMessage(), [
+                    'exception' => $e,
+                    'trace' => $e->getTrace()
+                ]);
                 if (in_array(get_class($e), $this->exception_names, true)) {
                     $this->current_retry_count++;
                     continue;

diff --git a/src/Lib/PhpInternals/Opcodes/OpcodeV70.php b/src/Lib/PhpInternals/Opcodes/OpcodeV70.php
@@ -539,7 +539,6 @@ public function __construct(
 
     public function getName(): string
     {
-        assert(isset(self::OPCODE_NAMES[$this->opcode]));
-        return self::OPCODE_NAMES[$this->opcode];
+        return self::OPCODE_NAMES[$this->opcode] ?? '';
     }
 }
diff --git a/src/Lib/PhpInternals/Opcodes/OpcodeV71.php b/src/Lib/PhpInternals/Opcodes/OpcodeV71.php
@@ -588,7 +588,6 @@ public function __construct(
 
     public function getName(): string
     {
-        assert(isset(self::OPCODE_NAMES[$this->opcode]));
-        return self::OPCODE_NAMES[$this->opcode];
+        return self::OPCODE_NAMES[$this->opcode] ?? '';
     }
 }
diff --git a/src/Lib/PhpInternals/Opcodes/OpcodeV72.php b/src/Lib/PhpInternals/Opcodes/OpcodeV72.php
@@ -621,7 +621,6 @@ public function __construct(
 
     public function getName(): string
     {
-        assert(isset(self::OPCODE_NAMES[$this->opcode]));
-        return self::OPCODE_NAMES[$this->opcode];
+        return self::OPCODE_NAMES[$this->opcode] ?? '';
     }
 }
diff --git a/src/Lib/PhpInternals/Opcodes/OpcodeV73.php b/src/Lib/PhpInternals/Opcodes/OpcodeV73.php
@@ -628,6 +628,6 @@ public function __construct(
 
     public function getName(): string
     {
-        return self::OPCODE_NAMES[$this->opcode];
+        return self::OPCODE_NAMES[$this->opcode] ?? '';
     }
 }
diff --git a/src/Lib/PhpInternals/Opcodes/OpcodeV74.php b/src/Lib/PhpInternals/Opcodes/OpcodeV74.php
@@ -616,6 +616,6 @@ public function __construct(
 
     public function getName(): string
     {
-        return self::OPCODE_NAMES[$this->opcode];
+        return self::OPCODE_NAMES[$this->opcode] ?? '';
     }
 }