[Squash] address feedback[Squash] address feedback

nodejs · jasnell · Apr 4, 2018 · Apr 16, 2018 · Apr 16, 2018 · Apr 16, 2018
commit 7be9b8d7a94d656830bd5ed876a11319452acdb2
diff --git a/doc/api/tracing.md b/doc/api/tracing.md
@@ -151,7 +151,9 @@ added: REPLACEME
 -->
 
 * `options` {Object}
-  * `categories` {string[]} An array of trace category names
+  * `categories` {string[]} An array of trace category names. Values included
+    in the array are coerced to a string when possible. An error will be
+    thrown if the value cannot be coerced.
 * Returns: {Tracing}.
 
 Creates and returns a `Tracing` object for the given set of `categories`.
@@ -193,13 +195,4 @@ t2.enable();
 console.log(trace_events.getEnabledCategories());
 ```
 
-### `trace_events.getEnabledTracingObjects()`
-<!-- YAML
-added: REPLACEME
--->
-
-* Returns: {Tracing[]}
-
-Returns an array of currently enabled `Tracing` objects.
-
 [Performance API]: perf_hooks.html
diff --git a/lib/trace_events.js b/lib/trace_events.js
@@ -5,6 +5,8 @@ const kHandle = Symbol('handle');
 const kEnabled = Symbol('enabled');
 const kCategories = Symbol('categories');
 
+const kMaxTracingCount = 10;
+
 const {
   ERR_TRACE_EVENTS_CATEGORY_REQUIRED,
   ERR_TRACE_EVENTS_UNAVAILABLE,
@@ -32,6 +34,12 @@ class Tracing {
       this[kEnabled] = true;
       this[kHandle].enable();
       enabledTracingObjects.add(this);
+      if (enabledTracingObjects.size > kMaxTracingCount) {
+        process.emitWarning(
+          'Possible trace_events memory leak detected. There are more than ' +
+          `${kMaxTracingCount} enabled Tracing objects.`
+        );
+      }
     }
   }
 
@@ -75,12 +83,7 @@ function createTracing(options) {
   return new Tracing(options.categories);
 }
 
-function getEnabledTracingObjects() {
-  return Array.from(enabledTracingObjects);
-}
-
 module.exports = {
   createTracing,
-  getEnabledCategories,
-  getEnabledTracingObjects
+  getEnabledCategories
 };
diff --git a/src/node_trace_events.cc b/src/node_trace_events.cc
@@ -41,7 +41,7 @@ class NodeCategorySet : public BaseObject {
   }
 
   bool enabled_ = false;
-  std::set<std::string> categories_;
+  const std::set<std::string> categories_;
 };
 
 void NodeCategorySet::New(const FunctionCallbackInfo<Value>& args) {

diff --git a/test/parallel/test-trace-events-api.js b/test/parallel/test-trace-events-api.js
@@ -13,8 +13,7 @@ const fs = require('fs');
 const tmpdir = require('../common/tmpdir');
 const {
   createTracing,
-  getEnabledCategories,
-  getEnabledTracingObjects
+  getEnabledCategories
 } = require('trace_events');
 
 const isChild = process.argv[2] === 'child';
@@ -91,16 +90,20 @@ if (isChild) {
       tracing3 = undefined;
     }
     global.gc();
-    assert.strictEqual(getEnabledTracingObjects().length, 1);
     assert.strictEqual(getEnabledCategories(), 'abc');
-    {
-      let tracing3 = getEnabledTracingObjects()[0];
-      assert(tracing3);
-      tracing3.disable();
-      assert.strictEqual(getEnabledCategories(), undefined);
-      tracing3 = undefined;
+    // Not able to disable the thing after this point, however.
+  }
+
+  {
+    common.expectWarning(
+      'Warning',
+      'Possible trace_events memory leak detected. There are more than ' +
+      '10 enabled Tracing objects.',
+      common.noWarnCode);
+    for (let n = 0; n < 10; n++) {
+      const tracing = createTracing({ categories: [ `a${n}` ] });
+      tracing.enable();
     }
-    global.gc();
   }
 
   tmpdir.refresh();