Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 36.7k
events: optimize once() and removeListener()#64373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -86,7 +86,6 @@ const { addAbortListener } = require('internal/events/abort_listener'); | ||
| const kCapture = Symbol('kCapture'); | ||
| const kErrorMonitor = Symbol('events.errorMonitor'); | ||
| const kShapeMode = Symbol('shapeMode'); | ||
| const kEmitting = Symbol('events.emitting'); | ||
| const kMaxEventTargetListeners = Symbol('events.maxEventTargetListeners'); | ||
| const kMaxEventTargetListenersWarned = | ||
| @@ -335,9 +334,6 @@ EventEmitter.init = function(opts) { | ||
| this._events === ObjectGetPrototypeOf(this)._events) { | ||
| this._events = { __proto__: null }; | ||
| this._eventsCount = 0; | ||
| this[kShapeMode] = false; | ||
| } else { | ||
| this[kShapeMode] = true; | ||
| } | ||
| this._maxListeners ||= undefined; | ||
| @@ -446,6 +442,39 @@ function enhanceStackTrace(err, own) { | ||
| return err.stack + sep + ArrayPrototypeJoin(ownStack, '\n'); | ||
| } | ||
| function getUnhandledErrorException(ee, args) { | ||
| let er; | ||
| if (args.length > 0) | ||
| er = args[0]; | ||
| if (er instanceof Error) { | ||
| try { | ||
| const capture = {}; | ||
| ErrorCaptureStackTrace(capture, EventEmitter.prototype.emit); | ||
| ObjectDefineProperty(er, kEnhanceStackBeforeInspector, { | ||
| __proto__: null, | ||
| value: FunctionPrototypeBind(enhanceStackTrace, ee, er, capture), | ||
| configurable: true, | ||
| }); | ||
| } catch { | ||
| // Continue regardless of error. | ||
| } | ||
| return er; | ||
| } | ||
| let stringifiedEr; | ||
| try { | ||
| stringifiedEr = inspect(er); | ||
| } catch { | ||
| stringifiedEr = er; | ||
| } | ||
| // At least give some kind of context to the user | ||
| const err = new ERR_UNHANDLED_ERROR(stringifiedEr); | ||
| err.context = er; | ||
| return err; | ||
| } | ||
| /** | ||
| * Synchronously calls each of the listeners registered | ||
| * for the event. | ||
| @@ -466,38 +495,10 @@ EventEmitter.prototype.emit = function emit(type, ...args) { | ||
| // If there is no 'error' event listener then throw. | ||
| if (doError) { | ||
| let er; | ||
| if (args.length > 0) | ||
| er = args[0]; | ||
| if (er instanceof Error) { | ||
| try { | ||
| const capture = {}; | ||
| ErrorCaptureStackTrace(capture, EventEmitter.prototype.emit); | ||
| ObjectDefineProperty(er, kEnhanceStackBeforeInspector, { | ||
| __proto__: null, | ||
| value: FunctionPrototypeBind(enhanceStackTrace, this, er, capture), | ||
| configurable: true, | ||
| }); | ||
| } catch { | ||
| // Continue regardless of error. | ||
| } | ||
| // Note: The comments on the `throw` lines are intentional, they show | ||
| // up in Node's output if this results in an unhandled exception. | ||
| throw er; // Unhandled 'error' event | ||
| } | ||
| let stringifiedEr; | ||
| try { | ||
| stringifiedEr = inspect(er); | ||
| } catch { | ||
| stringifiedEr = er; | ||
| } | ||
| // At least give some kind of context to the user | ||
| const err = new ERR_UNHANDLED_ERROR(stringifiedEr); | ||
| err.context = er; | ||
| throw err; // Unhandled 'error' event | ||
| const er = getUnhandledErrorException(this, args); | ||
| // Note: The comments on the `throw` lines are intentional, they show | ||
| // up in Node's output if this results in an unhandled exception. | ||
| throw er; // Unhandled 'error' event | ||
| } | ||
| const handler = events[type]; | ||
| @@ -584,20 +585,23 @@ function _addListener(target, type, listener, prepend) { | ||
| // Check for listener leak | ||
| m = _getMaxListeners(target); | ||
| if (m > 0 && existing.length > m && !existing.warned) { | ||
| existing.warned = true; | ||
| // No error code for this since it is a Warning | ||
| const w = genericNodeError( | ||
| `Possible EventEmitter memory leak detected. ${existing.length} ${String(type)} listeners ` + | ||
| `added to ${inspect(target, { depth: -1 })}. MaxListeners is ${m}. Use emitter.setMaxListeners() to increase limit`, | ||
| { name: 'MaxListenersExceededWarning', emitter: target, type: type, count: existing.length }); | ||
| process.emitWarning(w); | ||
| } | ||
| if (m > 0 && existing.length > m && !existing.warned) | ||
| warnMaxListenersExceeded(target, type, existing, m); | ||
| } | ||
| return target; | ||
| } | ||
| function warnMaxListenersExceeded(target, type, existing, m) { | ||
| existing.warned = true; | ||
| // No error code for this since it is a Warning | ||
| const w = genericNodeError( | ||
| `Possible EventEmitter memory leak detected. ${existing.length} ${String(type)} listeners ` + | ||
| `added to ${inspect(target, { depth: -1 })}. MaxListeners is ${m}. Use emitter.setMaxListeners() to increase limit`, | ||
| { name: 'MaxListenersExceededWarning', emitter: target, type: type, count: existing.length }); | ||
| process.emitWarning(w); | ||
| } | ||
| /** | ||
| * Adds a listener to the event emitter. | ||
| * @param {string | symbol} type | ||
| @@ -622,22 +626,16 @@ EventEmitter.prototype.prependListener = | ||
| return _addListener(this, type, listener, true); | ||
| }; | ||
| function onceWrapper() { | ||
| if (!this.fired) { | ||
| this.target.removeListener(this.type, this.wrapFn); | ||
| this.fired = true; | ||
| if (arguments.length === 0) | ||
| return this.listener.call(this.target); | ||
| return ReflectApply(this.listener, this.target, arguments); | ||
| } | ||
| } | ||
| function _onceWrap(target, type, listener) { | ||
| const state = { fired: false, wrapFn: undefined, target, type, listener }; | ||
| const wrapped = onceWrapper.bind(state); | ||
| wrapped.listener = listener; | ||
| state.wrapFn = wrapped; | ||
| return wrapped; | ||
| let fired = false; | ||
| function wrapper(...args) { | ||
| if (fired) return; | ||
| fired = true; | ||
| target.removeListener(type, wrapper); | ||
| return ReflectApply(listener, target, args); | ||
| } | ||
| wrapper.listener = listener; | ||
| return wrapper; | ||
| } | ||
| /** | ||
| @@ -689,13 +687,12 @@ EventEmitter.prototype.removeListener = | ||
| if (list === listener || list.listener === listener) { | ||
| this._eventsCount -= 1; | ||
| if (this[kShapeMode]) { | ||
| events[type] = undefined; | ||
| } else if (this._eventsCount === 0) { | ||
| this._events = { __proto__: null }; | ||
| } else { | ||
| delete events[type]; | ||
| } | ||
| // Leave the key in place with an `undefined` value: repeatedly | ||
| // adding and removing a listener for the same event this way keeps | ||
| // the `events` object in the same shape and avoids both a `delete` | ||
| // (which would put the object into dictionary mode) and allocating | ||
| // a fresh object when the last listener is removed. | ||
| events[type] = undefined; | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can cause memory leaks when using unstructured event names, I.e random ids etc... | ||
| if (events.removeListener !== undefined) | ||
| this.emit('removeListener', type, list.listener || listener); | ||
| @@ -755,7 +752,6 @@ EventEmitter.prototype.removeAllListeners = | ||
| else | ||
| delete events[type]; | ||
| } | ||
| this[kShapeMode] = false; | ||
| return this; | ||
| } | ||
| @@ -768,7 +764,6 @@ EventEmitter.prototype.removeAllListeners = | ||
| this.removeAllListeners('removeListener'); | ||
| this._events = { __proto__: null }; | ||
| this._eventsCount = 0; | ||
| this[kShapeMode] = false; | ||
| return this; | ||
| } | ||
| @@ -868,7 +863,16 @@ EventEmitter.prototype.listenerCount = function listenerCount(type, listener) { | ||
| * @returns {(string | symbol)[]} | ||
| */ | ||
| EventEmitter.prototype.eventNames = function eventNames() { | ||
| return this._eventsCount > 0 ? ReflectOwnKeys(this._events) : []; | ||
| if (this._eventsCount === 0) | ||
| return []; | ||
| const events = this._events; | ||
| const names = []; | ||
| for (const key of ReflectOwnKeys(events)) { | ||
| // Removed listeners leave the key in place with an `undefined` value. | ||
| if (events[key] !== undefined) | ||
| ArrayPrototypePush(names, key); | ||
| } | ||
| return names; | ||
| }; | ||
| function arrayClone(arr) { | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you expand this to explain why we're ignoring the error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this isn't new but it's always bugged me that's it's not explained :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the lines above we are trying to expand the stacktrace. If we can't for whatever reasons, we don't hide the original error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jasnell do you want me to update the comment?