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: #23716 Fixes: 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]>
nodejs · Dec 26, 2018 · 04f8d6b · 04f8d6b
1 parent c37b319
commit 04f8d6b
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 3 deletions.
diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js
@@ -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 {
@@ -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;
 
@@ -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) {

diff --git a/test/parallel/test-http-parser-lazy-loaded.js 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);
+  }));
+}