From 006b41b858d3d2bc368b08094f9b19d19f2820b4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20L=C3=BCck?= <christian@lueck.tv>
Date: Sat, 23 Dec 2017 20:04:15 +0100
Subject: [PATCH 1/2] Fix closing socket resource before removing from loop

---
 src/Connection.php         |  3 ++-
 tests/ConnectionTest.php   | 39 ++++++++++++++++++++++++++++++++++++++
 tests/TcpConnectorTest.php | 22 +++++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 tests/ConnectionTest.php

diff --git a/src/Connection.php b/src/Connection.php
index fa7a2229..1f56c5d8 100644
--- a/src/Connection.php
+++ b/src/Connection.php
@@ -128,9 +128,10 @@ public function handleClose()
         // side already closed. Shutting down may return to blocking mode on
         // some legacy versions, so reset to non-blocking just in case before
         // continuing to close the socket resource.
+        // Underlying Stream implementation will take care of closing file
+        // handle, so we otherwise keep this open here.
         @stream_socket_shutdown($this->stream, STREAM_SHUT_RDWR);
         stream_set_blocking($this->stream, false);
-        fclose($this->stream);
     }
 
     public function getRemoteAddress()
diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php
new file mode 100644
index 00000000..49d53e26
--- /dev/null
+++ b/tests/ConnectionTest.php
@@ -0,0 +1,39 @@
+<?php
+
+namespace React\Tests\Socket;
+
+use React\Socket\Connection;
+
+class ConnectionTest extends TestCase
+{
+    public function testCloseConnectionWillCloseSocketResource()
+    {
+        $resource = fopen('php://memory', 'r+');
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+
+        $connection = new Connection($resource, $loop);
+        $connection->close();
+
+        $this->assertFalse(is_resource($resource));
+    }
+
+    public function testCloseConnectionWillRemoveResourceFromLoopBeforeClosingResource()
+    {
+        $resource = fopen('php://memory', 'r+');
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+        $loop->expects($this->once())->method('addWriteStream')->with($resource);
+
+        $onRemove = null;
+        $loop->expects($this->once())->method('removeWriteStream')->with($this->callback(function ($param) use (&$onRemove) {
+            $onRemove = is_resource($param);
+            return true;
+        }));
+
+        $connection = new Connection($resource, $loop);
+        $connection->write('test');
+        $connection->close();
+
+        $this->assertTrue($onRemove);
+        $this->assertFalse(is_resource($resource));
+    }
+}
diff --git a/tests/TcpConnectorTest.php b/tests/TcpConnectorTest.php
index 17c55240..424957e8 100644
--- a/tests/TcpConnectorTest.php
+++ b/tests/TcpConnectorTest.php
@@ -98,6 +98,28 @@ public function connectionToTcpServerShouldSucceedWithNullAddressesAfterConnecti
         $this->assertNull($connection->getLocalAddress());
     }
 
+    /** @test */
+    public function connectionToTcpServerWillCloseWhenOtherSideCloses()
+    {
+        $loop = Factory::create();
+
+        // immediately close connection and server once connection is in
+        $server = new TcpServer(0, $loop);
+        $server->on('connection', function (ConnectionInterface $conn) use ($server) {
+            $conn->close();
+            $server->close();
+        });
+
+        $once = $this->expectCallableOnce();
+        $connector = new TcpConnector($loop);
+        $connector->connect($server->getAddress())->then(function (ConnectionInterface $conn) use ($once) {
+            $conn->write('hello');
+            $conn->on('close', $once);
+        });
+
+        $loop->run();
+    }
+
     /** @test */
     public function connectionToEmptyIp6PortShouldFail()
     {

From 3ce31854e2a94a50df3591a8cb685478955a0b12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20L=C3=BCck?= <christian@lueck.tv>
Date: Sat, 23 Dec 2017 20:24:53 +0100
Subject: [PATCH 2/2] Skip memory stream tests on unsupported HHVM

---
 tests/ConnectionTest.php | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/ConnectionTest.php b/tests/ConnectionTest.php
index 49d53e26..d3563dfc 100644
--- a/tests/ConnectionTest.php
+++ b/tests/ConnectionTest.php
@@ -8,6 +8,10 @@ class ConnectionTest extends TestCase
 {
     public function testCloseConnectionWillCloseSocketResource()
     {
+        if (defined('HHVM_VERSION')) {
+            $this->markTestSkipped('HHVM does not support socket operation on test memory stream');
+        }
+
         $resource = fopen('php://memory', 'r+');
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
 
@@ -19,6 +23,10 @@ public function testCloseConnectionWillCloseSocketResource()
 
     public function testCloseConnectionWillRemoveResourceFromLoopBeforeClosingResource()
     {
+        if (defined('HHVM_VERSION')) {
+            $this->markTestSkipped('HHVM does not support socket operation on test memory stream');
+        }
+
         $resource = fopen('php://memory', 'r+');
         $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
         $loop->expects($this->once())->method('addWriteStream')->with($resource);