Skip to content

Commit 7d86a53

Browse files
JimblyTrott
authored andcommitted
child_process: allow 'http_parser' monkey patching again
Lazy load _http_common and HTTPParser so that the 'http_parser' binding can be monkey patched before any internal modules require it. This also probably improves startup performance minimally for programs that never require the HTTP stack. Fixes: nodejs#23716 Fixes: creationix/http-parser-js#57 PR-URL: nodejs#24006 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent 1f6c4ba commit 7d86a53

File tree

2 files changed

+47
-2
lines changed

2 files changed

+47
-2
lines changed

lib/internal/child_process.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ const { owner_symbol } = require('internal/async_hooks').symbols;
3838
const { convertToValidSignal } = require('internal/util');
3939
const { isArrayBufferView } = require('internal/util/types');
4040
const spawn_sync = internalBinding('spawn_sync');
41-
const { HTTPParser } = internalBinding('http_parser');
42-
const { freeParser } = require('_http_common');
4341
const { kStateSymbol } = require('internal/dgram');
4442

4543
const {
@@ -57,6 +55,10 @@ const { SocketListSend, SocketListReceive } = SocketList;
5755

5856
// Lazy loaded for startup performance.
5957
let StringDecoder;
58+
// Lazy loaded for startup performance and to allow monkey patching of
59+
// internalBinding('http_parser').HTTPParser.
60+
let freeParser;
61+
let HTTPParser;
6062

6163
const MAX_HANDLE_RETRANSMISSIONS = 3;
6264

@@ -121,6 +123,12 @@ const handleConversion = {
121123
handle.onread = nop;
122124
socket._handle = null;
123125
socket.setTimeout(0);
126+
127+
if (freeParser === undefined)
128+
freeParser = require('_http_common').freeParser;
129+
if (HTTPParser === undefined)
130+
HTTPParser = internalBinding('http_parser').HTTPParser;
131+
124132
// In case of an HTTP connection socket, release the associated
125133
// resources
126134
if (socket.parser && socket.parser instanceof HTTPParser) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Flags: --expose-internals
2+
3+
'use strict';
4+
5+
const { internalBinding } = require('internal/test/binding');
6+
7+
// Monkey patch before requiring anything
8+
class DummyParser {
9+
constructor(type) {
10+
this.test_type = type;
11+
}
12+
}
13+
DummyParser.REQUEST = Symbol();
14+
internalBinding('http_parser').HTTPParser = DummyParser;
15+
16+
const common = require('../common');
17+
const assert = require('assert');
18+
const { spawn } = require('child_process');
19+
const { parsers } = require('_http_common');
20+
21+
// Test _http_common was not loaded before monkey patching
22+
const parser = parsers.alloc();
23+
assert.strictEqual(parser instanceof DummyParser, true);
24+
assert.strictEqual(parser.test_type, DummyParser.REQUEST);
25+
26+
if (process.argv[2] !== 'child') {
27+
// Also test in a child process with IPC (specific case of https://github.com/nodejs/node/issues/23716)
28+
const child = spawn(process.execPath, [
29+
'--expose-internals', __filename, 'child'
30+
], {
31+
stdio: ['inherit', 'inherit', 'inherit', 'ipc']
32+
});
33+
child.on('exit', common.mustCall((code, signal) => {
34+
assert.strictEqual(code, 0);
35+
assert.strictEqual(signal, null);
36+
}));
37+
}

0 commit comments

Comments
 (0)