Skip to content

Commit 4dfc331

Browse files
committed
[1.x] Ensure connection close handler is cleaned up for each request
This changeset resolves a small memory leak that causes roughly 1KB per connection tops. Which isn't a big issue but will make memory fluctuate more. The changeset doesn't introduce any performance degradation. Resolves: #514 Builds on top of: #405, #467, and many others
1 parent 706edec commit 4dfc331

File tree

2 files changed

+47
-3
lines changed

2 files changed

+47
-3
lines changed

src/Io/StreamingServer.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,12 @@ public function handleRequest(ConnectionInterface $conn, ServerRequestInterface
157157
}
158158

159159
// cancel pending promise once connection closes
160+
$connectionOnCloseResponseCanceler = null;
160161
if ($response instanceof PromiseInterface && \method_exists($response, 'cancel')) {
161-
$conn->on('close', function () use ($response) {
162+
$connectionOnCloseResponseCanceler = function () use ($response) {
162163
$response->cancel();
163-
});
164+
};
165+
$conn->on('close', $connectionOnCloseResponseCanceler);
164166
}
165167

166168
// happy path: response returned, handle and return immediately
@@ -201,7 +203,11 @@ function ($error) use ($that, $conn, $request) {
201203
$that->emit('error', array($exception));
202204
return $that->writeError($conn, Response::STATUS_INTERNAL_SERVER_ERROR, $request);
203205
}
204-
);
206+
)->always(function () use ($connectionOnCloseResponseCanceler, $conn) {
207+
if ($connectionOnCloseResponseCanceler !== null) {
208+
$conn->removeListener('close', $connectionOnCloseResponseCanceler);
209+
}
210+
});
205211
}
206212

207213
/** @internal */

tests/Io/StreamingServerTest.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3245,6 +3245,44 @@ public function testNewConnectionWillInvokeParserTwiceAfterInvokingRequestHandle
32453245
$this->assertCount(1, $this->connection->listeners('close'));
32463246
}
32473247

3248+
public function testCompletingARequestWillRemoveConnectionOnCloseListener()
3249+
{
3250+
$connection = $this->getMockBuilder('React\Socket\Connection')
3251+
->disableOriginalConstructor()
3252+
->setMethods(
3253+
array(
3254+
'write',
3255+
'end',
3256+
'close',
3257+
'pause',
3258+
'resume',
3259+
'isReadable',
3260+
'isWritable',
3261+
'getRemoteAddress',
3262+
'getLocalAddress',
3263+
'pipe',
3264+
'removeListener'
3265+
)
3266+
)
3267+
->getMock();
3268+
3269+
$connection->method('isWritable')->willReturn(true);
3270+
$connection->method('isReadable')->willReturn(true);
3271+
$request = new ServerRequest('GET', 'http://localhost/');
3272+
3273+
$server = new StreamingServer(Loop::get(), function () {
3274+
return \React\Promise\resolve(new Response());
3275+
});
3276+
3277+
$server->listen($this->socket);
3278+
$this->socket->emit('connection', array($connection));
3279+
3280+
$connection->expects($this->once())->method('removeListener');
3281+
3282+
// pretend parser just finished parsing
3283+
$server->handleRequest($connection, $request);
3284+
}
3285+
32483286
private function createGetRequest()
32493287
{
32503288
$data = "GET / HTTP/1.1\r\n";

0 commit comments

Comments
 (0)