fix(webserver): do not spawn webserver as a new process group (#18564)

This patch stops using `processLauncher` to launch web servers. Process
Launcher
will spawn a new process group which is separate from test runner.

This might result in unexpected behavior, e.g. `kill -sigkill
-<testrunnerpid>` will orphan web server process.

Instead, this patch simply spawns web server and sends `SIGTERM` to it
when ready.
This commit is contained in:
Andrey Lushnikov 2022-11-10 22:30:06 -08:00 committed by GitHub
parent 2841b0b3dc
commit 26044c11c0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -17,10 +17,11 @@ import http from 'http';
import https from 'https';
import path from 'path';
import net from 'net';
import { spawn, spawnSync } from 'child_process';
import type { ChildProcess } from 'child_process';
import { debug } from 'playwright-core/lib/utilsBundle';
import { raceAgainstTimeout } from 'playwright-core/lib/utils/timeoutRunner';
import { launchProcess } from 'playwright-core/lib/utils/processLauncher';
import type { FullConfig, Reporter, Suite } from '../../types/testReporter';
import type { TestRunnerPlugin } from '.';
@ -46,8 +47,8 @@ const debugWebServer = debug('pw:webserver');
export class WebServerPlugin implements TestRunnerPlugin {
private _isAvailable?: () => Promise<boolean>;
private _killProcess?: () => Promise<void>;
private _processExitedPromise!: Promise<any>;
private _processExitedPromise?: Promise<{ code: number|null, signal: NodeJS.Signals|null }>;
private _childProcess?: ChildProcess;
private _options: WebServerPluginOptions;
private _checkPortOnly: boolean;
private _reporter?: Reporter;
@ -64,7 +65,6 @@ export class WebServerPlugin implements TestRunnerPlugin {
this._options.cwd = this._options.cwd ? path.resolve(configDir, this._options.cwd) : configDir;
try {
await this._startProcess();
await this._waitForProcess();
} catch (error) {
await this.teardown();
throw error;
@ -72,13 +72,27 @@ export class WebServerPlugin implements TestRunnerPlugin {
}
public async teardown() {
await this._killProcess?.();
if (!this._childProcess || !this._childProcess.pid || this._childProcess.killed)
return;
// Send SIGTERM and wait for it to gracefully close.
try {
if (process.platform === 'win32') {
const taskkillProcess = spawnSync(`taskkill /pid ${this._childProcess.pid} /T /F`, { shell: true });
const [stdout, stderr] = [taskkillProcess.stdout.toString(), taskkillProcess.stderr.toString()];
if (stdout)
debugWebServer(`[pid=${this._childProcess.pid}] taskkill stdout: ${stdout}`);
if (stderr)
debugWebServer(`[pid=${this._childProcess.pid}] taskkill stderr: ${stderr}`);
} else {
process.kill(this._childProcess.pid, 'SIGTERM');
}
} catch (e) {
// the process might have already stopped
}
await this._processExitedPromise;
}
private async _startProcess(): Promise<void> {
let processExitedReject = (error: Error) => { };
this._processExitedPromise = new Promise((_, reject) => processExitedReject = reject);
const isAlreadyAvailable = await this._isAvailable!();
if (isAlreadyAvailable) {
debugWebServer(`WebServer is already available`);
@ -89,33 +103,28 @@ export class WebServerPlugin implements TestRunnerPlugin {
}
debugWebServer(`Starting WebServer process ${this._options.command}...`);
const { launchedProcess, kill } = await launchProcess({
command: this._options.command,
this._childProcess = spawn(this._options.command, [], {
env: {
...DEFAULT_ENVIRONMENT_VARIABLES,
...envWithoutExperimentalLoaderOptions(),
...this._options.env,
},
cwd: this._options.cwd,
stdio: 'stdin',
stdio: 'pipe',
shell: true,
attemptToGracefullyClose: async () => {},
log: () => {},
onExit: code => processExitedReject(new Error(code ? `Process from config.webServer was not able to start. Exit code: ${code}` : 'Process from config.webServer exited early.')),
tempDirectories: [],
});
this._killProcess = kill;
this._processExitedPromise = new Promise((resolve, reject) => {
this._childProcess!.once('exit', (code, signal) => resolve({ code, signal }));
});
debugWebServer(`Process started`);
launchedProcess.stderr!.on('data', line => this._reporter!.onStdErr?.('[WebServer] ' + line.toString()));
launchedProcess.stdout!.on('data', line => {
this._childProcess.stderr!.on('data', line => this._reporter!.onStdErr?.('[WebServer] ' + line.toString()));
this._childProcess.stdout!.on('data', line => {
if (debugWebServer.enabled)
this._reporter!.onStdOut?.('[WebServer] ' + line.toString());
});
}
private async _waitForProcess() {
debugWebServer(`Waiting for availability...`);
await this._waitForAvailability();
debugWebServer(`WebServer available`);
@ -126,7 +135,9 @@ export class WebServerPlugin implements TestRunnerPlugin {
const cancellationToken = { canceled: false };
const { timedOut } = (await Promise.race([
raceAgainstTimeout(() => waitFor(this._isAvailable!, cancellationToken), launchTimeout),
this._processExitedPromise,
this._processExitedPromise!.then(({ code, signal }) => {
throw new Error(`Process from config.webServer terminated with exit code "${code}" and signal "${signal}"`);
}),
]));
cancellationToken.canceled = true;
if (timedOut)