Skip to content

Commit 04e5542

Browse files
clueclue
committed
Improve memory consumption for failed connection attempts
1 parent e8e53d6 commit 04e5542

File tree

3 files changed

+161
-4
lines changed

3 files changed

+161
-4
lines changed

‎composer.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
"react/dns": "^0.4.13",
1010
"react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3.5",
1111
"react/stream": "^1.0 || ^0.7.1",
12-
"react/promise": "^2.1 || ^1.2",
13-
"react/promise-timer": "~1.0"
12+
"react/promise": "^2.6.0 || ^1.2.1",
13+
"react/promise-timer": "dev-master as 1.4.0"
1414
},
1515
"require-dev": {
1616
"clue/block-react": "^1.2",

‎src/TcpConnector.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ private function waitForStreamOnce($stream)
112112
$resolve(new Connection($stream, $loop));
113113
}
114114
});
115-
}, function ($resolve, $reject, $progress) use ($loop, $stream) {
115+
}, function () use ($loop, $stream) {
116116
$loop->removeWriteStream($stream);
117117
fclose($stream);
118118

@@ -123,7 +123,6 @@ private function waitForStreamOnce($stream)
123123
}
124124
// @codeCoverageIgnoreEnd
125125

126-
$resolve = $reject = $progress = null;
127126
throw new RuntimeException('Cancelled while waiting for TCP/IP connection to be established');
128127
});
129128
}

‎tests/IntegrationTest.php

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,164 @@ public function testConnectingFailsIfDnsUsesInvalidResolver()
114114
Block\await($connector->connect('google.com:80'), $loop, self::TIMEOUT);
115115
}
116116

117+
public function testCancellingPendingConnectionWithoutTimeoutShouldNotCreateAnyGarbageReferences()
118+
{
119+
if (class_exists('React\Promise\When')) {
120+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
121+
}
122+
123+
$loop = Factory::create();
124+
$connector = new Connector($loop, array('timeout' => false));
125+
126+
gc_collect_cycles();
127+
$promise = $connector->connect('8.8.8.8:80');
128+
$promise->cancel();
129+
unset($promise);
130+
131+
$this->assertEquals(0, gc_collect_cycles());
132+
}
133+
134+
public function testCancellingPendingConnectionShouldNotCreateAnyGarbageReferences()
135+
{
136+
if (class_exists('React\Promise\When')) {
137+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
138+
}
139+
140+
$loop = Factory::create();
141+
$connector = new Connector($loop);
142+
143+
gc_collect_cycles();
144+
$promise = $connector->connect('8.8.8.8:80');
145+
$promise->cancel();
146+
unset($promise);
147+
148+
$this->assertEquals(0, gc_collect_cycles());
149+
}
150+
151+
public function testWaitingForRejectedConnectionShouldNotCreateAnyGarbageReferences()
152+
{
153+
if (class_exists('React\Promise\When')) {
154+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
155+
}
156+
157+
$loop = Factory::create();
158+
$connector = new Connector($loop, array('timeout' => false));
159+
160+
gc_collect_cycles();
161+
162+
$wait = true;
163+
$promise = $connector->connect('127.0.0.1:1')->then(
164+
null,
165+
function ($e) use (&$wait) {
166+
$wait = false;
167+
throw $e;
168+
}
169+
);
170+
171+
// run loop for short period to ensure we detect connection refused error
172+
Block\sleep(0.01, $loop);
173+
if ($wait) {
174+
Block\sleep(0.2, $loop);
175+
if ($wait) {
176+
$this->fail('Connection attempt did not fail');
177+
}
178+
}
179+
unset($promise);
180+
181+
$this->assertEquals(0, gc_collect_cycles());
182+
}
183+
184+
/**
185+
* @requires PHP 7
186+
*/
187+
public function testWaitingForConnectionTimeoutShouldNotCreateAnyGarbageReferences()
188+
{
189+
if (class_exists('React\Promise\When')) {
190+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
191+
}
192+
193+
$loop = Factory::create();
194+
$connector = new Connector($loop, array('timeout' => 0.001));
195+
196+
gc_collect_cycles();
197+
198+
$wait = true;
199+
$promise = $connector->connect('google.com:80')->then(
200+
null,
201+
function ($e) use (&$wait) {
202+
$wait = false;
203+
throw $e;
204+
}
205+
);
206+
207+
// run loop for short period to ensure we detect connection timeout error
208+
Block\sleep(0.01, $loop);
209+
if ($wait) {
210+
Block\sleep(0.2, $loop);
211+
if ($wait) {
212+
$this->fail('Connection attempt did not fail');
213+
}
214+
}
215+
var_dump($promise);
216+
unset($promise);
217+
218+
$this->assertEquals(0, gc_collect_cycles());
219+
}
220+
221+
public function testWaitingForInvalidDnsConnectionShouldNotCreateAnyGarbageReferences()
222+
{
223+
if (class_exists('React\Promise\When')) {
224+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
225+
}
226+
227+
$loop = Factory::create();
228+
$connector = new Connector($loop, array('timeout' => false));
229+
230+
gc_collect_cycles();
231+
232+
$wait = true;
233+
$promise = $connector->connect('example.invalid:80')->then(
234+
null,
235+
function ($e) use (&$wait) {
236+
$wait = false;
237+
throw $e;
238+
}
239+
);
240+
241+
// run loop for short period to ensure we detect DNS error
242+
Block\sleep(0.01, $loop);
243+
if ($wait) {
244+
Block\sleep(0.2, $loop);
245+
if ($wait) {
246+
$this->fail('Connection attempt did not fail');
247+
}
248+
}
249+
unset($promise);
250+
251+
$this->assertEquals(0, gc_collect_cycles());
252+
}
253+
254+
public function testWaitingForSuccessfullyClosedConnectionShouldNotCreateAnyGarbageReferences()
255+
{
256+
if (class_exists('React\Promise\When')) {
257+
$this->markTestSkipped('Not supported on legacy Promise v1 API');
258+
}
259+
260+
$loop = Factory::create();
261+
$connector = new Connector($loop, array('timeout' => false));
262+
263+
gc_collect_cycles();
264+
$promise = $connector->connect('google.com:80')->then(
265+
function ($conn) {
266+
$conn->close();
267+
}
268+
);
269+
Block\await($promise, $loop, self::TIMEOUT);
270+
unset($promise);
271+
272+
$this->assertEquals(0, gc_collect_cycles());
273+
}
274+
117275
public function testConnectingFailsIfTimeoutIsTooSmall()
118276
{
119277
if (!function_exists('stream_socket_enable_crypto')) {

0 commit comments

Comments
 (0)