http: use symbol for FreeList's is_reused

basti1302 · basti1302 · commit 26d147113f22 · 2018-10-04T22:45:58.000+02:00
This also at least keeps up the appearance of FreeList being a generic
all purpose thing (right now it is only used for HTTP parsers but
theoretically it could be used for any reusable resource). The
previous name "needsAsyncReset" implied a tight coupling to to async
resources, the new name "is_reused" is more generic.
diff --git a/benchmark/misc/freelist.js b/benchmark/misc/freelist.js
@@ -9,7 +9,7 @@ const bench = common.createBenchmark(main, {
 });
 
 function main({ n }) {
-  const FreeList = require('internal/freelist');
+  const { FreeList } = require('internal/freelist');
   const poolSize = 1000;
   const list = new FreeList('test', poolSize, Object);
   var j;
diff --git a/lib/_http_client.js b/lib/_http_client.js
@@ -51,6 +51,7 @@ 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]/;
 
@@ -635,10 +636,10 @@ function tickOnSocket(req, socket) {
   var parser = parsers.alloc();
   req.socket = socket;
   req.connection = socket;
-  if (destroyHooksExist() && parser.needsAsyncReset && parser.getAsyncId()) {
+  if (destroyHooksExist() && parser[is_reused_symbol] && parser.getAsyncId()) {
     emitDestroy(parser.getAsyncId());
   }
-  parser.reinitialize(HTTPParser.RESPONSE, parser.needsAsyncReset);
+  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
@@ -23,7 +23,7 @@
 
 const { methods, HTTPParser } = internalBinding('http_parser');
 
-const FreeList = require('internal/freelist');
+const { FreeList } = require('internal/freelist');
 const { ondrain } = require('internal/http');
 const incoming = require('_http_incoming');
 const {
diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -44,6 +44,7 @@ const {
   emitDestroy,
   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,
@@ -340,10 +341,10 @@ function connectionListenerInternal(server, socket) {
   socket.on('timeout', socketOnTimeout);
 
   var parser = parsers.alloc();
-  if (destroyHooksExist() && parser.needsAsyncReset && parser.getAsyncId()) {
+  if (destroyHooksExist() && parser[is_reused_symbol] && parser.getAsyncId()) {
     emitDestroy(parser.getAsyncId());
   }
-  parser.reinitialize(HTTPParser.REQUEST, parser.needsAsyncReset);
+  parser.reinitialize(HTTPParser.REQUEST, parser[is_reused_symbol]);
   parser.socket = socket;
   socket.parser = parser;
 
diff --git a/lib/internal/freelist.js b/lib/internal/freelist.js
@@ -1,5 +1,7 @@
 'use strict';
 
+const is_reused_symbol = Symbol('isReused');
+
 class FreeList {
   constructor(name, max, ctor) {
     this.name = name;
@@ -10,8 +12,8 @@ class FreeList {
 
   alloc() {
     return this.list.length ?
-      needsToCallAsyncReset(this.list.pop()) :
-      mustNotCallAsyncReset(this.ctor.apply(this, arguments));
+      setIsReused(this.list.pop(), true) :
+      setIsReused(this.ctor.apply(this, arguments), false);
   }
 
   free(obj) {
@@ -23,14 +25,14 @@ class FreeList {
   }
 }
 
-function needsToCallAsyncReset(item) {
-  item.needsAsyncReset = true;
+function setIsReused(item, reused) {
+  item[is_reused_symbol] = reused;
   return item;
 }
 
-function mustNotCallAsyncReset(item) {
-  item.needsAsyncReset = false;
-  return item;
-}
-
-module.exports = FreeList;
+module.exports = {
+  FreeList,
+  symbols: {
+    is_reused_symbol
+  }
+};
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
@@ -466,7 +466,7 @@ class Parser : public AsyncWrap, public StreamListener {
 
     CHECK(args[0]->IsInt32());
     CHECK(args[1]->IsBoolean());
-    bool needsAsyncReset = args[1]->IsTrue();
+    bool isReused = args[1]->IsTrue();
     http_parser_type type =
         static_cast<http_parser_type>(args[0].As<Int32>()->Value());
 
@@ -478,7 +478,7 @@ class Parser : public AsyncWrap, public StreamListener {
     // 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 (needsAsyncReset) {
+    if (isReused) {
       parser->AsyncReset();
     }
     parser->Init(type);
diff --git a/test/parallel/test-freelist.js b/test/parallel/test-freelist.js
@@ -4,7 +4,7 @@
 
 require('../common');
 const assert = require('assert');
-const FreeList = require('internal/freelist');
+const { FreeList } = require('internal/freelist');
 
 assert.strictEqual(typeof FreeList, 'function');
 
diff --git a/test/parallel/test-internal-modules-expose.js b/test/parallel/test-internal-modules-expose.js
@@ -7,5 +7,5 @@ const config = process.binding('config');
 
 console.log(config, process.argv);
 
-assert.strictEqual(typeof require('internal/freelist'), 'function');
+assert.strictEqual(typeof require('internal/freelist').FreeList, 'function');
 assert.strictEqual(config.exposeInternals, true);
diff --git a/test/sequential/test-http-regr-gh-2928.js b/test/sequential/test-http-regr-gh-2928.js
@@ -7,6 +7,7 @@ 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');
 
@@ -25,7 +26,7 @@ function execAndClose() {
   process.stdout.write('.');
 
   const parser = parsers.pop();
-  parser.reinitialize(HTTPParser.RESPONSE, parser.needsAsyncReset);
+  parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]);
 
   const socket = net.connect(common.PORT);
   socket.on('error', (e) => {
