chore: remove separate process that cleans up directories (#24376)

A separate process is `spawnSync`'ed on process exit to cleanup
temporary directories, introduced in #13769 that followed up after
#13343.

A separate process might stall for various fs-related issues, which
prevents the original process from exiting.

With the recent changes, we always gracefully close and cleanup after
all launched executables before calling `process.exit()`, and so it
should only be possible to leave temp directories when using Playwright
and calling `process.exit()` programmatically without closing browsers.

We can now drop the extra process and rely on `rimraf.sync` for
last-resort cleanup in these rare circumstances.
This commit is contained in:
Dmitry Gozman 2023-07-24 15:24:29 -07:00 committed by GitHub
parent 1abff78de2
commit 4c8912f74e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 10 additions and 37 deletions

View File

@ -17,9 +17,9 @@
import * as childProcess from 'child_process';
import * as readline from 'readline';
import * as path from 'path';
import { isUnderTest } from './';
import { removeFolders } from './fileUtils';
import { rimraf } from '../utilsBundle';
export type Env = {[key: string]: string | number | boolean | undefined};
@ -243,13 +243,12 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
function killProcessAndCleanup() {
killProcess();
options.log(`[pid=${spawnedProcess.pid || 'N/A'}] starting temporary directories cleanup`);
if (options.tempDirectories.length) {
const cleanupProcess = childProcess.spawnSync(process.argv0, [path.join(__dirname, 'processLauncherCleanupEntrypoint.js'), ...options.tempDirectories]);
const [stdout, stderr] = [cleanupProcess.stdout.toString(), cleanupProcess.stderr.toString()];
if (stdout)
options.log(`[pid=${spawnedProcess.pid || 'N/A'}] ${stdout}`);
if (stderr)
options.log(`[pid=${spawnedProcess.pid || 'N/A'}] ${stderr}`);
for (const dir of options.tempDirectories) {
try {
rimraf.sync(dir);
} catch (e) {
options.log(`[pid=${spawnedProcess.pid || 'N/A'}] exception while removing ${dir}: ${e}`);
}
}
options.log(`[pid=${spawnedProcess.pid || 'N/A'}] finished temporary directories cleanup`);
}

View File

@ -1,28 +0,0 @@
/**
* 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.
*/
import { removeFolders } from './fileUtils';
(async () => {
const dirs = process.argv.slice(2);
const errors = await removeFolders(dirs);
for (let i = 0; i < dirs.length; ++i) {
if (errors[i]) {
// eslint-disable-next-line no-console
console.error(`exception while removing ${dirs[i]}: ${errors[i]}`);
}
}
})();

View File

@ -42,7 +42,9 @@ test('should close the browser when the node process closes', async ({ startRemo
expect(await remoteServer.childExitCode()).toBe(isWindows ? 1 : 0);
});
test('should remove temp dir on process.exit', async ({ startRemoteServer, server }, testInfo) => {
test('should remove temp dir on process.exit', async ({ startRemoteServer, server, platform }, testInfo) => {
test.skip(platform === 'win32', 'Removing user data dir synchronously is blocked on Windows');
const file = testInfo.outputPath('exit.file');
const remoteServer = await startRemoteServer('launchServer', { url: server.EMPTY_PAGE, exitOnFile: file });
const tempDir = await remoteServer.out('tempDir');