GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — http: avoid aborting IncomingMessage signal on normal close · nodejs/node@dfc192f

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
  • doc/api
    • http.md
  • lib
    • _http_client.js
    • _http_incoming.js
    • _http_server.js
  • test/parallel
    • test-http-request-signal.js
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎doc/api/http.md‎

Copy file name to clipboardExpand all lines: doc/api/http.md
+10-3Lines changed: 10 additions & 3 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -3031,13 +3031,20 @@ Calls `message.socket.setTimeout(msecs, callback)`.
30313031

30323032
<!-- YAML
30333033
added: v26.1.0
3034+
changes:
3035+
- version: REPLACEME
3036+
pr-url: https://github.com/nodejs/node/pull/64392
3037+
description: The signal is no longer aborted after the message
3038+
completes normally.
30343039
-->
30353040

30363041
* Type: {AbortSignal}
30373042

3038-
An {AbortSignal} that is aborted when the underlying socket closes or the
3039-
request is destroyed. The signal is created lazily on first access — no
3040-
{AbortController} is allocated for requests that never use this property.
3043+
An {AbortSignal} that is aborted when the message is destroyed before
3044+
completion or when its underlying socket closes before request handling or
3045+
response reading completes.
3046+
The signal is created lazily on first access — no {AbortController} is allocated
3047+
for requests that never use this property.
30413048

30423049
This is useful for cancelling downstream asynchronous work such as database
30433050
queries or `fetch` calls when a client disconnects mid-request.
Collapse file

‎lib/_http_client.js‎

