Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #203 from juse-less/bugfix/refactor-earlier-file-h…
…andle-fix

Tiny alteration of the fix in #202
  • Loading branch information
Sammyjo20 committed Apr 28, 2023
2 parents c054777 + d16b992 commit 35e1243
Showing 1 changed file with 14 additions and 12 deletions.
26 changes: 14 additions & 12 deletions src/Helpers/MiddlewarePipeline.php
Expand Up @@ -48,14 +48,17 @@ public function __construct()
public function onRequest(callable $callable, bool $prepend = false, ?string $name = null): static
{
/**
* For some reason, PHP is not destructing Closures, or 'things' using Closures, correctly, keeping unused classes intact.
* Binding to an empty, anonymous class is a workaround for the issue.
* For some reason, PHP is not destructing non-static Closures, or 'things' using non-static Closures, correctly, keeping unused objects intact.
* Using a *static* Closure, or re-binding it to an empty, anonymous class/object is a workaround for the issue.
* If we don't, things using the MiddlewarePipeline, in turn, won't destruct.
* Concretely speaking, for Saloon, this means that the Connector will *not* get destructed, and thereby also not the underlying client.
* Which in turn leaves open file handles until the process terminates.
*
* Do note that this is entirely about our *wrapping* Closure below.
* The provided callable doesn't affect the MiddlewarePipeline.
*/

$callbackWrapper = Closure::bind(function (PendingRequest $pendingRequest) use ($callable): PendingRequest {
$this->requestPipeline->pipe(static function (PendingRequest $pendingRequest) use ($callable): PendingRequest {
$result = $callable($pendingRequest);

if ($result instanceof PendingRequest) {
Expand All @@ -67,9 +70,7 @@ public function onRequest(callable $callable, bool $prepend = false, ?string $na
}

return $pendingRequest;
}, new class {});

$this->requestPipeline = $this->requestPipeline->pipe($callbackWrapper, $prepend, $name);
}, $prepend, $name);

return $this;
}
Expand All @@ -86,20 +87,21 @@ public function onRequest(callable $callable, bool $prepend = false, ?string $na
public function onResponse(callable $callable, bool $prepend = false, ?string $name = null): static
{
/**
* For some reason, PHP is not destructing Closures, or 'things' using Closures, correctly, keeping unused classes intact.
* Binding to an empty, anonymous class is a workaround for the issue.
* For some reason, PHP is not destructing non-static Closures, or 'things' using non-static Closures, correctly, keeping unused objects intact.
* Using a *static* Closure, or re-binding it to an empty, anonymous class/object is a workaround for the issue.
* If we don't, things using the MiddlewarePipeline, in turn, won't destruct.
* Concretely speaking, for Saloon, this means that the Connector will *not* get destructed, and thereby also not the underlying client.
* Which in turn leaves open file handles until the process terminates.
*
* Do note that this is entirely about our *wrapping* Closure below.
* The provided callable doesn't affect the MiddlewarePipeline.
*/

$callbackWrapper = Closure::bind(function (Response $response) use ($callable): Response {
$this->responsePipeline->pipe(static function (Response $response) use ($callable): Response {
$result = $callable($response);

return $result instanceof Response ? $result : $response;
}, new class {});

$this->responsePipeline = $this->responsePipeline->pipe($callbackWrapper, $prepend, $name);
}, $prepend, $name);

return $this;
}
Expand Down

0 comments on commit 35e1243

Please sign in to comment.