microsoft/playwright
1
1
Fork 0
mirror of https://github.com/microsoft/playwright.git synced 2024-12-01 08:34:02 +03:00
Code Issues Projects Releases Wiki Activity

fix(leak): do not retain test fixtures in worker fixtures (#14363)

Browse Source
This commit is contained in:
Pavel Feldman 2022-05-23 16:54:56 -07:00 committed by GitHub
parent 99f5eff400
commit 71a55c74da
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 107 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
packages/playwright-test/src/fixtures.ts
View File

@ -53,18 +53,18 @@ type FixtureRegistration = {
class Fixture {
runner: FixtureRunner;
registration: FixtureRegistration;
usages: Set<Fixture>;
value: any;
_useFuncFinished: ManualPromise<void> | undefined;
_selfTeardownComplete: Promise<void> | undefined;
_teardownWithDepsComplete: Promise<void> | undefined;
_runnableDescription: FixtureDescription;
_deps = new Set<Fixture>();
_usages = new Set<Fixture>();
constructor(runner: FixtureRunner, registration: FixtureRegistration) {
this.runner = runner;
this.registration = registration;
this.usages = new Set();
this.value = null;
this._runnableDescription = {
title: `fixture "${this.registration.customTitle || this.registration.name}" setup`,
@ -86,7 +86,12 @@ class Fixture {
for (const name of this.registration.deps) {
const registration = this.runner.pool!.resolveDependency(this.registration, name)!;
const dep = await this.runner.setupFixtureForRegistration(registration, testInfo);
dep.usages.add(this);
// Fixture teardown is root => leafs, when we need to teardown a fixture,
// it recursively tears down its usages first.
dep._usages.add(this);
// Don't forget to decrement all usages when fixture goes.
// Otherwise worker-scope fixtures will retain test-scope fixtures forever.
this._deps.add(dep);
params[name] = dep.value;
}
@ -125,9 +130,13 @@ class Fixture {
if (typeof this.registration.fn !== 'function')
return;
try {
for (const fixture of this.usages)
for (const fixture of this._usages)
await fixture.teardown(timeoutManager);
this.usages.clear();
if (this._usages.size !== 0) {
// TODO: replace with assert.
console.error('Internal error: fixture integrity at', this._runnableDescription.title); // eslint-disable-line no-console
this._usages.clear();
}
if (this._useFuncFinished) {
debugTest(`teardown ${this.registration.name}`);
this._runnableDescription.title = `fixture "${this.registration.customTitle || this.registration.name}" teardown`;
@ -137,6 +146,8 @@ class Fixture {
timeoutManager.setCurrentFixture(undefined);
}
} finally {
for (const dep of this._deps)
dep._usages.delete(this);
this.runner.instanceForId.delete(this.registration.id);
}
}

42
tests/config/queryObjects.ts Normal file
View File

@ -0,0 +1,42 @@
/**
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
export async function queryObjectCount(type: Function): Promise<number> {
globalThis.typeForQueryObjects = type;
const session: import('inspector').Session = new (require('node:inspector').Session)();
session.connect();
try {
await new Promise(f => session.post('Runtime.enable', f));
const { result: constructorFunction } = await new Promise(f => session.post('Runtime.evaluate', {
expression: `globalThis.typeForQueryObjects.prototype`,
includeCommandLineAPI: true,
}, (_, result) => f(result))) as any;
const { objects: instanceArray } = await new Promise(f => session.post('Runtime.queryObjects', {
prototypeObjectId: constructorFunction.objectId
}, (_, result) => f(result))) as any;
const { result: { value } } = await new Promise(f => session.post('Runtime.callFunctionOn', {
functionDeclaration: 'function (arr) { return this.length; }',
objectId: instanceArray.objectId,
arguments: [{ objectId: instanceArray.objectId }],
}, (_, result) => f(result as any)));
return value;
} finally {
session.disconnect();
}
}

80
tests/library/channels.spec.ts
View File

@ -15,7 +15,6 @@
* limitations under the License.
*/
import fs from 'fs';
import domain from 'domain';
import { playwrightTest as it, expect } from '../config/browserTest';
@ -205,85 +204,6 @@ it('should work with the domain module', async ({ browserType, server, browserNa
throw err;
});
it('make sure that the client/server side context, page, etc. objects were garbage collected', async ({ browserName, server, childProcess }, testInfo) => {
// WeakRef was added in Node.js 14
it.skip(parseInt(process.version.slice(1), 10) < 14);
const scriptPath = testInfo.outputPath('test.js');
const script = `
const playwright = require(${JSON.stringify(require.resolve('playwright'))});
const { kTestSdkObjects } = require(${JSON.stringify(require.resolve('../../packages/playwright-core/lib/server/instrumentation'))});
const { existingDispatcher } = require(${JSON.stringify(require.resolve('../../packages/playwright-core/lib/server/dispatchers/dispatcher'))});
const toImpl = playwright._toImpl;
(async () => {
const clientSideObjectsSizeBeforeLaunch = playwright._connection._objects.size;
const browser = await playwright['${browserName}'].launch();
const objectRefs = [];
const dispatcherRefs = [];
for (let i = 0; i < 5; i++) {
const context = await browser.newContext();
const page = await context.newPage();
const response = await page.goto('${server.EMPTY_PAGE}');
objectRefs.push(new WeakRef(toImpl(context)));
objectRefs.push(new WeakRef(toImpl(page)));
objectRefs.push(new WeakRef(toImpl(response)));
dispatcherRefs.push(
new WeakRef(existingDispatcher(toImpl(context))),
new WeakRef(existingDispatcher(toImpl(page))),
new WeakRef(existingDispatcher(toImpl(response))),
);
}
assertServerSideObjectsExistance(true);
assertServerSideDispatchersExistance(true);
await browser.close();
for (let i = 0; i < 5; i++) {
await new Promise(resolve => setTimeout(resolve, 100));
global.gc();
}
assertServerSideObjectsExistance(false);
assertServerSideDispatchersExistance(false);
assertClientSideObjects();
function assertClientSideObjects() {
if (playwright._connection._objects.size !== clientSideObjectsSizeBeforeLaunch)
throw new Error('Client-side objects were not cleaned up');
}
function assertServerSideObjectsExistance(expected) {
for (const ref of objectRefs) {
if (kTestSdkObjects.has(ref.deref()) !== expected) {
throw new Error('Unexpected SdkObject existence! (expected: ' + expected + ')');
}
}
}
function assertServerSideDispatchersExistance(expected) {
for (const ref of dispatcherRefs) {
const impl = ref.deref();
if (!!impl !== expected)
throw new Error('Dispatcher is still alive!');
}
}
})();
`;
await fs.promises.writeFile(scriptPath, script);
const testSdkObjectsProcess = childProcess({
command: ['node', '--expose-gc', scriptPath],
env: {
...process.env,
_PW_INTERNAL_COUNT_SDK_OBJECTS: '1',
}
});
const { exitCode } = await testSdkObjectsProcess.exited;
expect(exitCode).toBe(0);
});
async function expectScopeState(object, golden) {
golden = trimGuids(golden);
const remoteState = trimGuids(await object._channel.debugScopeState());

49
tests/stress/heap.spec.ts Normal file
View File

@ -0,0 +1,49 @@
/**
* Copyright (c) Microsoft Corporation. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { contextTest as test, expect } from '../config/browserTest';
import { queryObjectCount } from '../config/queryObjects';
test.describe.configure({ mode: 'serial' });
for (let i = 0; i < 3; ++i) {
test(`test #${i} to request page and context`, async ({ page, context }) => {
// This test is here to create page instance
});
}
test('test to request page and context', async ({ page, context }) => {
// This test is here to create page instance
});
test('should not leak fixtures w/ page', async ({ page }) => {
expect(await queryObjectCount(require('../../packages/playwright-core/lib/client/page').Page)).toBe(1);
expect(await queryObjectCount(require('../../packages/playwright-core/lib/client/browserContext').BrowserContext)).toBe(1);
expect(await queryObjectCount(require('../../packages/playwright-core/lib/client/browser').Browser)).toBe(1);
});
test('should not leak fixtures w/o page', async ({}) => {
expect(await queryObjectCount(require('../../packages/playwright-core/lib/client/page').Page)).toBe(0);
expect(await queryObjectCount(require('../../packages/playwright-core/lib/client/browserContext').BrowserContext)).toBe(0);
expect(await queryObjectCount(require('../../packages/playwright-core/lib/client/browser').Browser)).toBe(1);
});
test('should not leak server-side objects', async ({ page }) => {
expect(await queryObjectCount(require('../../packages/playwright-core/lib/server/page').Page)).toBe(1);
// 4 is because v8 heap creates obejcts for descendant classes, so WKContext, CRContext, FFContext and our context instance.
expect(await queryObjectCount(require('../../packages/playwright-core/lib/server/browserContext').BrowserContext)).toBe(4);
expect(await queryObjectCount(require('../../packages/playwright-core/lib/server/browser').Browser)).toBe(4);
});