Skip to content

Commit 6aaedfa

Browse files
GrahamCampbellGrahamCampbell
authored
memory leak fix (#194)
Fix memory leak recreating new chain instances
1 parent 38b66ea commit 6aaedfa

File tree

3 files changed

+164
-28
lines changed

3 files changed

+164
-28
lines changed

‎src/PluginChain.php

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Http\Client\Common;
6+
7+
use function array_reverse;
8+
use Http\Client\Common\Exception\LoopException;
9+
use Http\Promise\Promise;
10+
use Psr\Http\Message\RequestInterface;
11+
12+
final class PluginChain
13+
{
14+
/** @var Plugin[] */
15+
private $plugins;
16+
17+
/** @var callable */
18+
private $clientCallable;
19+
20+
/** @var int */
21+
private $maxRestarts;
22+
23+
/** @var int */
24+
private $restarts = 0;
25+
26+
/**
27+
* @param Plugin[] $plugins A plugin chain
28+
* @param callable $clientCallable Callable making the HTTP call
29+
* @param array $options {
30+
*
31+
* @var int $max_restarts
32+
* }
33+
*/
34+
public function __construct(array $plugins, callable $clientCallable, array $options = [])
35+
{
36+
$this->plugins = $plugins;
37+
$this->clientCallable = $clientCallable;
38+
$this->maxRestarts = (int) ($options['max_restarts'] ?? 0);
39+
}
40+
41+
private function createChain(): callable
42+
{
43+
$lastCallable = $this->clientCallable;
44+
$reversedPlugins = array_reverse($this->plugins);
45+
46+
foreach ($reversedPlugins as $plugin) {
47+
$lastCallable = function (RequestInterface $request) use ($plugin, $lastCallable) {
48+
return $plugin->handleRequest($request, $lastCallable, $this);
49+
};
50+
}
51+
52+
return $lastCallable;
53+
}
54+
55+
public function __invoke(RequestInterface $request): Promise
56+
{
57+
if ($this->restarts > $this->maxRestarts) {
58+
throw new LoopException('Too many restarts in plugin client', $request);
59+
}
60+
61+
++$this->restarts;
62+
63+
return $this->createChain()($request);
64+
}
65+
}

‎src/PluginClient.php

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
namespace Http\Client\Common;
66

7-
use Http\Client\Common\Exception\LoopException;
87
use Http\Client\Exception as HttplugException;
98
use Http\Client\HttpAsyncClient;
109
use Http\Client\HttpClient;
@@ -44,11 +43,11 @@ final class PluginClient implements HttpClient, HttpAsyncClient
4443
private $options;
4544

4645
/**
47-
* @param ClientInterface|HttpAsyncClient $client
48-
* @param Plugin[] $plugins
46+
* @param ClientInterface|HttpAsyncClient $client An HTTP async client
47+
* @param Plugin[] $plugins A plugin chain
4948
* @param array $options {
5049
*
51-
* @var int $max_restarts
50+
* @var int $max_restarts
5251
* }
5352
*/
5453
public function __construct($client, array $plugins = [], array $options = [])
@@ -120,32 +119,11 @@ private function configure(array $options = []): array
120119
/**
121120
* Create the plugin chain.
122121
*
123-
* @param Plugin[] $pluginList A list of plugins
122+
* @param Plugin[] $plugins A plugin chain
124123
* @param callable $clientCallable Callable making the HTTP call
125124
*/
126-
private function createPluginChain(array $pluginList, callable $clientCallable): callable
125+
private function createPluginChain(array $plugins, callable $clientCallable): callable
127126
{
128-
$firstCallable = $lastCallable = $clientCallable;
129-
130-
while ($plugin = array_pop($pluginList)) {
131-
$lastCallable = function (RequestInterface $request) use ($plugin, $lastCallable, &$firstCallable) {
132-
return $plugin->handleRequest($request, $lastCallable, $firstCallable);
133-
};
134-
135-
$firstCallable = $lastCallable;
136-
}
137-
138-
$firstCalls = 0;
139-
$firstCallable = function (RequestInterface $request) use ($lastCallable, &$firstCalls) {
140-
if ($firstCalls > $this->options['max_restarts']) {
141-
throw new LoopException('Too many restarts in plugin client', $request);
142-
}
143-
144-
++$firstCalls;
145-
146-
return $lastCallable($request);
147-
};
148-
149-
return $firstCallable;
127+
return new PluginChain($plugins, $clientCallable, $this->options);
150128
}
151129
}

‎tests/PluginChainTest.php

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace tests\Http\Client\Common;
6+
7+
use Http\Client\Common\Exception\LoopException;
8+
use Http\Client\Common\Plugin;
9+
use Http\Client\Common\PluginChain;
10+
use Http\Promise\Promise;
11+
use PHPUnit\Framework\TestCase;
12+
use Prophecy\Argument;
13+
use Psr\Http\Message\RequestInterface;
14+
15+
class PluginChainTest extends TestCase
16+
{
17+
private function createPlugin(callable $func): Plugin
18+
{
19+
return new class ($func) implements Plugin
20+
{
21+
public $func;
22+
23+
public function __construct(callable $func)
24+
{
25+
$this->func = $func;
26+
}
27+
28+
public function handleRequest(RequestInterface $request, callable $next, callable $first): Promise
29+
{
30+
($this->func)($request, $next, $first);
31+
32+
return $next($request);
33+
}
34+
};
35+
}
36+
37+
public function testChainShouldInvokePluginsInReversedOrder(): void
38+
{
39+
$pluginOrderCalls = [];
40+
41+
$plugin1 = $this->createPlugin(static function () use (&$pluginOrderCalls) {
42+
$pluginOrderCalls[] = 'plugin1';
43+
});
44+
$plugin2 = $this->createPlugin(static function () use (&$pluginOrderCalls) {
45+
$pluginOrderCalls[] = 'plugin2';
46+
});
47+
48+
$request = $this->prophesize(RequestInterface::class);
49+
$responsePromise = $this->prophesize(Promise::class);
50+
51+
$clientCallable = static function () use ($responsePromise) {
52+
return $responsePromise->reveal();
53+
};
54+
55+
$pluginOrderCalls = [];
56+
57+
$plugins = [
58+
$plugin1,
59+
$plugin2,
60+
];
61+
62+
$pluginChain = new PluginChain($plugins, $clientCallable);
63+
64+
$result = $pluginChain($request->reveal());
65+
66+
$this->assertSame($responsePromise->reveal(), $result);
67+
$this->assertSame(['plugin1', 'plugin2'], $pluginOrderCalls);
68+
}
69+
70+
public function testShouldThrowLoopExceptionOnMaxRestarts(): void
71+
{
72+
$this->expectException(LoopException::class);
73+
74+
$request = $this->prophesize(RequestInterface::class);
75+
$responsePromise = $this->prophesize(Promise::class);
76+
$calls = 0;
77+
$clientCallable = static function () use ($responsePromise, &$calls) {
78+
++$calls;
79+
80+
return $responsePromise->reveal();
81+
};
82+
83+
$pluginChain = new PluginChain([], $clientCallable, ['max_restarts' => 2]);
84+
85+
$pluginChain($request->reveal());
86+
$this->assertSame(1, $calls);
87+
$pluginChain($request->reveal());
88+
$this->assertSame(2, $calls);
89+
$pluginChain($request->reveal());
90+
$this->assertSame(3, $calls);
91+
$pluginChain($request->reveal());
92+
}
93+
}

0 commit comments

Comments
 (0)