diff --git a/analysis-baseline.toml b/analysis-baseline.toml index 536037537..dc9b1c7fe 100644 --- a/analysis-baseline.toml +++ b/analysis-baseline.toml @@ -198,12 +198,6 @@ code = "reference-to-undefined-variable" message = "Reference created from a previously undefined variable `$matches`." count = 1 -[[issues]] -file = "src/ErrorHandler.php" -code = "write-only-property" -message = "Property `$reservedMemory` is written to but never read." -count = 1 - [[issues]] file = "src/Event.php" code = "impossible-condition" diff --git a/src/ErrorHandler.php b/src/ErrorHandler.php index ec9ed6ade..280f8e395 100644 --- a/src/ErrorHandler.php +++ b/src/ErrorHandler.php @@ -112,8 +112,6 @@ final class ErrorHandler /** * @var string|null A portion of pre-allocated memory data that will be reclaimed in case a fatal error occurs to handle it - * - * @phpstan-ignore-next-line This property is used to reserve memory for the fatal error handler and is thus never read */ private static $reservedMemory; @@ -315,7 +313,10 @@ public static function resetFatalErrorHandlerState(): void self::$disableFatalErrorHandler = false; self::$didIncreaseMemoryLimit = false; - if (self::$handlerInstance !== null && self::$handlerInstance->isFatalErrorHandlerRegistered) { + if (self::$handlerInstance !== null + && self::$handlerInstance->isFatalErrorHandlerRegistered + && self::$reservedMemory === null + ) { self::$reservedMemory = str_repeat('x', self::$reservedMemorySize); } } diff --git a/src/SentrySdk.php b/src/SentrySdk.php index c7cdb36af..676c0d5b1 100644 --- a/src/SentrySdk.php +++ b/src/SentrySdk.php @@ -10,6 +10,7 @@ use Sentry\State\HubInterface; use Sentry\State\RuntimeContext; use Sentry\State\RuntimeContextManager; +use Sentry\State\RuntimeContextStorageInterface; /** * This class is the main entry point for all the most common SDK features. @@ -38,11 +39,20 @@ private function __construct() /** * Initializes the SDK by creating a new hub instance each time this method * gets called. + * + * @param RuntimeContextStorageInterface|null $runtimeContextStorage Storage for isolating overlapping logical executions */ - public static function init(): HubInterface + public static function init(?RuntimeContextStorageInterface $runtimeContextStorage = null): HubInterface { + if ($runtimeContextStorage !== null) { + // The new manager must not select a context the previous one left in host storage. + // The removed context is discarded unflushed, matching how reinitialization has + // always dropped active manager state. + $runtimeContextStorage->remove(); + } + self::$currentHub = new Hub(); - self::$runtimeContextManager = new RuntimeContextManager(self::$currentHub); + self::$runtimeContextManager = new RuntimeContextManager(self::$currentHub, $runtimeContextStorage); return self::getCurrentHub(); } @@ -89,7 +99,7 @@ public static function endContext(?int $timeout = null): void /** * Executes the given callback within an isolated context. * - * If a context is already active for the current execution key, this method + * If a context is already active for the current logical execution, this method * reuses it and only executes the callback. * * @param callable $callback The callback to execute @@ -105,11 +115,7 @@ public static function endContext(?int $timeout = null): void public static function withContext(callable $callback, ?int $timeout = null) { $runtimeContextManager = self::getRuntimeContextManager(); - $startedNewContext = !$runtimeContextManager->hasActiveContext(); - - if ($startedNewContext) { - $runtimeContextManager->startContext(); - } + $startedNewContext = $runtimeContextManager->startContext(); try { return $callback(); diff --git a/src/State/RuntimeContext.php b/src/State/RuntimeContext.php index 6910cae60..99aa2b7c1 100644 --- a/src/State/RuntimeContext.php +++ b/src/State/RuntimeContext.php @@ -13,7 +13,8 @@ * A unit of work can be an HTTP request, a queue job, a worker task, or any * explicit lifecycle wrapped with startContext()/endContext(). * - * @internal + * Storage implementations should treat instances as opaque values owned by the + * SDK and must not create or mutate them directly. */ final class RuntimeContext { @@ -37,6 +38,9 @@ final class RuntimeContext */ private $metricsAggregator; + /** + * @internal + */ public function __construct(string $id, HubInterface $hub) { $this->id = $id; @@ -45,26 +49,41 @@ public function __construct(string $id, HubInterface $hub) $this->metricsAggregator = new MetricsAggregator(); } + /** + * @internal + */ public function getId(): string { return $this->id; } + /** + * @internal + */ public function getHub(): HubInterface { return $this->hub; } + /** + * @internal + */ public function setHub(HubInterface $hub): void { $this->hub = $hub; } + /** + * @internal + */ public function getLogsAggregator(): LogsAggregator { return $this->logsAggregator; } + /** + * @internal + */ public function getMetricsAggregator(): MetricsAggregator { return $this->metricsAggregator; diff --git a/src/State/RuntimeContextManager.php b/src/State/RuntimeContextManager.php index 1c218b80a..845ee4bc9 100644 --- a/src/State/RuntimeContextManager.php +++ b/src/State/RuntimeContextManager.php @@ -12,18 +12,14 @@ /** * Manages runtime-local SDK state across different execution models. * - * Lifecycle model: - * - The manager keeps a lazily initialized global context as fallback. - * - startContext() creates an isolated runtime context for the current - * execution key when no context is active yet. - * - endContext() flushes context resources and removes that context. + * The manager keeps a lazily initialized global context as fallback. Explicit + * contexts use process-local storage by default, or the configured storage for + * runtimes with overlapping logical executions. * * @internal */ final class RuntimeContextManager { - private const PROCESS_EXECUTION_CONTEXT_KEY = 'process'; - /** * @var HubInterface */ @@ -35,25 +31,25 @@ final class RuntimeContextManager private $globalContext; /** - * @var array + * @var RuntimeContext|null */ - private $activeContexts = []; + private $runtimeContext; /** - * @var array + * @var RuntimeContextStorageInterface|null */ - private $executionContextToRuntimeContext = []; + private $runtimeContextStorage; - public function __construct(HubInterface $baseHub) + public function __construct(HubInterface $baseHub, ?RuntimeContextStorageInterface $runtimeContextStorage = null) { $this->baseHub = $baseHub; - $this->globalContext = null; + $this->runtimeContextStorage = $runtimeContextStorage; } /** * Sets the current hub with context-aware behavior. * - * If a runtime context is active for the current execution key, the hub is + * If a runtime context is active for the current logical execution, the hub is * updated only for that active context. Otherwise, the baseline/global hub * template is updated. * @@ -61,11 +57,10 @@ public function __construct(HubInterface $baseHub) */ public function setCurrentHub(HubInterface $hub): bool { - $executionContextKey = $this->getExecutionContextKey(); + $runtimeContext = $this->getActiveContext(); - if ($this->hasActiveContextForExecutionContextKey($executionContextKey)) { - $runtimeContextId = $this->executionContextToRuntimeContext[$executionContextKey]; - $this->activeContexts[$runtimeContextId]->setHub($hub); + if ($runtimeContext !== null) { + $runtimeContext->setHub($hub); return true; } @@ -86,78 +81,41 @@ public function getCurrentHub(): HubInterface public function getCurrentContext(): RuntimeContext { - $executionContextKey = $this->getExecutionContextKey(); - - if ($this->hasActiveContextForExecutionContextKey($executionContextKey)) { - $runtimeContextId = $this->executionContextToRuntimeContext[$executionContextKey]; - - return $this->activeContexts[$runtimeContextId]; - } - - return $this->getGlobalContext(); - } - - public function hasActiveContext(): bool - { - return $this->hasActiveContextForExecutionContextKey($this->getExecutionContextKey()); + return $this->getActiveContext() ?? $this->getGlobalContext(); } /** - * Starts an isolated context for the current execution key. + * Starts an isolated context for the current logical execution. + * + * @return bool Whether a new context was started */ - public function startContext(): void + public function startContext(): bool { - $executionContextKey = $this->getExecutionContextKey(); - - if ($this->hasActiveContextForExecutionContextKey($executionContextKey)) { - // Nested start calls for the same execution key should be a no-op. - return; + if ($this->getActiveContext() !== null) { + // Nested start calls for the same logical execution should be a no-op. + return false; } ErrorHandler::resetFatalErrorHandlerState(); - $this->createContextForExecutionContextKey($executionContextKey); + $this->setActiveContext(new RuntimeContext($this->generateRuntimeContextId(), $this->createHubFromBaseHub())); + + return true; } /** - * Ends and flushes the active context for the current execution key. + * Ends and flushes the active context for the current logical execution. * - * When no context is active for the key this is a no-op. + * When no context is active this is a no-op. */ public function endContext(?int $timeout = null): void { - $executionContextKey = $this->getExecutionContextKey(); - - if (!$this->hasActiveContextForExecutionContextKey($executionContextKey)) { - return; - } - - $runtimeContextId = $this->executionContextToRuntimeContext[$executionContextKey]; - unset($this->executionContextToRuntimeContext[$executionContextKey]); - - $this->removeContextById($runtimeContextId, $timeout); - } - - private function createContextForExecutionContextKey(string $executionContextKey): void - { - $runtimeContextId = $this->generateRuntimeContextId(); - $runtimeContext = new RuntimeContext($runtimeContextId, $this->createHubFromBaseHub()); - - $this->activeContexts[$runtimeContextId] = $runtimeContext; - $this->executionContextToRuntimeContext[$executionContextKey] = $runtimeContextId; - } + $runtimeContext = $this->removeActiveContext(); - private function removeContextById(string $runtimeContextId, ?int $timeout = null): void - { - if (!isset($this->activeContexts[$runtimeContextId])) { + if ($runtimeContext === null) { return; } - $runtimeContext = $this->activeContexts[$runtimeContextId]; - unset($this->activeContexts[$runtimeContextId]); - // Remove any key mappings that may still reference this context. - $this->removeExecutionContextMappingsForRuntimeContext($runtimeContextId); - $logger = $this->getLoggerFromHub($runtimeContext->getHub()); $this->flushRuntimeContextResources($runtimeContext, $timeout, $logger); @@ -205,31 +163,36 @@ private function flushRuntimeContextResources(RuntimeContext $runtimeContext, ?i } } - private function removeExecutionContextMappingsForRuntimeContext(string $runtimeContextId): void + private function getActiveContext(): ?RuntimeContext { - foreach ($this->executionContextToRuntimeContext as $executionContextKey => $mappedRuntimeContextId) { - if ($mappedRuntimeContextId === $runtimeContextId) { - unset($this->executionContextToRuntimeContext[$executionContextKey]); - } + if ($this->runtimeContextStorage !== null) { + return $this->runtimeContextStorage->get(); } + + return $this->runtimeContext; } - private function hasActiveContextForExecutionContextKey(string $executionContextKey): bool + private function setActiveContext(RuntimeContext $runtimeContext): void { - if (!isset($this->executionContextToRuntimeContext[$executionContextKey])) { - return false; - } + if ($this->runtimeContextStorage !== null) { + $this->runtimeContextStorage->set($runtimeContext); - $runtimeContextId = $this->executionContextToRuntimeContext[$executionContextKey]; + return; + } - if (!isset($this->activeContexts[$runtimeContextId])) { - // Mapping points to a context that was already evicted/ended; drop the stale index entry. - unset($this->executionContextToRuntimeContext[$executionContextKey]); + $this->runtimeContext = $runtimeContext; + } - return false; + private function removeActiveContext(): ?RuntimeContext + { + if ($this->runtimeContextStorage !== null) { + return $this->runtimeContextStorage->remove(); } - return true; + $runtimeContext = $this->runtimeContext; + $this->runtimeContext = null; + + return $runtimeContext; } private function createHubFromBaseHub(): HubInterface @@ -266,12 +229,6 @@ private function generateRuntimeContextId(): string return \sprintf('%s-%d', str_replace('.', '', uniqid('', true)), mt_rand()); } - private function getExecutionContextKey(): string - { - // All supported runtime modes currently use a process-local execution key. - return self::PROCESS_EXECUTION_CONTEXT_KEY; - } - private function getGlobalContext(): RuntimeContext { if ($this->globalContext === null) { diff --git a/src/State/RuntimeContextStorageInterface.php b/src/State/RuntimeContextStorageInterface.php new file mode 100644 index 000000000..0dc8fc95d --- /dev/null +++ b/src/State/RuntimeContextStorageInterface.php @@ -0,0 +1,47 @@ +getClient(); - SentrySdk::init()->bindClient($client); + SentrySdk::init($runtimeContextStorage)->bindClient($client); } /** @@ -235,7 +239,7 @@ function endContext(?int $timeout = null): void /** * Executes the given callback within an isolated context. * - * If a context is already active for the current execution key, it is reused. + * If a context is already active for the current logical execution, it is reused. * * @param callable $callback The callback to execute * @param int|null $timeout The maximum number of seconds to wait while flushing the client transport diff --git a/tests/FunctionsTest.php b/tests/FunctionsTest.php index c42999c1a..0fd290243 100644 --- a/tests/FunctionsTest.php +++ b/tests/FunctionsTest.php @@ -61,6 +61,22 @@ public function testInit(): void $this->assertNotNull(SentrySdk::getCurrentHub()->getClient()); } + public function testInitUsesRuntimeContextStorage(): void + { + $storage = new StubRuntimeContextStorage(); + + init(['default_integrations' => false], $storage); + + $storage->switchTo('request'); + startContext(); + + $this->assertNotNull($storage->get()); + + endContext(); + + $this->assertNull($storage->get()); + } + /** * @dataProvider captureMessageDataProvider */ diff --git a/tests/SentrySdkTest.php b/tests/SentrySdkTest.php index bb3d7e14a..cb5384cff 100644 --- a/tests/SentrySdkTest.php +++ b/tests/SentrySdkTest.php @@ -8,6 +8,8 @@ use PHPUnit\Framework\TestCase; use Sentry\ClientInterface; use Sentry\Event; +use Sentry\Logs\Logs; +use Sentry\Metrics\TraceMetrics; use Sentry\Options; use Sentry\SentrySdk; use Sentry\State\Hub; @@ -147,6 +149,142 @@ public function testNestedStartContextIsNoOp(): void $this->assertSame($globalHub, SentrySdk::getCurrentHub()); } + public function testRuntimeContextStorageIsolatesConcurrentExecutions(): void + { + $storage = new StubRuntimeContextStorage(); + $globalHub = SentrySdk::init($storage); + + $storage->switchTo('first'); + SentrySdk::startContext(); + + $firstContext = SentrySdk::getCurrentRuntimeContext(); + $firstLogsAggregator = $firstContext->getLogsAggregator(); + $firstMetricsAggregator = $firstContext->getMetricsAggregator(); + $firstHub = new Hub(); + + SentrySdk::setCurrentHub($firstHub); + + $this->assertSame($firstHub, $firstContext->getHub()); + + $firstHub->configureScope(static function (Scope $scope): void { + $scope->setTag('execution', 'first'); + }); + + $storage->switchTo('second'); + SentrySdk::startContext(); + + $secondContext = SentrySdk::getCurrentRuntimeContext(); + $secondHub = $secondContext->getHub(); + + $secondHub->configureScope(static function (Scope $scope): void { + $scope->setTag('execution', 'second'); + }); + + $this->assertNotSame($firstContext, $secondContext); + $this->assertNotSame($firstHub, $secondHub); + $this->assertNotSame($firstLogsAggregator, $secondContext->getLogsAggregator()); + $this->assertNotSame($firstMetricsAggregator, $secondContext->getMetricsAggregator()); + + $storage->switchTo('first'); + + $this->assertSame($firstContext, SentrySdk::getCurrentRuntimeContext()); + $this->assertSame('first', $this->getCurrentScopeTag('execution')); + + $storage->switchTo('second'); + + $this->assertSame($secondContext, SentrySdk::getCurrentRuntimeContext()); + $this->assertSame('second', $this->getCurrentScopeTag('execution')); + + SentrySdk::endContext(); + + $this->assertSame($globalHub, SentrySdk::getCurrentHub()); + + $storage->switchTo('first'); + + $this->assertSame($firstContext, SentrySdk::getCurrentRuntimeContext()); + + SentrySdk::endContext(); + + $this->assertSame($globalHub, SentrySdk::getCurrentHub()); + } + + public function testRuntimeContextStorageCanReleaseAbandonedExecutions(): void + { + $storage = new StubRuntimeContextStorage(); + $globalHub = SentrySdk::init($storage); + + $storage->switchTo('abandoned'); + SentrySdk::startContext(); + + $abandonedContext = SentrySdk::getCurrentRuntimeContext(); + + $storage->release('abandoned'); + + $this->assertNotSame($abandonedContext, SentrySdk::getCurrentRuntimeContext()); + $this->assertSame($globalHub, SentrySdk::getCurrentHub()); + } + + public function testRepeatedEndContextWithRuntimeContextStorageIsNoOp(): void + { + /** @var ClientInterface&MockObject $client */ + $client = $this->createMock(ClientInterface::class); + $client->expects($this->once()) + ->method('getOptions') + ->willReturn(new Options()); + $client->expects($this->once()) + ->method('flush') + ->willReturn(new Result(ResultStatus::success())); + + $storage = new StubRuntimeContextStorage(); + $globalHub = SentrySdk::init($storage); + $globalHub->bindClient($client); + + $storage->switchTo('request'); + SentrySdk::startContext(); + SentrySdk::endContext(); + + $this->assertNull($storage->get()); + + SentrySdk::endContext(); + + $this->assertNull($storage->get()); + $this->assertSame($globalHub, SentrySdk::getCurrentHub()); + } + + public function testInitClearsContextStoredByPreviousManager(): void + { + /** @var ClientInterface&MockObject $secondClient */ + $secondClient = $this->createMock(ClientInterface::class); + $secondClient->expects($this->once()) + ->method('getOptions') + ->willReturn(new Options()); + $secondClient->expects($this->once()) + ->method('flush') + ->willReturn(new Result(ResultStatus::success())); + + $storage = new StubRuntimeContextStorage(); + SentrySdk::init($storage)->bindClient($this->createMock(ClientInterface::class)); + + $storage->switchTo('request'); + SentrySdk::startContext(); + $previousHub = SentrySdk::getCurrentHub(); + + $freshHub = SentrySdk::init($storage); + + $this->assertNull($storage->get()); + $this->assertNotSame($previousHub, $freshHub); + + $freshHub->bindClient($secondClient); + + // This end/start transition exposes a stale context as a missing client. + SentrySdk::endContext(); + SentrySdk::startContext(); + + $this->assertSame($secondClient, SentrySdk::getCurrentHub()->getClient()); + + SentrySdk::endContext(); + } + public function testEndContextFlushesClientTransportWithOptionalTimeout(): void { /** @var ClientInterface&MockObject $client */ @@ -179,6 +317,43 @@ public function testFlushFlushesClientTransport(): void SentrySdk::flush(); } + public function testEndContextFlushesResourcesIndependently(): void + { + StubLogger::$logs = []; + + /** @var ClientInterface&MockObject $client */ + $client = $this->createMock(ClientInterface::class); + $client->expects($this->atLeastOnce()) + ->method('getOptions') + ->willReturn(new Options(['logger' => StubLogger::getInstance()])); + $client->expects($this->exactly(2)) + ->method('captureEvent') + ->willReturnCallback(static function (Event $event): void { + throw new \RuntimeException('Failed capturing ' . (string) $event->getType()); + }); + $client->expects($this->once()) + ->method('flush') + ->willThrowException(new \RuntimeException('Failed flushing transport')); + + SentrySdk::init()->bindClient($client); + SentrySdk::startContext(); + + Logs::getInstance()->info('log'); + TraceMetrics::getInstance()->count('metric', 1); + + SentrySdk::endContext(); + + $errors = array_filter(StubLogger::$logs, static function (array $log): bool { + return $log['level'] === 'error'; + }); + + $this->assertSame([ + 'Failed to flush logs while ending a runtime context.', + 'Failed to flush trace metrics while ending a runtime context.', + 'Failed to flush the client transport while ending a runtime context.', + ], array_column($errors, 'message')); + } + public function testWithContextReturnsCallbackResultAndRestoresGlobalHub(): void { SentrySdk::init(); @@ -275,4 +450,16 @@ private function getCurrentScopeTraceparent(): string return $traceparent; } + + private function getCurrentScopeTag(string $key): ?string + { + $value = null; + + SentrySdk::getCurrentHub()->configureScope(static function (Scope $scope) use ($key, &$value): void { + $event = $scope->applyToEvent(Event::createEvent()); + $value = $event !== null ? $event->getTags()[$key] ?? null : null; + }); + + return $value; + } } diff --git a/tests/StubRuntimeContextStorage.php b/tests/StubRuntimeContextStorage.php new file mode 100644 index 000000000..d3041e15f --- /dev/null +++ b/tests/StubRuntimeContextStorage.php @@ -0,0 +1,52 @@ + + */ + private $contexts = []; + + public function get(): ?RuntimeContext + { + return $this->contexts[$this->execution] ?? null; + } + + public function set(RuntimeContext $runtimeContext): void + { + $this->contexts[$this->execution] = $runtimeContext; + } + + public function remove(): ?RuntimeContext + { + $runtimeContext = $this->get(); + unset($this->contexts[$this->execution]); + + return $runtimeContext; + } + + public function switchTo(string $execution): void + { + $this->execution = $execution; + } + + public function release(string $execution): void + { + unset($this->contexts[$execution]); + } +} diff --git a/tests/phpt/error_handler_reset_fatal_error_handler_state.phpt b/tests/phpt/error_handler_reset_fatal_error_handler_state.phpt index eb23dfac5..eebc0c872 100644 --- a/tests/phpt/error_handler_reset_fatal_error_handler_state.phpt +++ b/tests/phpt/error_handler_reset_fatal_error_handler_state.phpt @@ -1,5 +1,5 @@ --TEST-- -Test that resetting the fatal error handler state re-arms OOM handling +Test that resetting the fatal error handler state re-arms OOM handling only when the reservation was released --FILE-- --EXPECT-- bool(false) bool(false) int(1234) +bool(false) +bool(false) +string(20) "existing reservation"