From 638c5dddd6f4c51c4025844ca3fbebf26e9919a9 Mon Sep 17 00:00:00 2001
From: Cees-Jan Kiewiet <ceesjank@gmail.com>
Date: Sat, 17 Feb 2024 11:17:31 +0100
Subject: [PATCH] [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
---
 src/Io/StreamingServer.php       | 13 +++++++---
 tests/Io/StreamingServerTest.php | 42 ++++++++++++++++++++++++++------
 2 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/src/Io/StreamingServer.php b/src/Io/StreamingServer.php
index 13f0b0c4..790c8cc1 100644
--- a/src/Io/StreamingServer.php
+++ b/src/Io/StreamingServer.php
@@ -157,10 +157,17 @@ public function handleRequest(ConnectionInterface $conn, ServerRequestInterface
         }
 
         // cancel pending promise once connection closes
+        $connectionOnCloseResponseCancelerHandler = function () {};
         if ($response instanceof PromiseInterface && \method_exists($response, 'cancel')) {
-            $conn->on('close', function () use ($response) {
+            $connectionOnCloseResponseCanceler = function () use ($response) {
                 $response->cancel();
-            });
+            };
+            $connectionOnCloseResponseCancelerHandler = function () use ($connectionOnCloseResponseCanceler, $conn) {
+                if ($connectionOnCloseResponseCanceler !== null) {
+                    $conn->removeListener('close', $connectionOnCloseResponseCanceler);
+                }
+            };
+            $conn->on('close', $connectionOnCloseResponseCanceler);
         }
 
         // happy path: response returned, handle and return immediately
@@ -201,7 +208,7 @@ function ($error) use ($that, $conn, $request) {
                 $that->emit('error', array($exception));
                 return $that->writeError($conn, Response::STATUS_INTERNAL_SERVER_ERROR, $request);
             }
-        );
+        )->then($connectionOnCloseResponseCancelerHandler, $connectionOnCloseResponseCancelerHandler);
     }
 
     /** @internal */
diff --git a/tests/Io/StreamingServerTest.php b/tests/Io/StreamingServerTest.php
index a2700b86..a578797e 100644
--- a/tests/Io/StreamingServerTest.php
+++ b/tests/Io/StreamingServerTest.php
@@ -25,9 +25,17 @@ class StreamingServerTest extends TestCase
      */
     public function setUpConnectionMockAndSocket()
     {
-        $this->connection = $this->getMockBuilder('React\Socket\Connection')
+        $this->connection = $this->mockConnection();
+
+        $this->socket = new SocketServerStub();
+    }
+
+
+    private function mockConnection(array $additionalMethods = null)
+    {
+        $connection = $this->getMockBuilder('React\Socket\Connection')
             ->disableOriginalConstructor()
-            ->setMethods(
+            ->setMethods(array_merge(
                 array(
                     'write',
                     'end',
@@ -39,14 +47,15 @@ public function setUpConnectionMockAndSocket()
                     'getRemoteAddress',
                     'getLocalAddress',
                     'pipe'
-                )
-            )
+                ),
+                (is_array($additionalMethods) ? $additionalMethods : array())
+            ))
             ->getMock();
 
-        $this->connection->method('isWritable')->willReturn(true);
-        $this->connection->method('isReadable')->willReturn(true);
+        $connection->method('isWritable')->willReturn(true);
+        $connection->method('isReadable')->willReturn(true);
 
-        $this->socket = new SocketServerStub();
+        return $connection;
     }
 
     public function testRequestEventWillNotBeEmittedForIncompleteHeaders()
@@ -3245,6 +3254,25 @@ public function testNewConnectionWillInvokeParserTwiceAfterInvokingRequestHandle
         $this->assertCount(1, $this->connection->listeners('close'));
     }
 
+    public function testCompletingARequestWillRemoveConnectionOnCloseListener()
+    {
+        $connection = $this->mockConnection(array('removeListener'));
+
+        $request = new ServerRequest('GET', 'http://localhost/');
+
+        $server = new StreamingServer(Loop::get(), function () {
+            return \React\Promise\resolve(new Response());
+        });
+
+        $server->listen($this->socket);
+        $this->socket->emit('connection', array($connection));
+
+        $connection->expects($this->once())->method('removeListener');
+
+        // pretend parser just finished parsing
+        $server->handleRequest($connection, $request);
+    }
+
     private function createGetRequest()
     {
         $data = "GET / HTTP/1.1\r\n";