Skip to content

Commit

Permalink
Simplify logic by moving static notifier work-around to static method
Browse files Browse the repository at this point in the history
  • Loading branch information
clue committed Apr 25, 2018
1 parent dc798b3 commit 571d212
Showing 1 changed file with 25 additions and 10 deletions.
35 changes: 25 additions & 10 deletions src/Promise.php
Original file line number Diff line number Diff line change
Expand Up @@ -228,23 +228,14 @@ private function call(callable $callback)
if ($args === 0) {
$callback();
} else {
// Store a reference to all progress handlers (will be cleared when settled)
// This way, we can use a static progress callback that is not bound to this promise instance.
// This helps avoiding garbage cycles if the callback creates an Exception.
$progress =& $this->progressHandlers;

$callback(
function ($value = null) {
$this->resolve($value);
},
function ($reason = null) {
$this->reject($reason);
},
\Closure::bind(function ($update = null) use (&$progress) {
foreach ($progress as $handler) {
$handler($update);
}
}, null)
self::notifier($this->progressHandlers)
);
}
} catch (\Throwable $e) {
Expand All @@ -253,4 +244,28 @@ function ($reason = null) {
$this->reject($e);
}
}

/**
* Creates a static progress callback that is not bound to a promise instance.
*
* Moving the closure creation to a static method allows us to create a
* callback that is not bound to a promise instance. By passing its progress
* handlers by reference, we can still execute them when requested and still
* clear this reference when settling the promise. This helps avoiding
* garbage cycles if any callback creates an Exception.
*
* These assumptions are covered by the test suite, so if you ever feel like
* refactoring this, go ahead, any alternative suggestions are welcome!
*
* @param array $progressHandlers
* @return callable
*/
private static function notifier(&$progressHandlers)
{
return function ($update = null) use (&$progressHandlers) {
foreach ($progressHandlers as $handler) {
$handler($update);
}
};
}
}

0 comments on commit 571d212

Please sign in to comment.