GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — events: remove weak listener for event target · nodejs/node@acd9872

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

lib/internal
- event_target.js
test/parallel
- test-eventtarget-memoryleakwarning.js
- test-eventtarget.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎lib/internal/event_target.js‎`

Copy file name to clipboardExpand all lines: lib/internal/event_target.js

+34-5Lines changed: 34 additions & 5 deletions

Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,7 @@ const kWeakHandler = Symbol('kWeak');`
`62`	`62`	`const kResistStopPropagation = Symbol('kResistStopPropagation');`
`63`	`63`
`64`	`64`	`const kHybridDispatch = SymbolFor('nodejs.internal.kHybridDispatch');`
	`65`	`+const kRemoveWeakListenerHelper = Symbol('nodejs.internal.removeWeakListenerHelper');`
`65`	`66`	`const kCreateEvent = Symbol('kCreateEvent');`
`66`	`67`	`const kNewListener = Symbol('kNewListener');`
`67`	`68`	`const kRemoveListener = Symbol('kRemoveListener');`
`@@ -392,7 +393,7 @@ let weakListenersState = null;`
`392`	`393`	`let objectToWeakListenerMap = null;`
`393`	`394`	`function weakListeners() {`
`394`	`395`	`weakListenersState ??= new SafeFinalizationRegistry(`
`395`		`- (listener) => listener.remove(),`
	`396`	`+ ({ eventTarget, listener, eventType }) => eventTarget.deref()?.[kRemoveWeakListenerHelper](eventType, listener),`
`396`	`397`	`);`
`397`	`398`	`objectToWeakListenerMap ??= new SafeWeakMap();`
`398`	`399`	`return { registry: weakListenersState, map: objectToWeakListenerMap };`
`@@ -414,7 +415,7 @@ const kFlagResistStopPropagation = 1 << 6;`
`414`	`415`	`// the linked list makes dispatching faster, even if adding/removing is`
`415`	`416`	`// slower.`
`416`	`417`	`class Listener {`
`417`		`- constructor(previous, listener, once, capture, passive,`
	`418`	`+ constructor(eventTarget, eventType, previous, listener, once, capture, passive,`
`418`	`419`	`isNodeStyleListener, weak, resistStopPropagation) {`
`419`	`420`	`this.next = undefined;`
`420`	`421`	`if (previous !== undefined)`
`@@ -441,7 +442,13 @@ class Listener {`
`441`	`442`
`442`	`443`	`if (this.weak) {`
`443`	`444`	`this.callback = new SafeWeakRef(listener);`
`444`		`- weakListeners().registry.register(listener, this, this);`
	`445`	`+ weakListeners().registry.register(listener, {`
	`446`	`+ __proto__: null,`
	`447`	`+ // Weak ref so the listener won't hold the eventTarget alive`
	`448`	`+ eventTarget: new SafeWeakRef(eventTarget),`
	`449`	`+ listener: this,`
	`450`	`+ eventType,`
	`451`	`+ }, this);`
`445`	`452`	`// Make the retainer retain the listener in a WeakMap`
`446`	`453`	`weakListeners().map.set(weak, listener);`
`447`	`454`	`this.listener = this.callback;`
`@@ -604,7 +611,7 @@ class EventTarget {`
`604`	`611`	`if (root === undefined) {`
`605`	`612`	`root = { size: 1, next: undefined, resistStopPropagation: Boolean(resistStopPropagation) };`
`606`	`613`	`// This is the first handler in our linked list.`
`607`		`- new Listener(root, listener, once, capture, passive,`
	`614`	`+ new Listener(this, type, root, listener, once, capture, passive,`
`608`	`615`	`isNodeStyleListener, weak, resistStopPropagation);`
`609`	`616`	`this[kNewListener](`
`610`	`617`	`root.size,`
`@@ -631,7 +638,7 @@ class EventTarget {`
`631`	`638`	`return;`
`632`	`639`	`}`
`633`	`640`
`634`		`- new Listener(previous, listener, once, capture, passive,`
	`641`	`+ new Listener(this, type, previous, listener, once, capture, passive,`
