Skip to content

Commit

Permalink
child_process: allow 'http_parser' monkey patching again
Browse files Browse the repository at this point in the history
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: #23716
Fixes: creationix/http-parser-js#57

PR-URL: #24006
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
Jimbly authored and MylesBorins committed Dec 26, 2018
1 parent c37b319 commit 04f8d6b
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 3 deletions.
14 changes: 11 additions & 3 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ const SocketList = require('internal/socket_list');
const { owner_symbol } = require('internal/async_hooks').symbols;
const { convertToValidSignal } = require('internal/util');
const { isArrayBufferView } = require('internal/util/types');
const spawn_sync = process.binding('spawn_sync');
const { HTTPParser } = process.binding('http_parser');
const { freeParser } = require('_http_common');
const spawn_sync = internalBinding('spawn_sync');
const { kStateSymbol } = require('internal/dgram');

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

// Lazy loaded for startup performance.
let StringDecoder;
// Lazy loaded for startup performance and to allow monkey patching of
// internalBinding('http_parser').HTTPParser.
let freeParser;
let HTTPParser;

const MAX_HANDLE_RETRANSMISSIONS = 3;

Expand Down Expand Up @@ -115,6 +117,12 @@ const handleConversion = {
handle.onread = nop;
socket._handle = null;
socket.setTimeout(0);

if (freeParser === undefined)
freeParser = require('_http_common').freeParser;
if (HTTPParser === undefined)
HTTPParser = internalBinding('http_parser').HTTPParser;

// In case of an HTTP connection socket, release the associated
// resources
if (socket.parser && socket.parser instanceof HTTPParser) {
Expand Down
37 changes: 37 additions & 0 deletions test/parallel/test-http-parser-lazy-loaded.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Flags: --expose-internals

'use strict';

const { internalBinding } = require('internal/test/binding');

// Monkey patch before requiring anything
class DummyParser {
constructor(type) {
this.test_type = type;
}
}
DummyParser.REQUEST = Symbol();
internalBinding('http_parser').HTTPParser = DummyParser;

const common = require('../common');
const assert = require('assert');
const { spawn } = require('child_process');
const { parsers } = require('_http_common');

// Test _http_common was not loaded before monkey patching
const parser = parsers.alloc();
assert.strictEqual(parser instanceof DummyParser, true);
assert.strictEqual(parser.test_type, DummyParser.REQUEST);

if (process.argv[2] !== 'child') {
// Also test in a child process with IPC (specific case of https://github.com/nodejs/node/issues/23716)
const child = spawn(process.execPath, [
'--expose-internals', __filename, 'child'
], {
stdio: ['inherit', 'inherit', 'inherit', 'ipc']
});
child.on('exit', common.mustCall((code, signal) => {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
}));
}

0 comments on commit 04f8d6b

Please sign in to comment.