mirror of
https://github.com/microsoft/playwright.git
synced 2024-10-27 05:46:28 +03:00
fix(launcher): unregister global process handlers when all browser are closed (#29011)
Otherwise, we forever block SIGTERM and SIGHUP by registering a handler that does not do anything (due to no browsers to close) and prevents default handler that exits from running. Fixes #28091.
This commit is contained in:
parent
775ef30e43
commit
9b657b54fb
@ -121,6 +121,13 @@ function addProcessHandlerIfNeeded(name: 'exit' | 'SIGINT' | 'SIGTERM' | 'SIGHUP
|
||||
process.on(name, processHandlers[name]);
|
||||
}
|
||||
}
|
||||
function removeProcessHandlersIfNeeded() {
|
||||
if (killSet.size)
|
||||
return;
|
||||
for (const handler of installedHandlers)
|
||||
process.off(handler, processHandlers[handler]);
|
||||
installedHandlers.clear();
|
||||
}
|
||||
|
||||
export async function launchProcess(options: LaunchProcessOptions): Promise<LaunchResult> {
|
||||
const stdio: ('ignore' | 'pipe')[] = options.stdio === 'pipe' ? ['ignore', 'pipe', 'pipe', 'pipe', 'pipe'] : ['pipe', 'pipe', 'pipe'];
|
||||
@ -178,6 +185,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
|
||||
processClosed = true;
|
||||
gracefullyCloseSet.delete(gracefullyClose);
|
||||
killSet.delete(killProcessAndCleanup);
|
||||
removeProcessHandlersIfNeeded();
|
||||
options.onExit(exitCode, signal);
|
||||
// Cleanup as process exits.
|
||||
cleanup().then(fulfillCleanup);
|
||||
@ -216,6 +224,7 @@ export async function launchProcess(options: LaunchProcessOptions): Promise<Laun
|
||||
function killProcess() {
|
||||
gracefullyCloseSet.delete(gracefullyClose);
|
||||
killSet.delete(killProcessAndCleanup);
|
||||
removeProcessHandlersIfNeeded();
|
||||
options.log(`[pid=${spawnedProcess.pid}] <kill>`);
|
||||
if (spawnedProcess.pid && !spawnedProcess.killed && !processClosed) {
|
||||
options.log(`[pid=${spawnedProcess.pid}] <will force kill>`);
|
||||
|
@ -2,7 +2,7 @@ const fs = require('fs');
|
||||
const cluster = require('cluster');
|
||||
|
||||
async function start() {
|
||||
const { browserTypeName, launchOptions, stallOnClose, disconnectOnSIGHUP, exitOnFile, exitOnWarning } = JSON.parse(process.argv[2]);
|
||||
const { browserTypeName, launchOptions, stallOnClose, disconnectOnSIGHUP, exitOnFile, exitOnWarning, startStopAndRunHttp } = JSON.parse(process.argv[2]);
|
||||
if (stallOnClose) {
|
||||
launchOptions.__testHookGracefullyClose = () => {
|
||||
console.log(`(stalled=>true)`);
|
||||
@ -11,11 +11,20 @@ async function start() {
|
||||
}
|
||||
if (exitOnWarning)
|
||||
process.on('warning', () => process.exit(43));
|
||||
if (disconnectOnSIGHUP)
|
||||
launchOptions.handleSIGHUP = false;
|
||||
|
||||
const playwright = require('playwright-core');
|
||||
|
||||
if (disconnectOnSIGHUP)
|
||||
launchOptions.handleSIGHUP = false;
|
||||
if (startStopAndRunHttp) {
|
||||
const browser = await playwright[browserTypeName].launch(launchOptions);
|
||||
await browser.close();
|
||||
console.log(`(wsEndpoint=>none)`);
|
||||
console.log(`(closed=>success)`);
|
||||
require('http').createServer(() => {}).listen();
|
||||
return;
|
||||
}
|
||||
|
||||
const browserServer = await playwright[browserTypeName].launchServer(launchOptions);
|
||||
if (disconnectOnSIGHUP)
|
||||
process.on('SIGHUP', () => browserServer._disconnectForTest());
|
||||
|
@ -68,6 +68,7 @@ export type RemoteServerOptions = {
|
||||
exitOnWarning?: boolean;
|
||||
inCluster?: boolean;
|
||||
url?: string;
|
||||
startStopAndRunHttp?: boolean;
|
||||
};
|
||||
|
||||
export class RemoteServer implements PlaywrightServer {
|
||||
@ -150,6 +151,10 @@ export class RemoteServer implements PlaywrightServer {
|
||||
return await this._process.exitCode;
|
||||
}
|
||||
|
||||
async childSignal() {
|
||||
return (await this._process.exited).signal;
|
||||
}
|
||||
|
||||
async close() {
|
||||
if (this._browser) {
|
||||
await this._browser.close();
|
||||
|
@ -133,4 +133,12 @@ test.describe('signals', () => {
|
||||
expect(await remoteServer.out('signal')).toBe('SIGKILL');
|
||||
expect(await remoteServer.childExitCode()).toBe(130);
|
||||
});
|
||||
|
||||
test('should not prevent default SIGTERM handling after browser close', async ({ startRemoteServer, server, platform }, testInfo) => {
|
||||
const remoteServer = await startRemoteServer('launchServer', { startStopAndRunHttp: true });
|
||||
expect(await remoteServer.out('closed')).toBe('success');
|
||||
process.kill(remoteServer.child().pid, 'SIGTERM');
|
||||
expect(await remoteServer.childExitCode()).toBe(null);
|
||||
expect(await remoteServer.childSignal()).toBe('SIGTERM');
|
||||
});
|
||||
});
|
||||
|
Loading…
Reference in New Issue
Block a user