Skip to content

Commit

Permalink
Fix Fiber leaks if Suspension never resumed, v2 (#82)
Browse files Browse the repository at this point in the history
  • Loading branch information
trowski authored Jul 20, 2023
1 parent 8405671 commit b018d0f
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 22 deletions.
2 changes: 1 addition & 1 deletion .php-cs-fixer.dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public function getRules(): array
"allow_single_line_closure" => true,
],
"array_syntax" => ["syntax" => "short"],
"blank_lines_before_namespace" => true,
"cast_spaces" => true,
"combine_consecutive_unsets" => true,
"declare_strict_types" => true,
Expand Down Expand Up @@ -73,7 +74,6 @@ public function getRules(): array
"psr_autoloading" => ['dir' => $this->src],
"return_type_declaration" => ["space_before" => "none"],
"short_scalar_cast" => true,
"single_blank_line_before_namespace" => true,
"line_ending" => true,
];
}
Expand Down
13 changes: 11 additions & 2 deletions src/EventLoop/Internal/AbstractDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,21 @@ public function getSuspension(): Suspension
\assert($fiber !== $this->fiber);

// Use current object in case of {main}
return $this->suspensions[$fiber ?? $this] ??= new DriverSuspension(
$suspension = ($this->suspensions[$fiber ?? $this] ?? null)?->get();
if ($suspension) {
return $suspension;
}

$suspension = new DriverSuspension(
$this->runCallback,
$this->queueCallback,
$this->interruptCallback,
$this->suspensions
$this->suspensions,
);

$this->suspensions[$fiber ?? $this] = \WeakReference::create($suspension);

return $suspension;
}

public function setErrorHandler(?\Closure $errorHandler): void
Expand Down
23 changes: 6 additions & 17 deletions src/EventLoop/Internal/DriverSuspension.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,15 @@ final class DriverSuspension implements Suspension

private bool $pending = false;

private readonly \WeakReference $suspensions;

/**
* @param \Closure $run
* @param \Closure $queue
* @param \Closure $interrupt
*
* @internal
*/
public function __construct(
private readonly \Closure $run,
private readonly \Closure $queue,
private readonly \Closure $interrupt,
\WeakMap $suspensions
private readonly \WeakMap $suspensions,
) {
$fiber = \Fiber::getCurrent();

$this->fiberRef = $fiber ? \WeakReference::create($fiber) : null;
$this->suspensions = \WeakReference::create($suspensions);
}

public function resume(mixed $value = null): void
Expand Down Expand Up @@ -101,13 +91,12 @@ public function suspend(): mixed
$this->pending = false;
$result && $result(); // Unwrap any uncaught exceptions from the event loop

$info = '';
$suspensions = $this->suspensions->get();
if ($suspensions) {
\gc_collect_cycles();
\gc_collect_cycles(); // Collect any circular references before dumping pending suspensions.

/** @var self $suspension */
foreach ($suspensions as $suspension) {
$info = '';
foreach ($this->suspensions as $suspensionRef) {
if ($suspension = $suspensionRef->get()) {
\assert($suspension instanceof self);
$fiber = $suspension->fiberRef?->get();
if ($fiber === null) {
continue;
Expand Down
3 changes: 1 addition & 2 deletions test/EventLoopTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,7 @@ public function testSuspensionWithinCallbackGarbageCollectionSuspended(): void

\gc_collect_cycles();

// This documents an expected failure, should actually be true, but suspensions have to be resumed currently.
self::assertNull($finally);
self::assertTrue($finally);
}

public function testSuspensionWithinQueue(): void
Expand Down

0 comments on commit b018d0f

Please sign in to comment.