Skip to content

Commit 04f8d6b

Browse files
JimblyMylesBorins
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: #23716Fixes: creationix/http-parser-js#57 PR-URL: #24006 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent c37b319 commit 04f8d6b

File tree

2 files changed

+48
-3
lines changed

2 files changed

+48
-3
lines changed

‎lib/internal/child_process.js‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,7 @@ const SocketList = require('internal/socket_list');
3131
const{ owner_symbol }=require('internal/async_hooks').symbols;
3232
const{ convertToValidSignal }=require('internal/util');
3333
const{ isArrayBufferView }=require('internal/util/types');
34-
constspawn_sync=process.binding('spawn_sync');
35-
const{ HTTPParser }=process.binding('http_parser');
36-
const{ freeParser }=require('_http_common');
34+
constspawn_sync=internalBinding('spawn_sync');
3735
const{ kStateSymbol }=require('internal/dgram');
3836

3937
const{
@@ -51,6 +49,10 @@ const{SocketListSend, SocketListReceive } = SocketList;
5149

5250
// Lazy loaded for startup performance.
5351
letStringDecoder;
52+
// Lazy loaded for startup performance and to allow monkey patching of
53+
// internalBinding('http_parser').HTTPParser.
54+
letfreeParser;
55+
letHTTPParser;
5456

5557
constMAX_HANDLE_RETRANSMISSIONS=3;
5658

@@ -115,6 +117,12 @@ const handleConversion ={
115117
handle.onread=nop;
116118
socket._handle=null;
117119
socket.setTimeout(0);
120+
121+
if(freeParser===undefined)
122+
freeParser=require('_http_common').freeParser;
123+
if(HTTPParser===undefined)
124+
HTTPParser=internalBinding('http_parser').HTTPParser;
125+
118126
// In case of an HTTP connection socket, release the associated
119127
// resources
120128
if(socket.parser&&socket.parserinstanceofHTTPParser){
Lines changed: 37 additions & 0 deletions
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+
classDummyParser{
9+
constructor(type){
10+
this.test_type=type;
11+
}
12+
}
13+
DummyParser.REQUEST=Symbol();
14+
internalBinding('http_parser').HTTPParser=DummyParser;
15+
16+
constcommon=require('../common');
17+
constassert=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+
constparser=parsers.alloc();
23+
assert.strictEqual(parserinstanceofDummyParser,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+
constchild=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)