From 362d8320ab8a0ddfd0a646d5c9f9ec89197961c7 Mon Sep 17 00:00:00 2001 From: Dave Date: Wed, 30 Dec 2015 00:20:33 -0800 Subject: [PATCH 1/6] events: make sure console functions exist if there's no global console cached, these calls will initialize it, which in some cases can cause a circular dependency, making the console received here an empty object. The program shouldn't crash in this case. Fixes https://github.com/nodejs/node/issues/4467 --- lib/events.js | 3 ++- lib/internal/util.js | 3 ++- test/parallel/test-global-console-exists.js | 21 +++++++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-global-console-exists.js diff --git a/lib/events.js b/lib/events.js index d4c6f167ee6fb9..bcd3ac2bf5f812 100644 --- a/lib/events.js +++ b/lib/events.js @@ -240,7 +240,8 @@ EventEmitter.prototype.addListener = function addListener(type, listener) { 'leak detected. %d %s listeners added. ' + 'Use emitter.setMaxListeners() to increase limit.', existing.length, type); - console.trace(); + if (console.trace) + console.trace(); } } } diff --git a/lib/internal/util.js b/lib/internal/util.js index 21aafff21824b3..8828d5d1286c9e 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -25,7 +25,8 @@ exports.error = function(msg) { args[0] = fmt; for (let i = 1; i < arguments.length; ++i) args[i] = arguments[i]; - console.error.apply(console, args); + if (console.error) + console.error.apply(console, args); } else { console.error(fmt); } diff --git a/test/parallel/test-global-console-exists.js b/test/parallel/test-global-console-exists.js new file mode 100644 index 00000000000000..0c393a40d99941 --- /dev/null +++ b/test/parallel/test-global-console-exists.js @@ -0,0 +1,21 @@ +'use strict'; + +const assert = require('assert'); +const events = require('events'); + +const old_default = events.defaultMaxListeners; +events.defaultMaxListeners = 1; + +const e = new events.EventEmitter(); +e.on('hello', function() {}); + +assert.ok(!e._events['hello'].hasOwnProperty('warned')); + +e.on('hello', function() {}); + +assert.ok(e._events['hello'].hasOwnProperty('warned')); + +events.defaultMaxListeners = old_default; + +// this caches console, so place it after the test just to appease the linter +require('../common'); From 9f8eddb8b98ff9c393ce255e192e3f0c4dfdd73d Mon Sep 17 00:00:00 2001 From: Dave Date: Fri, 1 Jan 2016 12:00:58 -0800 Subject: [PATCH 2/6] initialize console in defaultMaxListeners setter --- lib/events.js | 18 +++++++++++++++--- lib/internal/util.js | 3 +-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/events.js b/lib/events.js index bcd3ac2bf5f812..f57882ab6dc427 100644 --- a/lib/events.js +++ b/lib/events.js @@ -19,7 +19,20 @@ EventEmitter.prototype._maxListeners = undefined; // By default EventEmitters will print a warning if more than 10 listeners are // added to it. This is a useful default which helps finding memory leaks. -EventEmitter.defaultMaxListeners = 10; +var defaultMaxListeners = 10; + +Object.defineProperty(EventEmitter, 'defaultMaxListeners', { + enumerable: true, + get: function() { + return defaultMaxListeners; + }, + set: function(arg) { + // force global console to be compiled. + // see https://github.com/nodejs/node/issues/4467 + console; + defaultMaxListeners = arg; + } +}); EventEmitter.init = function() { this.domain = null; @@ -240,8 +253,7 @@ EventEmitter.prototype.addListener = function addListener(type, listener) { 'leak detected. %d %s listeners added. ' + 'Use emitter.setMaxListeners() to increase limit.', existing.length, type); - if (console.trace) - console.trace(); + console.trace(); } } } diff --git a/lib/internal/util.js b/lib/internal/util.js index 8828d5d1286c9e..21aafff21824b3 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -25,8 +25,7 @@ exports.error = function(msg) { args[0] = fmt; for (let i = 1; i < arguments.length; ++i) args[i] = arguments[i]; - if (console.error) - console.error.apply(console, args); + console.error.apply(console, args); } else { console.error(fmt); } From 04a927643dc772ccba4f1a606330277f2b8b2582 Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 4 Jan 2016 10:55:43 -0800 Subject: [PATCH 3/6] test: overwrite stderr.write instead of checking e._events --- test/parallel/test-global-console-exists.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-global-console-exists.js b/test/parallel/test-global-console-exists.js index 0c393a40d99941..d426a6e1ac3856 100644 --- a/test/parallel/test-global-console-exists.js +++ b/test/parallel/test-global-console-exists.js @@ -2,18 +2,28 @@ const assert = require('assert'); const events = require('events'); +const leak_warning = /EventEmitter memory leak detected. 2 hello listeners/; + +var write_calls = 0; +process.stderr.write = function(data) { + if (write_calls === 0) + assert.ok(data.match(leak_warning)); + else if (write_calls === 1) + assert.ok(data.match(/Trace/)); + else + assert.ok(false, 'stderr.write should be called only twice'); + + write_calls++; +}; const old_default = events.defaultMaxListeners; events.defaultMaxListeners = 1; const e = new events.EventEmitter(); e.on('hello', function() {}); - -assert.ok(!e._events['hello'].hasOwnProperty('warned')); - e.on('hello', function() {}); -assert.ok(e._events['hello'].hasOwnProperty('warned')); +assert.equal(write_calls, 2); events.defaultMaxListeners = old_default; From ff4065eab771c8aad104047f0eba9fdf0269c22e Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 11 Jan 2016 12:43:02 -0800 Subject: [PATCH 4/6] global console exists spec cleanups eslint-disable required-modules escape regex dot --- test/parallel/test-global-console-exists.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-global-console-exists.js b/test/parallel/test-global-console-exists.js index d426a6e1ac3856..22a07f76a6e686 100644 --- a/test/parallel/test-global-console-exists.js +++ b/test/parallel/test-global-console-exists.js @@ -1,8 +1,12 @@ +/* eslint-disable required-modules */ +// ordinarily test files must require('common') but that action causes +// the global console to be compiled, defeating the purpose of this test + 'use strict'; const assert = require('assert'); const events = require('events'); -const leak_warning = /EventEmitter memory leak detected. 2 hello listeners/; +const leak_warning = /EventEmitter memory leak detected\. 2 hello listeners/; var write_calls = 0; process.stderr.write = function(data) { @@ -26,6 +30,3 @@ e.on('hello', function() {}); assert.equal(write_calls, 2); events.defaultMaxListeners = old_default; - -// this caches console, so place it after the test just to appease the linter -require('../common'); From e1a14e7db76d26c12b5ea573954db56208ca9aee Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 11 Jan 2016 16:10:06 -0800 Subject: [PATCH 5/6] Use the default export EventEmitter from events --- test/parallel/test-global-console-exists.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-global-console-exists.js b/test/parallel/test-global-console-exists.js index 22a07f76a6e686..238aea1848eff8 100644 --- a/test/parallel/test-global-console-exists.js +++ b/test/parallel/test-global-console-exists.js @@ -5,7 +5,7 @@ 'use strict'; const assert = require('assert'); -const events = require('events'); +const EventEmitter = require('events'); const leak_warning = /EventEmitter memory leak detected\. 2 hello listeners/; var write_calls = 0; @@ -20,13 +20,13 @@ process.stderr.write = function(data) { write_calls++; }; -const old_default = events.defaultMaxListeners; -events.defaultMaxListeners = 1; +const old_default = EventEmitter.defaultMaxListeners; +EventEmitter.defaultMaxListeners = 1; -const e = new events.EventEmitter(); +const e = new EventEmitter(); e.on('hello', function() {}); e.on('hello', function() {}); assert.equal(write_calls, 2); -events.defaultMaxListeners = old_default; +EventEmitter.defaultMaxListeners = old_default; From 7152739f4cefeded4d17d792eb5660448218b69a Mon Sep 17 00:00:00 2001 From: Dave Date: Tue, 12 Jan 2016 08:15:05 -0800 Subject: [PATCH 6/6] TODO validate console --- test/parallel/test-global-console-exists.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-global-console-exists.js b/test/parallel/test-global-console-exists.js index 238aea1848eff8..41d8bb9ad20d0f 100644 --- a/test/parallel/test-global-console-exists.js +++ b/test/parallel/test-global-console-exists.js @@ -27,6 +27,7 @@ const e = new EventEmitter(); e.on('hello', function() {}); e.on('hello', function() {}); +// TODO validate console assert.equal(write_calls, 2); EventEmitter.defaultMaxListeners = old_default;