diff --git a/src/common/drain/drain.js b/src/common/drain/drain.js index 35de70286..3eb00af3e 100644 --- a/src/common/drain/drain.js +++ b/src/common/drain/drain.js @@ -32,8 +32,9 @@ export function registerDrain (agentIdentifier, group) { * @param {*} group - The named "bucket" to be removed from the registry */ export function deregisterDrain (agentIdentifier, group) { - curateRegistry(agentIdentifier) + if (!agentIdentifier || !registry[agentIdentifier]) return if (registry[agentIdentifier].get(group)) registry[agentIdentifier].delete(group) + drainGroup(agentIdentifier, group, false) if (registry[agentIdentifier].size) checkCanDrainAll(agentIdentifier) } @@ -86,29 +87,33 @@ function checkCanDrainAll (agentIdentifier) { * the subscribed handlers for the group. * @param {*} group - The name of a particular feature's event "bucket". */ -function drainGroup (agentIdentifier, group) { +function drainGroup (agentIdentifier, group, activateGroup = true) { const baseEE = agentIdentifier ? ee.get(agentIdentifier) : ee const handlers = defaultRegister.handlers // other storage in registerHandler if (!baseEE.backlog || !handlers) return - var bufferedEventsInGroup = baseEE.backlog[group] - var groupHandlers = handlers[group] // each group in the registerHandler storage - if (groupHandlers) { - // We don't cache the length of the buffer while looping because events might still be added while processing. - for (var i = 0; bufferedEventsInGroup && i < bufferedEventsInGroup.length; ++i) { // eslint-disable-line no-unmodified-loop-condition - emitEvent(bufferedEventsInGroup[i], groupHandlers) - } + // Only activated features being drained should run queued listeners on buffered events. Deactivated features only need to release memory. + if (activateGroup) { + const bufferedEventsInGroup = baseEE.backlog[group] + const groupHandlers = handlers[group] // each group in the registerHandler storage + if (groupHandlers) { + // We don't cache the length of the buffer while looping because events might still be added while processing. + for (let i = 0; bufferedEventsInGroup && i < bufferedEventsInGroup.length; ++i) { // eslint-disable-line no-unmodified-loop-condition + emitEvent(bufferedEventsInGroup[i], groupHandlers) + } - mapOwn(groupHandlers, function (eventType, handlerRegistrationList) { - mapOwn(handlerRegistrationList, function (i, registration) { - // registration is an array of: [targetEE, eventHandler] - registration[0].on(eventType, registration[1]) + mapOwn(groupHandlers, function (eventType, handlerRegistrationList) { + mapOwn(handlerRegistrationList, function (i, registration) { + // registration is an array of: [targetEE, eventHandler] + registration[0].on(eventType, registration[1]) + }) }) - }) + } } + if (!baseEE.isolatedBacklog) delete handlers[group] baseEE.backlog[group] = null - baseEE.emit('drain-' + group, []) + baseEE.emit('drain-' + group, []) // TODO: Code exists purely for a unit test and needs to be refined } /** diff --git a/tests/specs/cleanup-on-halt.e2e.js b/tests/specs/cleanup-on-halt.e2e.js new file mode 100644 index 000000000..18bb546a9 --- /dev/null +++ b/tests/specs/cleanup-on-halt.e2e.js @@ -0,0 +1,32 @@ +import { testRumRequest } from '../../tools/testing-server/utils/expect-tests' + +describe('Memory leaks', () => { + it('does not occur on ee backlog when RUM flags are 0', async () => { + await browser.testHandle.scheduleReply('bamServer', { + test: testRumRequest, + body: JSON.stringify({ st: 0, sr: 0, err: 0, ins: 0, spa: 0, loaded: 1 }) + }) + // This test relies on features to call deregisterDrain when not enabled by flags which in turn should clear their backlogs. + + await browser.url(await browser.testHandle.assetURL('obfuscate-pii.html')).then(() => browser.waitForAgentLoad()) + const backlog = await browser.execute(function () { + const backlog = {} + for (const key in newrelic.ee.backlog) { + const array = newrelic.ee.backlog[key] + backlog[key] = array === null ? 0 : array.length + } + return backlog + }) + + expect(backlog).toEqual(expect.objectContaining({ + ajax: 0, // ajax does not rely on any flags anyways so it's always drained + jserrors: 0, + metrics: 0, + page_action: 0, + page_view_event: 0, // no handler + page_view_timing: 0, // does not rely on any flags + session_trace: 0, + spa: 0 + })) + }) +}) diff --git a/tests/specs/harvesting/disable-harvesting.e2e.js b/tests/specs/harvesting/disable-harvesting.e2e.js index 1bcc6664b..05f66f6c2 100644 --- a/tests/specs/harvesting/disable-harvesting.e2e.js +++ b/tests/specs/harvesting/disable-harvesting.e2e.js @@ -8,7 +8,6 @@ describe('disable harvesting', () => { st: 1, err: 0, ins: 1, - cap: 1, spa: 1, loaded: 1 }) @@ -32,7 +31,6 @@ describe('disable harvesting', () => { st: 1, err: 1, ins: 1, - cap: 1, spa: 0, loaded: 1 }) @@ -59,7 +57,6 @@ describe('disable harvesting', () => { st: 1, err: 1, ins: 0, - cap: 1, spa: 1, loaded: 1 }) @@ -81,7 +78,6 @@ describe('disable harvesting', () => { st: 0, err: 1, ins: 1, - cap: 1, spa: 1, loaded: 1 }) diff --git a/tests/specs/session-replay/initialization.e2e.js b/tests/specs/session-replay/initialization.e2e.js index d986d8c69..8072dd333 100644 --- a/tests/specs/session-replay/initialization.e2e.js +++ b/tests/specs/session-replay/initialization.e2e.js @@ -12,7 +12,6 @@ async function disqualifySR () { sts: 1, err: 1, ins: 1, - cap: 1, spa: 1, loaded: 1, sr: 0, diff --git a/tests/specs/session-replay/session-state.e2e.js b/tests/specs/session-replay/session-state.e2e.js index 358fdf6cb..6d3cff3fb 100644 --- a/tests/specs/session-replay/session-state.e2e.js +++ b/tests/specs/session-replay/session-state.e2e.js @@ -85,6 +85,9 @@ describe.withBrowsersMatching(notIE)('session manager state behavior', () => { await browser.enableSessionReplay() await browser.url(await browser.testHandle.assetURL('instrumented.html', srConfig())) .then(() => browser.waitForSessionReplayRecording()) + + await browser.closeWindow() + await browser.switchToWindow((await browser.getWindowHandles())[0]) }) }) diff --git a/tests/specs/session-trace/trace-nodes.e2e.js b/tests/specs/session-trace/trace-nodes.e2e.js index f42188359..d3f0d564c 100644 --- a/tests/specs/session-trace/trace-nodes.e2e.js +++ b/tests/specs/session-trace/trace-nodes.e2e.js @@ -33,7 +33,7 @@ describe('Trace nodes', () => { await browser.testHandle.clearScheduledReplies('bamServer') await browser.testHandle.scheduleReply('bamServer', { test: testRumRequest, - body: JSON.stringify({ st: 0, sts: 0, sr: 0, err: 1, ins: 1, cap: 1, spa: 0, loaded: 1 }) + body: JSON.stringify({ st: 0, sts: 0, sr: 0, err: 1, ins: 1, spa: 0, loaded: 1 }) }) const url = await browser.testHandle.assetURL('pagehide.html', stConfig()) diff --git a/tests/specs/soft_navigations/integration.e2e.js b/tests/specs/soft_navigations/integration.e2e.js index c40a235ea..4a31a0864 100644 --- a/tests/specs/soft_navigations/integration.e2e.js +++ b/tests/specs/soft_navigations/integration.e2e.js @@ -32,7 +32,7 @@ describe('Soft navigations', () => { it('does not harvest when spa is blocked by rum response', async () => { await browser.testHandle.scheduleReply('bamServer', { test: testRumRequest, - body: `${JSON.stringify({ st: 1, err: 1, ins: 1, cap: 1, spa: 0, loaded: 1 })}` + body: `${JSON.stringify({ st: 1, err: 1, ins: 1, spa: 0, loaded: 1 })}` }) let url = await browser.testHandle.assetURL('instrumented.html', config) diff --git a/tests/unit/common/drain/drain.test.js b/tests/unit/common/drain/drain.test.js index 3a1df2688..76248229e 100644 --- a/tests/unit/common/drain/drain.test.js +++ b/tests/unit/common/drain/drain.test.js @@ -1,8 +1,9 @@ -let registerDrain, drain, ee +let registerDrain, drain, deregisterDrain, ee, registerHandler beforeEach(async () => { jest.resetModules() - ;({ registerDrain, drain } = await import('../../../../src/common/drain/drain')) + ;({ registerDrain, drain, deregisterDrain } = await import('../../../../src/common/drain/drain')) ;({ ee } = await import('../../../../src/common/event-emitter/contextual-ee')) + ;({ registerHandler } = await import('../../../../src/common/event-emitter/register-handler')) }) test('can register a feat and drain it', () => { @@ -25,6 +26,15 @@ test('other unregistered drains do not affect feat reg & drain', () => { expect(emitSpy).toHaveBeenCalledWith('drain-page_view_event', expect.anything()) }) +test('unregistering groups clears their handlers from the buffering system', () => { + registerDrain('abcd', 'pumbaa') + registerHandler('tusks', () => {}, 'pumbaa', ee) + expect(registerHandler.handlers.pumbaa).toEqual({ tusks: expect.anything() }) + + deregisterDrain('abcd', 'pumbaa') + expect(registerHandler.handlers.pumbaa).toBeUndefined() +}) + describe('drain', () => { test('does not execute until all registered groups calls it and in order', () => { registerDrain('abcd', 'page_view_timing')