Skip to content

Commit 9af6703

Browse files
author
Vita Batrla
committed
test: fix test-net-connect-reset-until-connected
Fixes a race condition in test that causes the test to randomly timeout on Solaris 11.4, SmartOS and potentially also FreeBSD. The client resets the connection using conn.resetAndDestroy(). This call is asynchronous and if it's effect occurs before server's listening socket accepts the connection, the test hangs. The fix is to put a synchronization barrier that resets the connection only after it is established on both server and client side. Below is a little bit more about the root cause. I show positive (test works) and negative (when test hangs) scenarios. The output contains only relevant system / library calls that were collected using truss(1). Without the fix the test randomly hangs. With the fix the test completes thousands of runs without single issue. Race condition scenario: ``` connect(23, 0x7FFFBFFF7F10, 32, SOV_XPG4_2)Err#150 EINPROGRESS ^ client socket connects to server close(23)= 0 ^ client closes the socket too early accept(21, 0x00000000, 0x00000000, SOV_DEFAULT)Err#130 ECONNABORTED accept(21, 0x00000000, 0x00000000, SOV_DEFAULT)Err#11 EAGAIN ^ accept on listening socket fails ... test hangs and times out... ``` Working (good) scenario: ``` connect(23, 0x7FFFBFFF7F00, 32, SOV_XPG4_2)Err#150 EINPROGRESS ^ client socket connects to server accept(21, 0x00000000, 0x00000000, SOV_DEFAULT)= 24 ^ server accepts client connection on listening socket close(23)= 0 ^ client socket closes read(24, 0x046B6010, 65536)Err#131 ECONNRESET ^ test gets so much wanted error while reading on accepted FD close(24)= 0 ^ accepted FD closes ... test completes, passes ... ``` Fixes: #43446
1 parent ee22706 commit 9af6703

File tree

2 files changed

+10
-5
lines changed

2 files changed

+10
-5
lines changed

test/parallel/parallel.status

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ test-crypto-dh-stateless: SKIP
3131
test-crypto-keygen: SKIP
3232

3333
[$system==solaris] # Also applies to SmartOS
34-
# https://github.com/nodejs/node/issues/43446
35-
test-net-connect-reset-until-connected: PASS, FLAKY
3634
# https://github.com/nodejs/node/issues/43457
3735
test-domain-no-error-handler-abort-on-uncaught-0: PASS, FLAKY
3836
test-domain-no-error-handler-abort-on-uncaught-1: PASS,FLAKY
@@ -52,8 +50,6 @@ test-domain-with-abort-on-uncaught-exception: PASS, FLAKY
5250
test-fs-stat-bigint: PASS,FLAKY
5351
# https://github.com/nodejs/node/issues/31280
5452
test-worker-message-port-message-before-close: PASS,FLAKY
55-
# https://github.com/nodejs/node/issues/43446
56-
test-net-connect-reset-until-connected: PASS, FLAKY
5753

5854
[$system==ibmi]
5955
# https://github.com/nodejs/node/pull/30819

test/parallel/test-net-connect-reset-until-connected.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,27 @@
33
const common = require('../common');
44
const net = require('net');
55

6+
function barrier(count, cb) {
7+
return function () {
8+
if (--count == 0)
9+
cb();
10+
}
11+
}
12+
613
const server = net.createServer();
714
server.listen(0, common.mustCall(function() {
815
const port = server.address().port;
916
const conn = net.createConnection(port);
17+
const connok = barrier(2, () => conn.resetAndDestroy());
1018
conn.on('close', common.mustCall());
1119
server.on('connection', (socket) => {
20+
connok();
1221
socket.on('error', common.expectsError({
1322
code: 'ECONNRESET',
1423
message: 'read ECONNRESET',
1524
name: 'Error'
1625
}));
1726
server.close();
1827
});
19-
conn.resetAndDestroy();
28+
conn.on('connect', connok);
2029
}));

0 commit comments

Comments
 (0)