From 4c8912f74eac25b03e5ff16a888d77ea88c2d8a6 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Mon, 24 Jul 2023 15:24:29 -0700
Subject: [PATCH] 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.
---
 .../src/utils/processLauncher.ts              | 15 +++++-----
 .../utils/processLauncherCleanupEntrypoint.ts | 28 -------------------
 tests/library/signals.spec.ts                 |  4 ++-
 3 files changed, 10 insertions(+), 37 deletions(-)
 delete mode 100644 packages/playwright-core/src/utils/processLauncherCleanupEntrypoint.ts

diff --git a/packages/playwright-core/src/utils/processLauncher.ts b/packages/playwright-core/src/utils/processLauncher.ts
index 184241f8f0..f4222744a7 100644
--- a/packages/playwright-core/src/utils/processLauncher.ts
+++ b/packages/playwright-core/src/utils/processLauncher.ts
@@ -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`);
   }
diff --git a/packages/playwright-core/src/utils/processLauncherCleanupEntrypoint.ts b/packages/playwright-core/src/utils/processLauncherCleanupEntrypoint.ts
deleted file mode 100644
index e11c67f158..0000000000
--- a/packages/playwright-core/src/utils/processLauncherCleanupEntrypoint.ts
+++ /dev/null
@@ -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]}`);
-    }
-  }
-})();
diff --git a/tests/library/signals.spec.ts b/tests/library/signals.spec.ts
index 50b34c15e5..85f6d45ca6 100644
--- a/tests/library/signals.spec.ts
+++ b/tests/library/signals.spec.ts
@@ -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');