mirror of
https://github.com/nodejs/node.git
synced 2026-10-28 10:15:07 +08:00
perf_hooks: fix PerformanceObserver observer count leaks
Observing the same entry type more than once on a PerformanceObserver,
through repeated `observe({ type })` calls or duplicates in
`entryTypes`, incremented the internal observer count each time, while
`disconnect()` decremented it only once per observed type. The counts
never returned to zero, so GC tracking stayed installed, and entries
for types such as `'http'` and `'dns'` kept being created after all
observers had disconnected.
Count each entry type at most once per observer.
Assisted-by: OpenCode
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66097
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
1 parent
e31193da3d
commit
4290a2d5b9
2 files changed
+99
-5
No files matched your search
@@ -291,16 +291,23 @@ class PerformanceObserver {
|
||||
maybeDecrementObserverCounts(this.#entryTypes);
|
||||
this.#entryTypes.clear();
|
||||
for (let n = 0; n < entryTypes.length; n++) {
|
||||
if (ArrayPrototypeIncludes(kSupportedEntryTypes, entryTypes[n])) {
|
||||
this.#entryTypes.add(entryTypes[n]);
|
||||
maybeIncrementObserverCount(entryTypes[n]);
|
||||
const entryType = entryTypes[n];
|
||||
// Count each entry type at most once per observer, as disconnect()
|
||||
// decrements the counts once per observed type.
|
||||
if (ArrayPrototypeIncludes(kSupportedEntryTypes, entryType) &&
|
||||
!this.#entryTypes.has(entryType)) {
|
||||
this.#entryTypes.add(entryType);
|
||||
maybeIncrementObserverCount(entryType);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
if (!ArrayPrototypeIncludes(kSupportedEntryTypes, type))
|
||||
return;
|
||||
this.#entryTypes.add(type);
|
||||
maybeIncrementObserverCount(type);
|
||||
// Observing the same type again only replaces the options.
|
||||
if (!this.#entryTypes.has(type)) {
|
||||
this.#entryTypes.add(type);
|
||||
maybeIncrementObserverCount(type);
|
||||
}
|
||||
if (buffered) {
|
||||
const entries = filterBufferMapByNameAndType(undefined, type);
|
||||
SafeArrayPrototypePushApply(this.#buffer, entries);
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
// Flags: --expose-internals
|
||||
'use strict';
|
||||
|
||||
// Tests that the observer counts, which gate the creation of performance
|
||||
// entries, return to zero once observers disconnect, however the entry types
|
||||
// were observed.
|
||||
|
||||
require('../common');
|
||||
const assert = require('node:assert');
|
||||
const { PerformanceObserver } = require('node:perf_hooks');
|
||||
const { internalBinding } = require('internal/test/binding');
|
||||
const { hasObserver } = require('internal/perf/observe');
|
||||
|
||||
const {
|
||||
observerCounts,
|
||||
constants: {
|
||||
NODE_PERFORMANCE_ENTRY_TYPE_GC,
|
||||
NODE_PERFORMANCE_ENTRY_TYPE_HTTP,
|
||||
NODE_PERFORMANCE_ENTRY_TYPE_DNS,
|
||||
},
|
||||
} = internalBinding('performance');
|
||||
|
||||
const kTypes = {
|
||||
gc: NODE_PERFORMANCE_ENTRY_TYPE_GC,
|
||||
http: NODE_PERFORMANCE_ENTRY_TYPE_HTTP,
|
||||
dns: NODE_PERFORMANCE_ENTRY_TYPE_DNS,
|
||||
};
|
||||
|
||||
function assertCounts(expected) {
|
||||
for (const { 0: type, 1: index } of Object.entries(kTypes)) {
|
||||
const count = expected[type] ?? 0;
|
||||
assert.strictEqual(observerCounts[index], count,
|
||||
`observer count for '${type}'`);
|
||||
assert.strictEqual(hasObserver(type), count > 0, `hasObserver('${type}')`);
|
||||
}
|
||||
}
|
||||
|
||||
assertCounts({});
|
||||
|
||||
{
|
||||
// Observing the same type more than once counts it once.
|
||||
const obs = new PerformanceObserver(() => {});
|
||||
for (const type of ['gc', 'http', 'dns']) {
|
||||
obs.observe({ type });
|
||||
obs.observe({ type });
|
||||
}
|
||||
assertCounts({ gc: 1, http: 1, dns: 1 });
|
||||
obs.disconnect();
|
||||
assertCounts({});
|
||||
// Disconnecting again must not decrement the counts any further.
|
||||
obs.disconnect();
|
||||
assertCounts({});
|
||||
}
|
||||
|
||||
{
|
||||
// Duplicate entry types are counted once.
|
||||
const obs = new PerformanceObserver(() => {});
|
||||
obs.observe({ entryTypes: ['http', 'http', 'gc', 'gc'] });
|
||||
assertCounts({ gc: 1, http: 1 });
|
||||
obs.disconnect();
|
||||
assertCounts({});
|
||||
}
|
||||
|
||||
{
|
||||
// Replacing the observed entry types updates the counts.
|
||||
const obs = new PerformanceObserver(() => {});
|
||||
obs.observe({ entryTypes: ['gc', 'http'] });
|
||||
assertCounts({ gc: 1, http: 1 });
|
||||
obs.observe({ entryTypes: ['http'] });
|
||||
assertCounts({ http: 1 });
|
||||
obs.disconnect();
|
||||
assertCounts({});
|
||||
}
|
||||
|
||||
{
|
||||
// Each observer is counted separately.
|
||||
const obs1 = new PerformanceObserver(() => {});
|
||||
const obs2 = new PerformanceObserver(() => {});
|
||||
obs1.observe({ type: 'gc' });
|
||||
obs2.observe({ type: 'gc' });
|
||||
obs2.observe({ type: 'gc' });
|
||||
assertCounts({ gc: 2 });
|
||||
obs1.disconnect();
|
||||
assertCounts({ gc: 1 });
|
||||
obs2.disconnect();
|
||||
assertCounts({});
|
||||
}
|
||||
Reference in new issue
Block a user