From 14edcdee8002aee3b5f2c164854773b71d32fb59 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 5 Nov 2025 15:36:43 +0100 Subject: [PATCH] Fix memory leak in Deferred by clearing closure references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit addresses issue #972 where using Deferred objects caused excessive memory usage due to closures holding references to their captured context. Changes: - Modified SyncPromise constructor to clear executor reference after execution using pass-by-reference and finally block - Modified enqueueWaitingPromises() to avoid capturing $this in closures by extracting state and result to local variables and making the closure static - Added unset($task) in runQueue() to explicitly clear task references after execution - Added test case to verify memory doesn't leak across multiple query executions The fix reduces memory usage and ensures proper cleanup of closure references after promise resolution, preventing memory from accumulating across multiple query executions. Fixes #972 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/Executor/Promise/Adapter/SyncPromise.php | 34 ++++--- tests/Executor/DeferredFieldsTest.php | 99 ++++++++++++++++++++ 2 files changed, 119 insertions(+), 14 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 9bc1ac079..c0cee6cf1 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -48,6 +48,8 @@ public static function runQueue(): void while (! $q->isEmpty()) { $task = $q->dequeue(); $task(); + // Explicitly clear the task reference to help garbage collection + unset($task); } } @@ -58,11 +60,16 @@ public function __construct(?callable $executor = null) return; } - self::getQueue()->enqueue(function () use ($executor): void { + self::getQueue()->enqueue(function () use (&$executor): void { try { + assert(is_callable($executor)); $this->resolve($executor()); } catch (\Throwable $e) { $this->reject($e); + } finally { + // Clear the executor reference to allow garbage collection + // of the closure and its captured context + $executor = null; } }); } @@ -143,26 +150,25 @@ private function enqueueWaitingPromises(): void throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending'); } + $state = $this->state; + $result = $this->result; + foreach ($this->waiting as $descriptor) { - self::getQueue()->enqueue(function () use ($descriptor): void { + self::getQueue()->enqueue(static function () use ($descriptor, $state, $result): void { [$promise, $onFulfilled, $onRejected] = $descriptor; - if ($this->state === self::FULFILLED) { - try { - $promise->resolve($onFulfilled === null ? $this->result : $onFulfilled($this->result)); - } catch (\Throwable $e) { - $promise->reject($e); - } - } elseif ($this->state === self::REJECTED) { - try { + try { + if ($state === self::FULFILLED) { + $promise->resolve($onFulfilled === null ? $result : $onFulfilled($result)); + } elseif ($state === self::REJECTED) { if ($onRejected === null) { - $promise->reject($this->result); + $promise->reject($result); } else { - $promise->resolve($onRejected($this->result)); + $promise->resolve($onRejected($result)); } - } catch (\Throwable $e) { - $promise->reject($e); } + } catch (\Throwable $e) { + $promise->reject($e); } }); } diff --git a/tests/Executor/DeferredFieldsTest.php b/tests/Executor/DeferredFieldsTest.php index a66a44952..a6c8df50e 100644 --- a/tests/Executor/DeferredFieldsTest.php +++ b/tests/Executor/DeferredFieldsTest.php @@ -668,4 +668,103 @@ private function assertPathsMatch(array $expectedPaths): void self::assertContains($expectedPath, $this->paths, 'Missing path: ' . json_encode($expectedPath, JSON_THROW_ON_ERROR)); } } + + public function testDeferredMemoryUsage(): void + { + // Generate test data similar to the issue reproduction + $authors = []; + for ($i = 0; $i <= 100; ++$i) { + $authors[$i] = ['name' => 'Name ' . $i]; + } + + $books = []; + for ($i = 0; $i <= 1000; ++$i) { + $books[$i] = ['title' => 'Title ' . $i, 'authorId' => random_int(0, 100)]; + } + + $authorType = new ObjectType([ + 'name' => 'Author', + 'fields' => [ + 'name' => [ + 'type' => Type::string(), + ], + ], + ]); + + $bookType = new ObjectType([ + 'name' => 'Book', + 'fields' => [ + 'title' => [ + 'type' => Type::string(), + ], + 'author' => [ + 'type' => $authorType, + 'resolve' => static fn ($rootValue): Deferred => new Deferred(static fn (): array => $authors[$rootValue['authorId']]), + ], + ], + ]); + + $queryType = new ObjectType([ + 'name' => 'Query', + 'fields' => [ + 'getBooks' => [ + 'type' => Type::listOf($bookType), + 'resolve' => static fn (): array => $books, + ], + ], + ]); + + $schema = new Schema([ + 'query' => $queryType, + ]); + + $query = Parser::parse(' + { + getBooks { + title + author { + name + } + } + } + '); + + // Run the query multiple times to detect memory leaks + // If there's a leak, memory will grow with each iteration + $memoryMeasurements = []; + + for ($iteration = 0; $iteration < 3; ++$iteration) { + gc_collect_cycles(); + $memoryBefore = memory_get_usage(); + + $result = Executor::execute($schema, $query); + + // Verify the query executed successfully + self::assertArrayNotHasKey('errors', $result->toArray()); + + $memoryAfter = memory_get_usage(); + $memoryMeasurements[$iteration] = $memoryAfter - $memoryBefore; + + // Clear result to prepare for next iteration + unset($result); + } + + // With proper cleanup, memory usage should be stable across iterations + // Allow some variation (10%) but memory shouldn't grow significantly + $firstIteration = $memoryMeasurements[0]; + $lastIteration = $memoryMeasurements[2]; + $memoryGrowth = $lastIteration - $firstIteration; + $allowedGrowth = $firstIteration * 0.10; // 10% tolerance + + self::assertLessThan( + $allowedGrowth, + $memoryGrowth, + sprintf( + 'Memory leak detected: memory grew by %.2fMB across iterations (%.2fMB -> %.2fMB)', + $memoryGrowth / 1024 / 1024, + $firstIteration / 1024 / 1024, + $lastIteration / 1024 / 1024 + ) + ); + } }