Skip to content

Commit 006b41b

Browse files
committed
Fix closing socket resource before removing from loop
1 parent 2e86601 commit 006b41b

3 files changed

Lines changed: 63 additions & 1 deletion

File tree

src/Connection.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,10 @@ public function handleClose()
128128
// side already closed. Shutting down may return to blocking mode on
129129
// some legacy versions, so reset to non-blocking just in case before
130130
// continuing to close the socket resource.
131+
// Underlying Stream implementation will take care of closing file
132+
// handle, so we otherwise keep this open here.
131133
@stream_socket_shutdown($this->stream, STREAM_SHUT_RDWR);
132134
stream_set_blocking($this->stream, false);
133-
fclose($this->stream);
134135
}
135136

136137
public function getRemoteAddress()

tests/ConnectionTest.php

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
namespace React\Tests\Socket;
4+
5+
use React\Socket\Connection;
6+
7+
class ConnectionTest extends TestCase
8+
{
9+
public function testCloseConnectionWillCloseSocketResource()
10+
{
11+
$resource = fopen('php://memory', 'r+');
12+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
13+
14+
$connection = new Connection($resource, $loop);
15+
$connection->close();
16+
17+
$this->assertFalse(is_resource($resource));
18+
}
19+
20+
public function testCloseConnectionWillRemoveResourceFromLoopBeforeClosingResource()
21+
{
22+
$resource = fopen('php://memory', 'r+');
23+
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
24+
$loop->expects($this->once())->method('addWriteStream')->with($resource);
25+
26+
$onRemove = null;
27+
$loop->expects($this->once())->method('removeWriteStream')->with($this->callback(function ($param) use (&$onRemove) {
28+
$onRemove = is_resource($param);
29+
return true;
30+
}));
31+
32+
$connection = new Connection($resource, $loop);
33+
$connection->write('test');
34+
$connection->close();
35+
36+
$this->assertTrue($onRemove);
37+
$this->assertFalse(is_resource($resource));
38+
}
39+
}

tests/TcpConnectorTest.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,28 @@ public function connectionToTcpServerShouldSucceedWithNullAddressesAfterConnecti
9898
$this->assertNull($connection->getLocalAddress());
9999
}
100100

101+
/** @test */
102+
public function connectionToTcpServerWillCloseWhenOtherSideCloses()
103+
{
104+
$loop = Factory::create();
105+
106+
// immediately close connection and server once connection is in
107+
$server = new TcpServer(0, $loop);
108+
$server->on('connection', function (ConnectionInterface $conn) use ($server) {
109+
$conn->close();
110+
$server->close();
111+
});
112+
113+
$once = $this->expectCallableOnce();
114+
$connector = new TcpConnector($loop);
115+
$connector->connect($server->getAddress())->then(function (ConnectionInterface $conn) use ($once) {
116+
$conn->write('hello');
117+
$conn->on('close', $once);
118+
});
119+
120+
$loop->run();
121+
}
122+
101123
/** @test */
102124
public function connectionToEmptyIp6PortShouldFail()
103125
{

0 commit comments

Comments
 (0)