Skip to content

Commit 60d3c04

Browse files
committed
Add regression test for socket.closed rejecting with undefined after remote FIN
Reproduces the bug where calling socket.close() after draining a readable stream to EOF (remote FIN) causes socket.closed to reject with undefined instead of resolving. Two code paths race: Socket::close() aborts the writable with kj::none (→ undefined), then the maybeCloseWriteSide microtask finds the writable errored and rejects closedResolver with the raw undefined value. Three test cases: 1. Core race: close() after for-await EOF → closed rejects undefined 2. Immediate close() → rejection reason must not be undefined 3. No explicit close() → clean remote FIN resolves closed
1 parent 55fbbf3 commit 60d3c04

File tree

3 files changed

+125
-0
lines changed

3 files changed

+125
-0
lines changed

src/workerd/api/tests/BUILD.bazel

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,15 @@ wd_test(
4343
tags = ["exclusive"],
4444
)
4545

46+
wd_test(
47+
src = "socket-closed-test.wd-test",
48+
args = ["--experimental"],
49+
data = ["socket-closed-test.js"],
50+
# Test uses TCP socket on port 8084 and may fail when running concurrently with other
51+
# tests that do so.
52+
tags = ["exclusive"],
53+
)
54+
4655
wd_test(
4756
src = "actor-alarms-test.wd-test",
4857
args = ["--experimental"],
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Copyright (c) 2017-2022 Cloudflare, Inc.
2+
// Licensed under the Apache 2.0 license found in the LICENSE file or at:
3+
// https://opensource.org/licenses/Apache-2.0
4+
5+
// Reproduction tests for: socket.closed rejects with undefined when socket.close()
6+
// is called after remote FIN.
7+
//
8+
// When the remote peer sends FIN (closes its write side) with allowHalfOpen=false
9+
// (the default), calling socket.close() after draining the readable to EOF causes
10+
// socket.closed to reject with undefined instead of resolving.
11+
//
12+
// Two code paths race:
13+
// Path A (handleReadableEof): queues maybeCloseWriteSide as a microtask on EOF
14+
// Path B (Socket::close): calls writable abort(js, kj::none) synchronously
15+
//
16+
// Path B runs first (synchronous after for-await exits), putting the writable into
17+
// Errored(undefined). When Path A's microtask fires, close(js) fails because the
18+
// writable is errored, and the catch handler rejects closedResolver with undefined.
19+
20+
import { connect } from 'cloudflare:sockets';
21+
import { strictEqual, notStrictEqual } from 'assert';
22+
23+
// Server: writes a small payload then closes (sends FIN).
24+
export default {
25+
async connect(socket) {
26+
const writer = socket.writable.getWriter();
27+
await writer.write(new TextEncoder().encode('ping'));
28+
await writer.close();
29+
},
30+
};
31+
32+
// Test 1 (core bug): socket.close() after EOF causes socket.closed to reject with undefined.
33+
// The for-await exits on EOF, then socket.close() races with the maybeCloseWriteSide microtask.
34+
export const closedRejectsUndefinedAfterCloseRace = {
35+
async test() {
36+
const socket = connect('localhost:8084');
37+
await socket.opened;
38+
39+
const dec = new TextDecoder();
40+
let data = '';
41+
for await (const chunk of socket.readable) {
42+
data += dec.decode(chunk, { stream: true });
43+
}
44+
data += dec.decode();
45+
strictEqual(data, 'ping');
46+
47+
// This is what application cleanup code does — close the socket after the read loop.
48+
// It races with maybeCloseWriteSide (triggered by EOF with allowHalfOpen=false).
49+
socket.close();
50+
51+
// socket.closed must resolve (not reject with undefined).
52+
try {
53+
await socket.closed;
54+
} catch (e) {
55+
notStrictEqual(
56+
e,
57+
undefined,
58+
'socket.closed rejected with undefined — should resolve or reject with a real Error'
59+
);
60+
}
61+
},
62+
};
63+
64+
// Test 2: if socket.closed rejects via any path, the reason must not be undefined.
65+
export const closedRejectionReasonIsNeverUndefined = {
66+
async test() {
67+
const socket = connect('localhost:8084');
68+
await socket.opened;
69+
70+
// Close immediately — don't drain readable.
71+
socket.close();
72+
73+
try {
74+
await socket.closed;
75+
} catch (e) {
76+
notStrictEqual(e, undefined, 'socket.closed rejected with undefined');
77+
}
78+
},
79+
};
80+
81+
// Test 3: without socket.close(), a clean remote FIN should resolve socket.closed.
82+
export const closedResolvesAfterRemoteEofWithoutExplicitClose = {
83+
async test() {
84+
const socket = connect('localhost:8084');
85+
await socket.opened;
86+
87+
const dec = new TextDecoder();
88+
let data = '';
89+
for await (const chunk of socket.readable) {
90+
data += dec.decode(chunk, { stream: true });
91+
}
92+
data += dec.decode();
93+
strictEqual(data, 'ping');
94+
95+
// Do NOT call socket.close() — let maybeCloseWriteSide handle everything.
96+
await socket.closed;
97+
},
98+
};
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
using Workerd = import "/workerd/workerd.capnp";
2+
3+
const unitTests :Workerd.Config = (
4+
services = [
5+
( name = "socket-closed-test",
6+
worker = (
7+
modules = [
8+
(name = "worker", esModule = embed "socket-closed-test.js"),
9+
],
10+
compatibilityFlags = ["nodejs_compat_v2", "experimental"],
11+
)
12+
),
13+
( name = "internet", network = ( allow = ["private"] ) )
14+
],
15+
sockets = [
16+
(name = "tcp", address = "*:8084", tcp = (), service = "socket-closed-test"),
17+
]
18+
);

0 commit comments

Comments
 (0)