diff --git a/benchmark/http/bench-parser.js b/benchmark/http/bench-parser.js index a54f0efa75e3c1..1a1d0c749e63e1 100644 --- a/benchmark/http/bench-parser.js +++ b/benchmark/http/bench-parser.js @@ -19,12 +19,10 @@ function main({ len, n }) { const CRLF = '\r\n'; function processHeader(header, n) { - const parser = newParser(REQUEST); - bench.start(); for (var i = 0; i < n; i++) { + const parser = newParser(REQUEST); parser.execute(header, 0, header.length); - parser.reinitialize(REQUEST, i > 0); } bench.end(n); } diff --git a/lib/_http_client.js b/lib/_http_client.js index d91b43516fa4ee..52421a5f5a5990 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -29,9 +29,9 @@ const assert = require('assert').ok; const { _checkIsHttpToken: checkIsHttpToken, debug, + createParser, freeParser, - httpSocketSetup, - parsers + httpSocketSetup } = require('_http_common'); const { OutgoingMessage } = require('_http_outgoing'); const Agent = require('_http_agent'); @@ -47,7 +47,6 @@ const { ERR_UNESCAPED_CHARACTERS } = require('internal/errors').codes; const { validateTimerDuration } = require('internal/timers'); -const is_reused_symbol = require('internal/freelist').symbols.is_reused_symbol; const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/; @@ -629,10 +628,9 @@ function emitFreeNT(socket) { } function tickOnSocket(req, socket) { - var parser = parsers.alloc(); + const parser = createParser(HTTPParser.RESPONSE); req.socket = socket; req.connection = socket; - parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]); parser.socket = socket; parser.outgoing = req; req.parser = parser; diff --git a/lib/_http_common.js b/lib/_http_common.js index 8b68d0c81500a8..5ec5fb53d1b5a3 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -23,7 +23,6 @@ const { methods, HTTPParser } = internalBinding('http_parser'); -const { FreeList } = require('internal/freelist'); const { ondrain } = require('internal/http'); const incoming = require('_http_incoming'); const { @@ -147,9 +146,8 @@ function parserOnMessageComplete() { readStart(parser.socket); } - -const parsers = new FreeList('parsers', 1000, function parsersCb() { - const parser = new HTTPParser(HTTPParser.REQUEST); +function createParser(type) { + const parser = new HTTPParser(type); cleanParser(parser); @@ -160,7 +158,7 @@ const parsers = new FreeList('parsers', 1000, function parsersCb() { parser[kOnMessageComplete] = parserOnMessageComplete; return parser; -}); +} function closeParserInstance(parser) { parser.close(); } @@ -168,23 +166,13 @@ function closeParserInstance(parser) { parser.close(); } // might have to any other things. // TODO: All parser data should be attached to a // single object, so that it can be easily cleaned -// up by doing `parser.data = {}`, which should -// be done in FreeList.free. `parsers.free(parser)` -// should be all that is needed. +// up by doing `parser.data = {}`. function freeParser(parser, req, socket) { if (parser) { if (parser._consumed) parser.unconsume(); cleanParser(parser); - if (parsers.free(parser) === false) { - // Make sure the parser's stack has unwound before deleting the - // corresponding C++ object through .close(). - setImmediate(closeParserInstance, parser); - } else { - // Since the Parser destructor isn't going to run the destroy() callbacks - // it needs to be triggered manually. - parser.free(); - } + setImmediate(closeParserInstance, parser); } if (req) { req.parser = null; @@ -239,9 +227,9 @@ module.exports = { continueExpression: /(?:^|\W)100-continue(?:$|\W)/i, CRLF: '\r\n', debug, + createParser, freeParser, httpSocketSetup, methods, - parsers, kIncomingMessage }; diff --git a/lib/_http_server.js b/lib/_http_server.js index 3b2d7f50419127..7ae1fd1a52918f 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -26,7 +26,7 @@ const net = require('net'); const { HTTPParser } = internalBinding('http_parser'); const assert = require('assert').ok; const { - parsers, + createParser, freeParser, debug, CRLF, @@ -42,7 +42,6 @@ const { defaultTriggerAsyncIdScope, getOrSetAsyncId } = require('internal/async_hooks'); -const is_reused_symbol = require('internal/freelist').symbols.is_reused_symbol; const { IncomingMessage } = require('_http_incoming'); const { ERR_HTTP_HEADERS_SENT, @@ -338,8 +337,7 @@ function connectionListenerInternal(server, socket) { socket.setTimeout(server.timeout); socket.on('timeout', socketOnTimeout); - var parser = parsers.alloc(); - parser.reinitialize(HTTPParser.REQUEST, parser[is_reused_symbol]); + const parser = createParser(HTTPParser.REQUEST); parser.socket = socket; socket.parser = parser; diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index c9e057711d9253..f973bc18be3a7e 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -494,30 +494,6 @@ class Parser : public AsyncWrap, public StreamListener { } - static void Reinitialize(const FunctionCallbackInfo<Value>& args) { - Environment* env = Environment::GetCurrent(args); - - CHECK(args[0]->IsInt32()); - CHECK(args[1]->IsBoolean()); - bool isReused = args[1]->IsTrue(); - parser_type_t type = - static_cast<parser_type_t>(args[0].As<Int32>()->Value()); - - CHECK(type == HTTP_REQUEST || type == HTTP_RESPONSE); - Parser* parser; - ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder()); - // Should always be called from the same context. - CHECK_EQ(env, parser->env()); - // This parser has either just been created or it is being reused. - // We must only call AsyncReset for the latter case, because AsyncReset has - // already been called via the constructor for the former case. - if (isReused) { - parser->AsyncReset(); - } - parser->Init(type); - } - - template <bool should_pause> static void Pause(const FunctionCallbackInfo<Value>& args) { Environment* env = Environment::GetCurrent(args); @@ -923,7 +899,6 @@ void Initialize(Local<Object> target, env->SetProtoMethod(t, "free", Parser::Free); env->SetProtoMethod(t, "execute", Parser::Execute); env->SetProtoMethod(t, "finish", Parser::Finish); - env->SetProtoMethod(t, "reinitialize", Parser::Reinitialize); env->SetProtoMethod(t, "pause", Parser::Pause<true>); env->SetProtoMethod(t, "resume", Parser::Pause<false>); env->SetProtoMethod(t, "consume", Parser::Consume); diff --git a/test/parallel/test-http-parser-free.js b/test/parallel/test-http-parser-free.js index 935d4d2ef443b0..b0e5da43ef92dd 100644 --- a/test/parallel/test-http-parser-free.js +++ b/test/parallel/test-http-parser-free.js @@ -41,7 +41,7 @@ server.listen(0, function() { if (!parser) { parser = req.parser; } else { - assert.strictEqual(req.parser, parser); + assert.notStrictEqual(req.parser, parser); } countdown.dec(); diff --git a/test/parallel/test-http-parser-lazy-loaded.js b/test/parallel/test-http-parser-lazy-loaded.js index c1eb29fb163d00..b121fb85909fa3 100644 --- a/test/parallel/test-http-parser-lazy-loaded.js +++ b/test/parallel/test-http-parser-lazy-loaded.js @@ -16,10 +16,10 @@ internalBinding('http_parser').HTTPParser = DummyParser; const common = require('../common'); const assert = require('assert'); const { spawn } = require('child_process'); -const { parsers } = require('_http_common'); +const { createParser } = require('_http_common'); // Test _http_common was not loaded before monkey patching -const parser = parsers.alloc(); +const parser = createParser(DummyParser.REQUEST); assert.strictEqual(parser instanceof DummyParser, true); assert.strictEqual(parser.test_type, DummyParser.REQUEST); diff --git a/test/parallel/test-http-parser.js b/test/parallel/test-http-parser.js index 36f41f79e59dcf..2aad798bd1ad17 100644 --- a/test/parallel/test-http-parser.js +++ b/test/parallel/test-http-parser.js @@ -98,8 +98,6 @@ function expectBody(expected) { throw new Error('hello world'); }; - parser.reinitialize(HTTPParser.REQUEST, false); - assert.throws( () => { parser.execute(request, 0, request.length); }, { name: 'Error', message: 'hello world' } @@ -558,10 +556,10 @@ function expectBody(expected) { parser[kOnBody] = expectBody('ping'); parser.execute(req1, 0, req1.length); - parser.reinitialize(REQUEST, false); - parser[kOnBody] = expectBody('pong'); - parser[kOnHeadersComplete] = onHeadersComplete2; - parser.execute(req2, 0, req2.length); + const parser2 = newParser(REQUEST); + parser2[kOnBody] = expectBody('pong'); + parser2[kOnHeadersComplete] = onHeadersComplete2; + parser2.execute(req2, 0, req2.length); } // Test parser 'this' safety diff --git a/test/sequential/test-http-regr-gh-2928.js b/test/sequential/test-http-regr-gh-2928.js index 3794eddaa09369..3e7bafa5d990f5 100644 --- a/test/sequential/test-http-regr-gh-2928.js +++ b/test/sequential/test-http-regr-gh-2928.js @@ -7,15 +7,14 @@ const common = require('../common'); const assert = require('assert'); const httpCommon = require('_http_common'); const { internalBinding } = require('internal/test/binding'); -const is_reused_symbol = require('internal/freelist').symbols.is_reused_symbol; const { HTTPParser } = internalBinding('http_parser'); const net = require('net'); -const COUNT = httpCommon.parsers.max + 1; +const COUNT = 1; const parsers = new Array(COUNT); for (let i = 0; i < parsers.length; i++) - parsers[i] = httpCommon.parsers.alloc(); + parsers[i] = httpCommon.createParser(HTTPParser.RESPONSE); let gotRequests = 0; let gotResponses = 0; @@ -26,7 +25,6 @@ function execAndClose() { process.stdout.write('.'); const parser = parsers.pop(); - parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]); const socket = net.connect(common.PORT); socket.on('error', (e) => {