Remove automatic `::shutdown()`, add `ShutdownHandler::register()` (#760)

* Remove automatic shutdown handling, add `ShutdownHandler::register()`

* Clear `WeakMap` values on destruct

composer.json

@@ -59,7 +59,8 @@
         },
         "files": [
             "src/Context/fiber/initialize_fiber_handler.php",
-            "src/SDK/Common/Dev/Compatibility/_load.php"
+            "src/SDK/Common/Dev/Compatibility/_load.php",
+            "src/SDK/Common/Util/functions.php"
         ]
     },
     "autoload-dev": {

examples/traces/demo/src/index.php

@@ -128,3 +128,4 @@ $app->get('/three', function (Response $response) {
 });
 
 $app->run();
+$tracerProvider->shutdown();

src/SDK/Common/Util/ShutdownHandler.php

@@ -0,0 +1,82 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\SDK\Common\Util;
+
+use function array_key_last;
+use ArrayAccess;
+use Closure;
+use function register_shutdown_function;
+
+final class ShutdownHandler
+{
+    /** @var array<int, Closure>|null */
+    private static ?array $handlers = null;
+    /** @var ArrayAccess<object, self>|null  */
+    private static ?ArrayAccess $weakMap = null;
+
+    private array $ids = [];
+
+    private function __construct()
+    {
+    }
+
+    public function __destruct()
+    {
+        if (!self::$handlers) {
+            return;
+        }
+        foreach ($this->ids as $id) {
+            unset(self::$handlers[$id]);
+        }
+    }
+
+    /**
+     * Registers a function that will be executed on shutdown.
+     *
+     * If the given function is bound to an object, then the function will only
+     * be executed if the bound object is still referenced on shutdown handler
+     * invocation.
+     *
+     * ```php
+     * ShutdownHandler::register([$tracerProvider, 'shutdown']);
+     * ```
+     *
+     * @param callable $shutdownFunction function to register
+     *
+     * @see register_shutdown_function
+     */
+    public static function register(callable $shutdownFunction): void
+    {
+        self::registerShutdownFunction();
+        self::$handlers[] = weaken(closure($shutdownFunction), $target);
+
+        if (!$object = $target) {
+            return;
+        }
+
+        self::$weakMap ??= WeakMap::create();
+        $handler = self::$weakMap[$object] ??= new self();
+        $handler->ids[] = array_key_last(self::$handlers);
+    }
+
+    private static function registerShutdownFunction(): void
+    {
+        if (self::$handlers === null) {
+            register_shutdown_function(static function (): void {
+                $handlers = self::$handlers;
+                self::$handlers = null;
+                self::$weakMap = null;
+
+                // Push shutdown to end of queue
+                // @phan-suppress-next-line PhanTypeMismatchArgumentInternal
+                register_shutdown_function(static function (array $handlers): void {
+                    foreach ($handlers as $handler) {
+                        $handler();
+                    }
+                }, $handlers);
+            });
+        }
+    }
+}

src/SDK/Common/Util/WeakMap.php

@@ -0,0 +1,175 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\SDK\Common\Util;
+
+use ArrayAccess;
+use function assert;
+use function class_exists;
+use function count;
+use Countable;
+use Error;
+use function get_class;
+use function is_object;
+use IteratorAggregate;
+use const PHP_VERSION_ID;
+use function spl_object_id;
+use function sprintf;
+use Traversable;
+use TypeError;
+use WeakReference;
+
+/**
+ * @internal
+ */
+final class WeakMap implements ArrayAccess, Countable, IteratorAggregate
+{
+    private const KEY = '__otel_weak_map';
+
+    /**
+     * @var array<int, WeakReference>
+     */
+    private array $objects = [];
+
+    private function __construct()
+    {
+    }
+
+    /**
+     * @return ArrayAccess&Countable&IteratorAggregate
+     */
+    public static function create(): ArrayAccess
+    {
+        if (PHP_VERSION_ID >= 80000) {
+            /** @phan-suppress-next-line PhanUndeclaredClassReference */
+            assert(class_exists(\WeakMap::class, false));
+            /** @phan-suppress-next-line PhanUndeclaredClassMethod */
+            $map = new \WeakMap();
+            assert($map instanceof ArrayAccess);
+            assert($map instanceof Countable);
+            assert($map instanceof IteratorAggregate);
+
+            return $map;
+        }
+
+        return new self();
+    }
+
+    public function offsetExists($offset): bool
+    {
+        if (!is_object($offset)) {
+            throw new TypeError('WeakMap key must be an object');
+        }
+
+        return isset($offset->{self::KEY}[spl_object_id($this)]);
+    }
+
+    /**
+     * @phan-suppress PhanUndeclaredClassAttribute
+     */
+    #[\ReturnTypeWillChange]
+    public function offsetGet($offset)
+    {
+        if (!is_object($offset)) {
+            throw new TypeError('WeakMap key must be an object');
+        }
+        if (!$this->contains($offset)) {
+            throw new Error(sprintf('Object %s#%d not contained in WeakMap', get_class($offset), spl_object_id($offset)));
+        }
+
+        return $offset->{self::KEY}[spl_object_id($this)];
+    }
+
+    public function offsetSet($offset, $value): void
+    {
+        if ($offset === null) {
+            throw new Error('Cannot append to WeakMap');
+        }
+        if (!is_object($offset)) {
+            throw new TypeError('WeakMap key must be an object');
+        }
+        if (!$this->contains($offset)) {
+            $this->expunge();
+        }
+
+        $offset->{self::KEY}[spl_object_id($this)] = $value;
+        $this->objects[spl_object_id($offset)] = WeakReference::create($offset);
+    }
+
+    public function offsetUnset($offset): void
+    {
+        if (!is_object($offset)) {
+            throw new TypeError('WeakMap key must be an object');
+        }
+        if (!$this->contains($offset)) {
+            return;
+        }
+
+        unset(
+            $offset->{self::KEY}[spl_object_id($this)],
+            $this->objects[spl_object_id($offset)],
+        );
+        if (!$offset->{self::KEY}) {
+            unset($offset->{self::KEY});
+        }
+    }
+
+    public function count(): int
+    {
+        $this->expunge();
+
+        return count($this->objects);
+    }
+
+    public function getIterator(): Traversable
+    {
+        $this->expunge();
+
+        foreach ($this->objects as $reference) {
+            if (($object = $reference->get()) && $this->contains($object)) {
+                yield $object => $this[$object];
+            }
+        }
+    }
+
+    public function __debugInfo(): array
+    {
+        $debugInfo = [];
+        foreach ($this as $key => $value) {
+            $debugInfo[] = ['key' => $key, 'value' => $value];
+        }
+
+        return $debugInfo;
+    }
+
+    public function __destruct()
+    {
+        foreach ($this->objects as $reference) {
+            if ($object = $reference->get()) {
+                unset($this[$object]);
+            }
+        }
+    }
+
+    private function contains(object $offset): bool
+    {
+        $reference = $this->objects[spl_object_id($offset)] ?? null;
+        if ($reference && $reference->get() === $offset) {
+            return true;
+        }
+
+        unset($this->objects[spl_object_id($offset)]);
+
+        return false;
+    }
+
+    private function expunge(): void
+    {
+        foreach ($this->objects as $id => $reference) {
+            if (!$reference->get()) {
+                unset($this->objects[$id]);
+            }
+        }
+    }
+}

src/SDK/Common/Util/functions.php

@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\SDK\Common\Util;
+
+use Closure;
+use function get_class;
+use ReflectionFunction;
+use stdClass;
+use WeakReference;
+
+/**
+ * @internal
+ */
+function closure(callable $callable): Closure
+{
+    return Closure::fromCallable($callable);
+}
+
+/**
+ * @internal
+ * @see https://github.com/amphp/amp/blob/f682341c856b1f688026f787bef4f77eaa5c7970/src/functions.php#L140-L191
+ */
+function weaken(Closure $closure, ?object &$target = null): Closure
+{
+    $reflection = new ReflectionFunction($closure);
+    if (!$target = $reflection->getClosureThis()) {
+        return $closure;
+    }
+
+    $scope = $reflection->getClosureScopeClass();
+    $name = $reflection->getShortName();
+    if ($name !== '{closure}') {
+        /** @psalm-suppress InvalidScope @phpstan-ignore-next-line @phan-suppress-next-line PhanUndeclaredThis */
+        $closure = fn (...$args) => $this->$name(...$args);
+        if ($scope !== null) {
+            $closure->bindTo(null, $scope->name);
+        }
+    }
+
+    static $placeholder;
+    $placeholder ??= new stdClass();
+    $closure = $closure->bindTo($placeholder);
+
+    $ref = WeakReference::create($target);
+
+    /** @psalm-suppress PossiblyInvalidFunctionCall */
+    return $scope && get_class($target) === $scope->name && !$scope->isInternal()
+        ? static fn (...$args) => ($obj = $ref->get()) ? $closure->call($obj, ...$args) : null
+        : static fn (...$args) => ($obj = $ref->get()) ? $closure->bindTo($obj)(...$args) : null;
+}

src/SDK/Trace/TracerProvider.php

@@ -14,15 +14,9 @@ use OpenTelemetry\SDK\Resource\ResourceInfo;
 use OpenTelemetry\SDK\Resource\ResourceInfoFactory;
 use OpenTelemetry\SDK\Trace\Sampler\AlwaysOnSampler;
 use OpenTelemetry\SDK\Trace\Sampler\ParentBased;
-use function register_shutdown_function;
-use function spl_object_id;
-use WeakReference;
 
 final class TracerProvider implements TracerProviderInterface
 {
-    /** @var array<int, WeakReference<self>>|null */
-    private static ?array $tracerProviders = null;
-
     /** @readonly */
     private TracerSharedState $tracerSharedState;
     private InstrumentationScopeFactoryInterface $instrumentationScopeFactory;
@@ -54,8 +48,6 @@ final class TracerProvider implements TracerProviderInterface
             $spanProcessors
         );
         $this->instrumentationScopeFactory = $instrumentationScopeFactory ?? new InstrumentationScopeFactory(Attributes::factory());
-
-        self::registerShutdownFunction($this);
     }
 
     public function forceFlush(): bool
