Skip to content

Commit 9aca7c4

Browse files
committed
http2: Abort immediately if passed an already aborted signal
- Also add test to that effect
1 parent 3d5177d commit 9aca7c4

File tree

2 files changed

+44
-5
lines changed

2 files changed

+44
-5
lines changed

lib/internal/http2/core.js

+9-5
Original file line numberDiff line numberDiff line change
@@ -1726,11 +1726,15 @@ class ClientHttp2Session extends Http2Session {
17261726
const { signal } = options;
17271727
if (signal) {
17281728
validateAbortSignal(signal, 'options.signal');
1729-
const listener = () => stream.destroy(new AbortError());
1730-
signal.addEventListener('abort', listener);
1731-
stream.once('close', () => {
1732-
signal.removeEventListener('abort', listener);
1733-
});
1729+
const aborter = () => stream.destroy(new AbortError());
1730+
if (signal.aborted) {
1731+
aborter();
1732+
} else {
1733+
signal.addEventListener('abort', aborter);
1734+
stream.once('close', () => {
1735+
signal.removeEventListener('abort', aborter);
1736+
});
1737+
}
17341738
}
17351739

17361740
const onConnect = FunctionPrototypeBind(requestOnConnect,

test/parallel/test-http2-client-destroy.js

+35
Original file line numberDiff line numberDiff line change
@@ -206,3 +206,38 @@ const Countdown = require('../common/countdown');
206206
assert.strictEqual(req.destroyed, true);
207207
}));
208208
}
209+
// Pass an already destroyed signal to abort immediately.
210+
{
211+
const server = h2.createServer();
212+
const controller = new AbortController();
213+
214+
server.on('stream', common.mustNotCall());
215+
server.listen(0, common.mustCall(() => {
216+
const client = h2.connect(`http://localhost:${server.address().port}`);
217+
client.on('close', common.mustCall());
218+
219+
const { signal } = controller;
220+
controller.abort();
221+
222+
assert.strictEqual(signal[kEvents].get('abort'), undefined);
223+
224+
client.on('error', common.mustCall(() => {
225+
// After underlying stream dies, signal listener detached
226+
assert.strictEqual(signal[kEvents].get('abort'), undefined);
227+
}));
228+
229+
const req = client.request({}, { signal });
230+
// Signal already aborted, so no event listener attached.
231+
assert.strictEqual(signal[kEvents].get('abort'), undefined);
232+
233+
assert.strictEqual(req.aborted, false);
234+
// Destroyed on same tick as request made
235+
assert.strictEqual(req.destroyed, true);
236+
237+
req.on('error', common.mustCall((err) => {
238+
assert.strictEqual(err.code, 'ABORT_ERR');
239+
assert.strictEqual(err.name, 'AbortError');
240+
}));
241+
req.on('close', common.mustCall(() => server.close()));
242+
}));
243+
}

0 commit comments

Comments
 (0)