diff options
author | Jimb Esser <wasteland@gmail.com> | 2018-10-31 12:55:00 -0700 |
---|---|---|
committer | Rich Trott <rtrott@gmail.com> | 2018-11-08 14:38:23 -0800 |
commit | 7d86a5324bc29c21afce8cba3dba508b78d48cd5 (patch) | |
tree | e52982270b7712f847eef6c88fa05d865d3b70a9 | |
parent | 1f6c4ba61d70318d1422a0dac2297f519ee35045 (diff) | |
download | node-new-7d86a5324bc29c21afce8cba3dba508b78d48cd5.tar.gz |
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: https://github.com/nodejs/node/issues/23716
Fixes: https://github.com/creationix/http-parser-js/issues/57
PR-URL: https://github.com/nodejs/node/pull/24006
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
-rw-r--r-- | lib/internal/child_process.js | 12 | ||||
-rw-r--r-- | test/parallel/test-http-parser-lazy-loaded.js | 37 |
2 files changed, 47 insertions, 2 deletions
diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index ddb7e58d7c..2f19f28b59 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -38,8 +38,6 @@ const { owner_symbol } = require('internal/async_hooks').symbols; const { convertToValidSignal } = require('internal/util'); const { isArrayBufferView } = require('internal/util/types'); const spawn_sync = internalBinding('spawn_sync'); -const { HTTPParser } = internalBinding('http_parser'); -const { freeParser } = require('_http_common'); const { kStateSymbol } = require('internal/dgram'); const { @@ -57,6 +55,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; @@ -121,6 +123,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) { diff --git a/test/parallel/test-http-parser-lazy-loaded.js b/test/parallel/test-http-parser-lazy-loaded.js new file mode 100644 index 0000000000..c1eb29fb16 --- /dev/null +++ b/test/parallel/test-http-parser-lazy-loaded.js @@ -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); + })); +} |