@@ -65,8 +57,6 @@ final class TracerProvider implements TracerProviderInterface
 
     /**
      * @inheritDoc
-     * @note Getting a tracer without keeping a strong reference to the TracerProvider will cause the TracerProvider to
-     *       immediately shut itself down including its shared state, ie don't do this: $tracer = (new TracerProvider())->getTracer('foo')
      */
     public function getTracer(
         string $name,
@@ -98,41 +88,6 @@ final class TracerProvider implements TracerProviderInterface
             return true;
         }
 
-        self::unregisterShutdownFunction($this);
-
         return $this->tracerSharedState->shutdown();
     }
-
-    public function __destruct()
-    {
-        $this->shutdown();
-    }
-
-    private static function registerShutdownFunction(TracerProvider $tracerProvider): void
-    {
-        if (self::$tracerProviders === null) {
-            register_shutdown_function(static function (): void {
-                $tracerProviders = self::$tracerProviders;
-                self::$tracerProviders = null;
-
-                // Push tracer provider shutdown to end of queue
-                // @phan-suppress-next-line PhanTypeMismatchArgumentInternal
-                register_shutdown_function(static function (array $tracerProviders): void {
-                    foreach ($tracerProviders as $reference) {
-                        if ($tracerProvider = $reference->get()) {
-                            $tracerProvider->shutdown();
-                        }
-                    }
-                }, $tracerProviders);
-            });
-        }
-
-        self::$tracerProviders[spl_object_id($tracerProvider)] = WeakReference::create($tracerProvider);
-    }
-
-    private static function unregisterShutdownFunction(TracerProvider $tracerProvider): void
-    {
-        /** @psalm-suppress PossiblyNullArrayAccess */
-        unset(self::$tracerProviders[spl_object_id($tracerProvider)]);
-    }
 }

