Skip to content

Commit

Permalink
fix(webserver): do not spawn webserver as a new process group (#18564)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aslushnikov authored Nov 11, 2022
1 parent 2841b0b commit 26044c1
Showing 1 changed file with 33 additions and 22 deletions.
55 changes: 33 additions & 22 deletions packages/playwright-test/src/plugins/webServerPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 '.';
Expand All @@ -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;
Expand All @@ -64,21 +65,34 @@ 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;
}
}

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`);
Expand All @@ -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`);
Expand All @@ -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)
Expand Down

0 comments on commit 26044c1

Please sign in to comment.