`635`	`642`	`isNodeStyleListener, weak, resistStopPropagation);`
`636`	`643`	`root.size++;`
`637`	`644`	`root.resistStopPropagation \|\|= Boolean(resistStopPropagation);`
`@@ -674,6 +681,28 @@ class EventTarget {`
`674`	`681`	`}`
`675`	`682`	`}`
`676`	`683`
	`684`	`+ [kRemoveWeakListenerHelper](type, listener) {`
	`685`	`+ const root = this[kEvents].get(type);`
	`686`	`+ if (root === undefined \|\| root.next === undefined)`
	`687`	`+ return;`
	`688`	`+`
	`689`	`+ const capture = listener.capture === true;`
	`690`	`+`
	`691`	`+ let handler = root.next;`
	`692`	`+ while (handler !== undefined) {`
	`693`	`+ if (handler === listener) {`
	`694`	`+ handler.remove();`
	`695`	`+ root.size--;`
	`696`	`+ if (root.size === 0)`
	`697`	`+ this[kEvents].delete(type);`
	`698`	`+ // Undefined is passed as the listener as the listener was GCed`
	`699`	`+ this[kRemoveListener](root.size, type, undefined, capture);`
	`700`	`+ break;`
	`701`	`+ }`
	`702`	`+ handler = handler.next;`
	`703`	`+ }`
	`704`	`+ }`
	`705`	`+`
`677`	`706`	`/**`
`678`	`707`	`* @param {Event} event`
`679`	`708`	`*/`

Collapse file

`‎test/parallel/test-eventtarget-memoryleakwarning.js‎`

Copy file name to clipboardExpand all lines: test/parallel/test-eventtarget-memoryleakwarning.js

+20-1Lines changed: 20 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -1,11 +1,13 @@`
`1`		`-// Flags: --no-warnings`
	`1`	`+// Flags: --expose-internals --no-warnings --expose-gc`
`2`	`2`	`'use strict';`
`3`	`3`	`const common = require('../common');`
`4`	`4`	`const {`
`5`	`5`	`setMaxListeners,`
`6`	`6`	`EventEmitter,`
`7`	`7`	`} = require('events');`
`8`	`8`	`const assert = require('assert');`
	`9`	`+const { kWeakHandler } = require('internal/event_target');`
	`10`	`+const { setTimeout } = require('timers/promises');`
`9`	`11`
`10`	`12`	`common.expectWarning({`
`11`	`13`	`MaxListenersExceededWarning: [`
`@@ -73,3 +75,20 @@ common.expectWarning({`
`73`	`75`	`setMaxListeners(2, ee);`
`74`	`76`	`assert.strictEqual(ee.getMaxListeners(), 2);`
`75`	`77`	`}`
	`78`	`+`
	`79`	`+{`
	`80`	`+ (async () => {`
	`81`	`+ // Test that EventTarget listener don't emit MaxListenersExceededWarning for weak listeners that GCed`
	`82`	`+ const et = new EventTarget();`
	`83`	`+ setMaxListeners(2, et);`
	`84`	`+`
	`85`	`+ for (let i = 0; i <= 3; i++) {`
	`86`	`+ et.addEventListener('foo', () => {}, {`
	`87`	`+ [kWeakHandler]: {},`
	`88`	`+ });`
	`89`	`+`
	`90`	`+ await setTimeout(0);`
	`91`	`+ global.gc();`
	`92`	`+ }`
	`93`	`+ })().then(common.mustCall(), common.mustNotCall());`
	`94`	`+}`

Collapse file

`‎test/parallel/test-eventtarget.js‎`

Copy file name to clipboardExpand all lines: test/parallel/test-eventtarget.js

+2-2Lines changed: 2 additions & 2 deletions

Original file line number	Diff line number	Diff line change
`@@ -16,8 +16,8 @@ const {`
`16`	`16`
`17`	`17`	`const { once } = require('events');`
`18`	`18`
`19`		`-const { promisify, inspect } = require('util');`
`20`		`-const delay = promisify(setTimeout);`
	`19`	`+const { inspect } = require('util');`
	`20`	`+const { setTimeout: delay } = require('timers/promises');`
`21`	`21`
`22`	`22`	`// The globals are defined.`
`23`	`23`	`ok(Event);`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.