src/SDK/composer.json

@@ -26,7 +26,10 @@
     "autoload": {
         "psr-4": {
             "OpenTelemetry\\SDK\\": "."
-        }
+        },
+        "files": [
+            "src/SDK/Common/Util/functions.php"
+        ]
     },
     "suggest": {
         "ext-mbstring": "To increase performance of string operations"

tests/Integration/SDK/TracerProviderTest.php

@@ -1,49 +0,0 @@
-<?php
-
-declare(strict_types=1);
-
-namespace OpenTelemetry\Tests\Integration\SDK;
-
-use OpenTelemetry\SDK\Trace\SpanProcessorInterface;
-use OpenTelemetry\SDK\Trace\TracerProvider;
-use PHPUnit\Framework\TestCase;
-use Prophecy\Prophet;
-
-/**
- * @coversNothing
- */
-class TracerProviderTest extends TestCase
-{
-    /**
-     * @doesNotPerformAssertions
-     */
-    public function test_tracer_shuts_down_immediately_when_out_of_scope(): void
-    {
-        $prophet = new Prophet();
-        $spanProcessor = $prophet->prophesize(SpanProcessorInterface::class);
-        // @phpstan-ignore-next-line
-        $spanProcessor->shutdown()->shouldBeCalledTimes(1);
-
-        /* Because no reference is kept to the TracerProvider, it will immediately __destruct and shutdown,
-        which will also shut down span processors in shared state. */
-        $tracer = (new TracerProvider($spanProcessor->reveal()))->getTracer('test');
-
-        $spanProcessor->checkProphecyMethodsPredictions();
-    }
-
-    /**
-     * @doesNotPerformAssertions
-     */
-    public function test_tracer_remains_in_scope(): void
-    {
-        $prophet = new Prophet();
-        $spanProcessor = $prophet->prophesize(SpanProcessorInterface::class);
-        // @phpstan-ignore-next-line
-        $spanProcessor->shutdown()->shouldBeCalledTimes(0);
-
-        $tracerProvider = new TracerProvider($spanProcessor->reveal());
-        $tracer = $tracerProvider->getTracer('test');
-
-        $spanProcessor->checkProphecyMethodsPredictions();
-    }
-}

tests/Unit/SDK/Common/Util/ShutdownHandlerTest.php

@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\Tests\Unit\SDK\Common\Util;
+
+use OpenTelemetry\SDK\Common\Util\ShutdownHandler;
+use PHPUnit\Framework\TestCase;
+use WeakReference;
+
+/**
+ * @covers \OpenTelemetry\SDK\Common\Util\ShutdownHandler
+ */
+final class ShutdownHandlerTest extends TestCase
+{
+    public function test_shutdown_handler_does_not_keep_reference_to_shutdown_function_this(): void
+    {
+        $object = new class() {
+            public function foo(): void
+            {
+            }
+        };
+
+        ShutdownHandler::register([$object, 'foo']);
+
+        $reference = WeakReference::create($object);
+        $object = null;
+
+        $this->assertNull($reference->get());
+    }
+}

tests/Unit/SDK/Common/Util/WeakenTest.php

@@ -0,0 +1,70 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OpenTelemetry\Tests\Unit\SDK\Common\Util;
+
+use function OpenTelemetry\SDK\Common\Util\closure;
+use function OpenTelemetry\SDK\Common\Util\weaken;
+use PHPUnit\Framework\TestCase;
+use WeakReference;
+
+/**
+ * @covers \OpenTelemetry\SDK\Common\Util\closure
+ * @covers \OpenTelemetry\SDK\Common\Util\weaken
+ */
+final class WeakenTest extends TestCase
+{
+    public function test_weakened_closure_calls_original_closure(): void
+    {
+        $object = new class() {
+            public function foo(): int
+            {
+                return 5;
+            }
+        };
+
+        $weakened = weaken(closure([$object, 'foo']));
+
+        $this->assertSame(5, $weakened());
+    }
+
+    public function test_weakened_closure_weakens_bound_this(): void
+    {
+        $object = new class() {
+            public function foo(): int
+            {
+                return 5;
+            }
+        };
+
+        $weakened = weaken(closure([$object, 'foo']));
+        $reference = WeakReference::create($object);
+
+        $object = null;
+
+        $this->assertNull($reference->get());
+        $this->assertNull($weakened());
+    }
+
+    public function test_weaken_assigns_bound_this_to_target(): void
+    {
+        $object = new class() {
+            public function foo(): int
+            {
+                return 5;
+            }
+        };
+
+        weaken(closure([$object, 'foo']), $target);
+
+        $this->assertSame($object, $target);
+    }
+
+    public function test_weaken_is_noop_if_no_bound_this(): void
+    {
+        $closure = static fn (): int => 5;
+
+        $this->assertSame($closure, weaken($closure));
+    }
+}

tests/Unit/SDK/Common/Util/test_shutdown_handler_triggers_on_shutdown.phpt

@@ -0,0 +1,24 @@
+--TEST--
+ShutdownHandler is triggered on shutdown
+--FILE--
+<?php
+use OpenTelemetry\SDK\Common\Util\ShutdownHandler;
+
+class ShutdownTest {
+    public function shutdown(): void {
+        var_dump(spl_object_id($this));
+    }
+}
+
+$a = new ShutdownTest();
+$b = new ShutdownTest();
+
+require_once 'vendor/autoload.php';
+
+ShutdownHandler::register([$a, 'shutdown']);
+ShutdownHandler::register([$b, 'shutdown']);
+
+?>
+--EXPECT--
+int(1)
+int(2)

tests/Unit/SDK/Trace/TracerProviderTest.php

@@ -8,7 +8,6 @@ use OpenTelemetry\API\Trace\NoopTracer;
 use OpenTelemetry\SDK\Trace\SamplerInterface;
 use OpenTelemetry\SDK\Trace\TracerProvider;
 use PHPUnit\Framework\TestCase;
-use WeakReference;
 
 /**
  * @coversDefaultClass \OpenTelemetry\SDK\Trace\TracerProvider
@@ -118,16 +117,4 @@ class TracerProviderTest extends TestCase
             $provider->getTracer('foo')
         );
     }
-
-    /**
-     * @coversNothing
-     */
-    public function test_tracer_register_shutdown_function_does_not_leak_reference(): void
-    {
-        $provider = new TracerProvider();
-        $reference = WeakReference::create($provider);
-
-        $provider = null;
-        $this->assertTrue($reference->get() === null);
-    }
 }