diff --git a/composer.json b/composer.json index 8730cf74c..8086e549a 100644 --- a/composer.json +++ b/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": { diff --git a/examples/traces/demo/src/index.php b/examples/traces/demo/src/index.php index 6819a53df..b99b203d1 100644 --- a/examples/traces/demo/src/index.php +++ b/examples/traces/demo/src/index.php @@ -128,3 +128,4 @@ }); $app->run(); +$tracerProvider->shutdown(); diff --git a/src/SDK/Common/Util/ShutdownHandler.php b/src/SDK/Common/Util/ShutdownHandler.php new file mode 100644 index 000000000..2de6d47df --- /dev/null +++ b/src/SDK/Common/Util/ShutdownHandler.php @@ -0,0 +1,82 @@ +|null */ + private static ?array $handlers = null; + /** @var ArrayAccess|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); + }); + } + } +} diff --git a/src/SDK/Common/Util/WeakMap.php b/src/SDK/Common/Util/WeakMap.php new file mode 100644 index 000000000..3b62d6d64 --- /dev/null +++ b/src/SDK/Common/Util/WeakMap.php @@ -0,0 +1,175 @@ + + */ + 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]); + } + } + } +} diff --git a/src/SDK/Common/Util/functions.php b/src/SDK/Common/Util/functions.php new file mode 100644 index 000000000..eff0328e1 --- /dev/null +++ b/src/SDK/Common/Util/functions.php @@ -0,0 +1,52 @@ +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; +} diff --git a/src/SDK/Trace/TracerProvider.php b/src/SDK/Trace/TracerProvider.php index e981ea6a8..f4553a907 100644 --- a/src/SDK/Trace/TracerProvider.php +++ b/src/SDK/Trace/TracerProvider.php @@ -14,15 +14,9 @@ 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>|null */ - private static ?array $tracerProviders = null; - /** @readonly */ private TracerSharedState $tracerSharedState; private InstrumentationScopeFactoryInterface $instrumentationScopeFactory; @@ -54,8 +48,6 @@ public function __construct( $spanProcessors ); $this->instrumentationScopeFactory = $instrumentationScopeFactory ?? new InstrumentationScopeFactory(Attributes::factory()); - - self::registerShutdownFunction($this); } public function forceFlush(): bool @@ -65,8 +57,6 @@ public function forceFlush(): bool /** * @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 @@ public function shutdown(): bool 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)]); - } } diff --git a/src/SDK/composer.json b/src/SDK/composer.json index 4cdf06418..e16511f87 100644 --- a/src/SDK/composer.json +++ b/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" diff --git a/tests/Integration/SDK/TracerProviderTest.php b/tests/Integration/SDK/TracerProviderTest.php deleted file mode 100644 index 74328ddb8..000000000 --- a/tests/Integration/SDK/TracerProviderTest.php +++ /dev/null @@ -1,49 +0,0 @@ -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(); - } -} diff --git a/tests/Unit/SDK/Common/Util/ShutdownHandlerTest.php b/tests/Unit/SDK/Common/Util/ShutdownHandlerTest.php new file mode 100644 index 000000000..225306a81 --- /dev/null +++ b/tests/Unit/SDK/Common/Util/ShutdownHandlerTest.php @@ -0,0 +1,31 @@ +assertNull($reference->get()); + } +} diff --git a/tests/Unit/SDK/Common/Util/WeakenTest.php b/tests/Unit/SDK/Common/Util/WeakenTest.php new file mode 100644 index 000000000..b7ef71555 --- /dev/null +++ b/tests/Unit/SDK/Common/Util/WeakenTest.php @@ -0,0 +1,70 @@ +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)); + } +} diff --git a/tests/Unit/SDK/Common/Util/test_shutdown_handler_triggers_on_shutdown.phpt b/tests/Unit/SDK/Common/Util/test_shutdown_handler_triggers_on_shutdown.phpt new file mode 100644 index 000000000..cf425f4d3 --- /dev/null +++ b/tests/Unit/SDK/Common/Util/test_shutdown_handler_triggers_on_shutdown.phpt @@ -0,0 +1,24 @@ +--TEST-- +ShutdownHandler is triggered on shutdown +--FILE-- + +--EXPECT-- +int(1) +int(2) diff --git a/tests/Unit/SDK/Trace/TracerProviderTest.php b/tests/Unit/SDK/Trace/TracerProviderTest.php index 5cbe7b8ab..f73116ccc 100644 --- a/tests/Unit/SDK/Trace/TracerProviderTest.php +++ b/tests/Unit/SDK/Trace/TracerProviderTest.php @@ -8,7 +8,6 @@ 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 @@ public function test_get_tracer_returns_noop_tracer_after_shutdown(): void $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); - } }