From 7378d2e23533a5d724c456474754c48bf9e6d5b5 Mon Sep 17 00:00:00 2001 From: Josh Di Fabio Date: Fri, 29 Apr 2016 13:24:47 +0100 Subject: [PATCH 1/2] De-reference downstream promises when cancelled --- src/Promise.php | 35 ++++++++++++++++++++++++------- tests/CancellationMemLeakTest.php | 20 ++++++++++++++++++ 2 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 tests/CancellationMemLeakTest.php diff --git a/src/Promise.php b/src/Promise.php index bfbdc06d..dea948d9 100644 --- a/src/Promise.php +++ b/src/Promise.php @@ -13,6 +13,8 @@ class Promise implements ExtendedPromiseInterface, CancellablePromiseInterface private $requiredCancelRequests = 0; private $cancelRequests = 0; + private $nextHandlerId = '0'; + public function __construct(callable $resolver, callable $canceller = null) { $this->canceller = $canceller; @@ -25,13 +27,26 @@ public function then(callable $onFulfilled = null, callable $onRejected = null, return $this->result()->then($onFulfilled, $onRejected, $onProgress); } + $handlerId = $this->nextHandlerId; + $cleanUpFn = function () use ($handlerId) { + if (isset($this->handlers[$handlerId])) { + unset($this->handlers[$handlerId]); + } + if (isset($this->progressHandlers[$handlerId])) { + unset($this->progressHandlers[$handlerId]); + } + }; + $this->nextHandlerId++; + if (null === $this->canceller) { - return new static($this->resolver($onFulfilled, $onRejected, $onProgress)); + return new static($this->resolver($handlerId, $onFulfilled, $onRejected, $onProgress), $cleanUpFn); } $this->requiredCancelRequests++; - return new static($this->resolver($onFulfilled, $onRejected, $onProgress), function () { + return new static($this->resolver($handlerId, $onFulfilled, $onRejected, $onProgress), function () use ($cleanUpFn) { + $cleanUpFn(); + if (++$this->cancelRequests < $this->requiredCancelRequests) { return; } @@ -46,14 +61,18 @@ public function done(callable $onFulfilled = null, callable $onRejected = null, return $this->result()->done($onFulfilled, $onRejected, $onProgress); } - $this->handlers[] = function (ExtendedPromiseInterface $promise) use ($onFulfilled, $onRejected) { + $handlerId = $this->nextHandlerId; + + $this->handlers[$this->nextHandlerId] = function (ExtendedPromiseInterface $promise) use ($onFulfilled, $onRejected) { $promise ->done($onFulfilled, $onRejected); }; if ($onProgress) { - $this->progressHandlers[] = $onProgress; + $this->progressHandlers[$handlerId] = $onProgress; } + + $this->nextHandlerId++; } public function otherwise(callable $onRejected) @@ -97,9 +116,9 @@ public function cancel() $this->call($canceller); } - private function resolver(callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null) + private function resolver($handlerId, callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null) { - return function ($resolve, $reject, $notify) use ($onFulfilled, $onRejected, $onProgress) { + return function ($resolve, $reject, $notify) use ($handlerId, $onFulfilled, $onRejected, $onProgress) { if ($onProgress) { $progressHandler = function ($update) use ($notify, $onProgress) { try { @@ -114,13 +133,13 @@ private function resolver(callable $onFulfilled = null, callable $onRejected = n $progressHandler = $notify; } - $this->handlers[] = function (ExtendedPromiseInterface $promise) use ($onFulfilled, $onRejected, $resolve, $reject, $progressHandler) { + $this->handlers[$handlerId] = function (ExtendedPromiseInterface $promise) use ($onFulfilled, $onRejected, $resolve, $reject, $progressHandler) { $promise ->then($onFulfilled, $onRejected) ->done($resolve, $reject, $progressHandler); }; - $this->progressHandlers[] = $progressHandler; + $this->progressHandlers[$handlerId] = $progressHandler; }; } diff --git a/tests/CancellationMemLeakTest.php b/tests/CancellationMemLeakTest.php new file mode 100644 index 00000000..b65a11f6 --- /dev/null +++ b/tests/CancellationMemLeakTest.php @@ -0,0 +1,20 @@ +promise()->then(), $innerDeferred->promise()]); + $innerDeferred->resolve(); + } + + $this->assertLessThan(2 * $memoryUsage, memory_get_usage()); + } +} From cfc6bcaa6602d4c6afcadc59c1811cdf08a87315 Mon Sep 17 00:00:00 2001 From: Josh Di Fabio Date: Fri, 29 Apr 2016 13:32:31 +0100 Subject: [PATCH 2/2] Remove unnecessary assignment --- src/Promise.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Promise.php b/src/Promise.php index dea948d9..f099768e 100644 --- a/src/Promise.php +++ b/src/Promise.php @@ -61,15 +61,13 @@ public function done(callable $onFulfilled = null, callable $onRejected = null, return $this->result()->done($onFulfilled, $onRejected, $onProgress); } - $handlerId = $this->nextHandlerId; - $this->handlers[$this->nextHandlerId] = function (ExtendedPromiseInterface $promise) use ($onFulfilled, $onRejected) { $promise ->done($onFulfilled, $onRejected); }; if ($onProgress) { - $this->progressHandlers[$handlerId] = $onProgress; + $this->progressHandlers[$this->nextHandlerId] = $onProgress; } $this->nextHandlerId++;