Copy file name to clipboardExpand all lines: lib/_http_client.js
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ const {
5050
prepareError,
5151
kSkipPendingData,
5252
} = require('_http_common');
53+
const { kDetachAbortSignal } = require('_http_incoming');
5354
const {
5455
kHighWaterMark,
5556
kUniqueHeaders,
@@ -1019,6 +1020,8 @@ function responseOnEnd() {
10191020
const req = this.req;
10201021
const socket = req.socket;
10211022

1023+
this[kDetachAbortSignal]();
1024+
10221025
if (socket) {
10231026
if (req.timeoutCb) socket.removeListener('timeout', emitRequestTimeout);
10241027
socket.removeListener('timeout', responseOnTimeout);
Collapse file

‎lib/_http_incoming.js‎

Copy file name to clipboardExpand all lines: lib/_http_incoming.js
+54-4Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ const kTrailers = Symbol('kTrailers');
3838
const kTrailersDistinct = Symbol('kTrailersDistinct');
3939
const kTrailersCount = Symbol('kTrailersCount');
4040
const kAbortController = Symbol('kAbortController');
41+
const kAbortSignalSocket = Symbol('kAbortSignalSocket');
42+
const kAbortSignalListener = Symbol('kAbortSignalListener');
43+
const kAbortSignalDetached = Symbol('kAbortSignalDetached');
44+
const kAttachAbortSignal = Symbol('kAttachAbortSignal');
45+
const kDetachAbortSignal = Symbol('kDetachAbortSignal');
4146

4247
function readStart(socket) {
4348
if (socket && !socket._paused && socket.readable)
@@ -94,6 +99,9 @@ function IncomingMessage(socket) {
9499
// read by the user, so there's no point continuing to handle it.
95100
this._dumped = false;
96101
this[kAbortController] = null;
102+
this[kAbortSignalSocket] = null;
103+
this[kAbortSignalListener] = null;
104+
this[kAbortSignalDetached] = false;
97105
}
98106
ObjectSetPrototypeOf(IncomingMessage.prototype, Readable.prototype);
99107
ObjectSetPrototypeOf(IncomingMessage, Readable);
@@ -195,18 +203,51 @@ ObjectDefineProperty(IncomingMessage.prototype, 'signal', {
195203
if (this[kAbortController] === null) {
196204
const ac = new AbortController();
197205
this[kAbortController] = ac;
198-
if (this.destroyed) {
206+
if (this.destroyed && (!this.readableEnded || !this.complete)) {
199207
ac.abort();
200208
} else {
201-
this.once('close', function() {
202-
ac.abort();
203-
});
209+
this[kAttachAbortSignal]();
204210
}
205211
}
206212
return this[kAbortController].signal;
207213
},
208214
});
209215

216+
IncomingMessage.prototype[kAttachAbortSignal] = function() {
217+
if (this[kAbortController].signal.aborted ||
218+
this[kAbortSignalDetached] ||
219+
this[kAbortSignalListener] !== null) {
220+
return;
221+
}
222+
223+
const socket = this.socket;
224+
if (!socket) {
225+
return;
226+
}
227+
228+
if (socket.destroyed) {
229+
abortSignal(this);
230+
return;
231+
}
232+
233+
this[kAbortSignalSocket] = socket;
234+
this[kAbortSignalListener] = () => {
235+
abortSignal(this);
236+
};
237+
socket.once('close', this[kAbortSignalListener]);
238+
};
239+
240+
IncomingMessage.prototype[kDetachAbortSignal] = function() {
241+
const socket = this[kAbortSignalSocket];
242+
const listener = this[kAbortSignalListener];
243+
this[kAbortSignalDetached] = true;
244+
this[kAbortSignalSocket] = null;
245+
this[kAbortSignalListener] = null;
246+
if (socket !== null && listener !== null) {
247+
socket.removeListener('close', listener);
248+
}
249+
};
250+
210251
IncomingMessage.prototype.setTimeout = function setTimeout(msecs, callback) {
211252
if (callback)
212253
this.on('timeout', callback);
@@ -234,6 +275,7 @@ IncomingMessage.prototype._destroy = function _destroy(err, cb) {
234275
if (!this.readableEnded || !this.complete) {
235276
this.aborted = true;
236277
this.emit('aborted');
278+
abortSignal(this);
237279
}
238280

239281
// If aborted and the underlying socket is not already destroyed,
@@ -255,6 +297,13 @@ IncomingMessage.prototype._destroy = function _destroy(err, cb) {
255297
}
256298
};
257299

300+
function abortSignal(self) {
301+
self[kDetachAbortSignal]();
302+
if (self[kAbortController] !== null) {
303+
self[kAbortController].abort();
304+
}
305+
}
306+
258307
IncomingMessage.prototype._addHeaderLines = _addHeaderLines;
259308
function _addHeaderLines(headers, n) {
260309
if (headers?.length) {
@@ -472,6 +521,7 @@ function onError(self, error, cb) {
472521

473522
module.exports = {
474523
IncomingMessage,
524+
kDetachAbortSignal,
475525
readStart,
476526
readStop,
477527
};
Collapse file

‎lib/_http_server.js‎

Copy file name to clipboardExpand all lines: lib/_http_server.js
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,10 @@ const {
7070
defaultTriggerAsyncIdScope,
7171
getOrSetAsyncId,
7272
} = require('internal/async_hooks');
73-
const { IncomingMessage } = require('_http_incoming');
73+
const {
74+
IncomingMessage,
75+
kDetachAbortSignal,
76+
} = require('_http_incoming');
7477
const {
7578
ConnResetException,
7679
codes: {
@@ -1205,6 +1208,7 @@ function resOnFinish(req, res, socket, state, server) {
12051208
// array will be empty.
12061209
assert(state.incoming.length === 0 || state.incoming[0] === req);
12071210

1211+
req[kDetachAbortSignal]();
12081212
state.incoming.shift();
12091213

12101214
// If the user never called req.read(), and didn't pipe() or
Collapse file

‎test/parallel/test-http-request-signal.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-http-request-signal.js
+81-5Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const common = require('../common');
44
const assert = require('assert');
55
const http = require('http');
66

7-
// Test 1: req.signal is an AbortSignal and aborts on 'close'
7+
// Test 1: req.signal is an AbortSignal and aborts on socket close
88
{
99
const server = http.createServer(common.mustCall((req, res) => {
1010
assert.ok(req.signal instanceof AbortSignal);
@@ -21,21 +21,68 @@ const http = require('http');
2121
}));
2222
}
2323

24-
// Test 2: req.signal is aborted if accessed after destroy
24+
// Test 2: req.signal is not aborted when a request body completes normally.
25+
{
26+
const body = JSON.stringify({ hello: 'world' });
27+
const server = http.createServer(common.mustCall((req, res) => {
28+
assert.ok(req.signal instanceof AbortSignal);
29+
assert.strictEqual(req.signal.aborted, false);
30+
req.signal.onabort = common.mustNotCall();
31+
32+
req.on('close', common.mustCall(() => {
33+
assert.strictEqual(req.aborted, false);
34+
assert.strictEqual(req.complete, true);
35+
assert.strictEqual(req.signal.aborted, false);
36+
}));
37+
38+
req.on('end', common.mustCall(() => {
39+
setTimeout(common.mustCall(() => {
40+
assert.strictEqual(req.aborted, false);
41+
assert.strictEqual(req.complete, true);
42+
assert.strictEqual(req.signal.aborted, false);
43+
res.end('ok');
44+
}), 10);
45+
}));
46+
req.resume();
47+
}));
48+
49+
server.listen(0, common.mustCall(() => {
50+
const clientReq = http.request(
51+
{
52+
port: server.address().port,
53+
method: 'PATCH',
54+
path: '/tables/1',
55+
headers: {
56+
'content-type': 'application/json',
57+
'content-length': Buffer.byteLength(body),
58+
},
59+
},
60+
common.mustCall((res) => {
61+
res.resume();
62+
res.on('end', common.mustCall(() => {
63+
server.close();
64+
}));
65+
}),
66+
);
67+
clientReq.end(body);
68+
}));
69+
}
70+
71+
// Test 3: req.signal is aborted if accessed after destroy
2572
{
2673
const req = new http.IncomingMessage(null);
2774
req.destroy();
2875
assert.strictEqual(req.signal.aborted, true);
2976
}
3077

31-
// Test 3: Multiple accesses return the same signal
78+
// Test 4: Multiple accesses return the same signal
3279
{
3380
const req = new http.IncomingMessage(null);
3481
assert.strictEqual(req.signal, req.signal);
3582
}
3683

3784

38-
// Test 4: res.signal on a client-side http.request() response (IncomingMessage).
85+
// Test 5: res.signal on a client-side http.request() response (IncomingMessage).
3986
{
4087
const server = http.createServer(common.mustCall((req, res) => {
4188
res.writeHead(200);
@@ -61,7 +108,36 @@ const http = require('http');
61108
}));
62109
}
63110

64-
// Test 5: Client cancels a pending request.
111+
// Test 6: res.signal is not aborted when a response body completes normally.
112+
{
113+
const server = http.createServer(common.mustCall((req, res) => {
114+
res.end('ok');
115+
}));
116+
117+
server.listen(0, common.mustCall(() => {
118+
const clientReq = http.request(
119+
{ port: server.address().port },
120+
common.mustCall((res) => {
121+
assert.ok(res.signal instanceof AbortSignal);
122+
assert.strictEqual(res.signal.aborted, false);
123+
res.signal.onabort = common.mustNotCall();
124+
125+
res.resume();
126+
res.on('end', common.mustCall(() => {
127+
assert.strictEqual(res.complete, true);
128+
assert.strictEqual(res.signal.aborted, false);
129+
}));
130+
res.on('close', common.mustCall(() => {
131+
assert.strictEqual(res.signal.aborted, false);
132+
server.close();
133+
}));
134+
}),
135+
);
136+
clientReq.end();
137+
}));
138+
}
139+
140+
// Test 7: Client cancels a pending request.
65141
{
66142
const server = http.createServer(common.mustCall((req, res) => {
67143
req.signal.onabort = common.mustCall(() => {

0 commit comments

Comments
0 (0)
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.