diff --git a/src/sentry/class_map/RuntimeContextManager.php b/src/sentry/class_map/RuntimeContextManager.php index 640f35e89..ad3b44c18 100644 --- a/src/sentry/class_map/RuntimeContextManager.php +++ b/src/sentry/class_map/RuntimeContextManager.php @@ -26,6 +26,8 @@ * - 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. + * - startContext() only takes effect inside a coroutine; in non-coroutine + * environments it is a no-op and the global fallback context is used. * - endContext() flushes context resources and removes that context. * * @internal @@ -120,6 +122,13 @@ public function hasActiveContext(): bool */ public function startContext(): void { + // The main coroutine (non-coroutine environment, Coroutine::id() <= 0) + // uses a process-level context store that is not reaped together with + // a coroutine, so falling back to the global context is safer there. + if (\Hyperf\Engine\Coroutine::id() <= 0) { + return; + } + $executionContextKey = $this->getExecutionContextKey(); if ($this->hasActiveContextForExecutionContextKey($executionContextKey)) { @@ -279,8 +288,11 @@ private function generateRuntimeContextId(): string private function getExecutionContextKey(): string { - // All supported runtime modes currently use a process-local execution key. - return self::PROCESS_EXECUTION_CONTEXT_KEY; + // The key is scoped per coroutine so execution context mappings cannot + // leak across coroutines or linger on the main coroutine. CoArrayObject + // already stores values per current coroutine, so normal behavior is + // unchanged. + return \sprintf('%s.%d', self::PROCESS_EXECUTION_CONTEXT_KEY, \Hyperf\Engine\Coroutine::id()); } private function getGlobalContext(): RuntimeContext diff --git a/tests/Sentry/RuntimeContextLifecycleTest.php b/tests/Sentry/RuntimeContextLifecycleTest.php new file mode 100644 index 000000000..6f4205b74 --- /dev/null +++ b/tests/Sentry/RuntimeContextLifecycleTest.php @@ -0,0 +1,137 @@ +createRuntimeContextManager(); + + $manager->startContext(); + + $this->assertTrue($manager->hasActiveContext()); + + $manager->endContext(); + + $this->assertFalse($manager->hasActiveContext()); + } + + #[NonCoroutine] + public function testStartContextIsIgnoredOnMainCoroutine(): void + { + $this->assertSame(-1, Coroutine::getCid()); + + $manager = $this->createRuntimeContextManager(); + + $manager->startContext(); + + // The main coroutine uses a process-level context store that is never + // reaped, so startContext() is a no-op and the global fallback is used. + $this->assertFalse($manager->hasActiveContext()); + $this->assertSame('global', $manager->getCurrentContext()->getId()); + } + + public function testContextsAreIsolatedAcrossCoroutines(): void + { + $manager = $this->createRuntimeContextManager(); + $channelA = new Channel(1); + $channelB = new Channel(1); + $result = []; + + Coroutine::create(function () use ($manager, $channelA, &$result): void { + $manager->startContext(); + $result['a_id'] = $manager->getCurrentContext()->getId(); + $result['a_active'] = $manager->hasActiveContext(); + $channelA->push('started'); + $channelA->pop(); // Wait for the "end A" signal. + $manager->endContext(); + $result['a_active_after_end'] = $manager->hasActiveContext(); + $channelA->push('done'); + }); + + Coroutine::create(function () use ($manager, $channelB, &$result): void { + $manager->startContext(); + $result['b_id'] = $manager->getCurrentContext()->getId(); + $result['b_active'] = $manager->hasActiveContext(); + $channelB->push('started'); + $channelB->pop(); // Wait for the "A ended" signal. + $result['b_active_after_a_end'] = $manager->hasActiveContext(); + $channelB->push('done'); + }); + + $channelA->pop(); // A started. + $channelB->pop(); // B started. + + $this->assertNotSame($result['a_id'], $result['b_id']); + $this->assertTrue($result['a_active']); + $this->assertTrue($result['b_active']); + + $channelA->push('end'); + $channelA->pop(); // A finished ending its context. + + $channelB->push('check'); + $channelB->pop(); // B verified its own context is still active. + + $this->assertFalse($result['a_active_after_end']); + $this->assertTrue($result['b_active_after_a_end']); + } + + public function testStartContextIsIdempotentWithinSameCoroutine(): void + { + $manager = $this->createRuntimeContextManager(); + + $manager->startContext(); + $firstId = $manager->getCurrentContext()->getId(); + $manager->startContext(); + + // A nested start for the same execution key is a no-op. + $this->assertSame($firstId, $manager->getCurrentContext()->getId()); + $this->assertTrue($manager->hasActiveContext()); + + $manager->endContext(); + + $this->assertFalse($manager->hasActiveContext()); + } + + private function createRuntimeContextManager(): RuntimeContextManager + { + $client = Mockery::mock(ClientInterface::class); + $client->shouldReceive('getOptions')->andReturn(new Options()); + $client->shouldReceive('flush')->andReturn(new Result(ResultStatus::success())); + + $hub = Mockery::mock(HubInterface::class); + $hub->shouldReceive('getClient')->andReturn($client); + + return new RuntimeContextManager($hub); + } +}