diff --git a/.github/workflows/build-and-make.yaml b/.github/workflows/build-and-make.yaml index 4b2e724db..dafffd24e 100644 --- a/.github/workflows/build-and-make.yaml +++ b/.github/workflows/build-and-make.yaml @@ -1150,6 +1150,16 @@ jobs: electron-backend-e2e:packaged-frame-copy-smoke \ --skip-nx-cache + - name: Upload packaged frame-copy smoke diagnostics + if: always() && matrix.os == 'linux' && matrix.linux_profile == 'portable' + uses: actions/upload-artifact@v7 + with: + name: packaged-frame-copy-smoke + path: | + dist/playwright-report/electron-backend-e2e/packaged-frame-copy-smoke/ + dist/test-results/electron-backend-e2e/packaged-frame-copy-smoke/ + retention-days: 7 + - name: Diagnose packaged x64 frame-copy hardware path if: matrix.os == 'linux' && matrix.linux_profile == 'portable' continue-on-error: true diff --git a/.github/workflows/e2e-tests.yaml b/.github/workflows/e2e-tests.yaml index 5879a477f..866208985 100644 --- a/.github/workflows/e2e-tests.yaml +++ b/.github/workflows/e2e-tests.yaml @@ -68,6 +68,9 @@ jobs: - name: Build Backend run: pnpm nx build electron-backend + - name: Verify Electron process cleanup + run: pnpm exec tsx --test apps/electron-backend-e2e/src/performance/electron-process-termination.spec.ts + - name: Install Playwright Browsers run: pnpm exec playwright install --with-deps diff --git a/apps/electron-backend-e2e/src/electron-process-termination.ts b/apps/electron-backend-e2e/src/electron-process-termination.ts new file mode 100644 index 000000000..9f239eb25 --- /dev/null +++ b/apps/electron-backend-e2e/src/electron-process-termination.ts @@ -0,0 +1,25 @@ +import { execFileSync, type ChildProcess } from 'node:child_process'; + +type ElectronProcess = Pick; + +export function terminateElectronProcess( + child: ElectronProcess, + signal: NodeJS.Signals = 'SIGTERM', + platform: NodeJS.Platform = process.platform, + run: typeof execFileSync = execFileSync +): void { + // Playwright launches Electron through cmd.exe on Windows. Killing only + // that ChildProcess leaves Electron (and its profile lock) alive. + if (platform === 'win32') { + if (!Number.isSafeInteger(child.pid) || (child.pid ?? 0) <= 0) { + throw new Error('electron-process-pid-unavailable'); + } + run('taskkill.exe', ['/pid', String(child.pid), '/T', '/F'], { + stdio: 'pipe', + timeout: 5000, + windowsHide: true, + }); + return; + } + child.kill(signal); +} diff --git a/apps/electron-backend-e2e/src/electron-test-fixtures.ts b/apps/electron-backend-e2e/src/electron-test-fixtures.ts index 3702752cd..6fb3abc1e 100644 --- a/apps/electron-backend-e2e/src/electron-test-fixtures.ts +++ b/apps/electron-backend-e2e/src/electron-test-fixtures.ts @@ -31,6 +31,7 @@ import { closeElectronApplicationAndConfirmExit, prepareElectronApplication, } from './electron-process-lifecycle'; +import { terminateElectronProcess } from './electron-process-termination'; export const workspaceRoot = resolve(__dirname, '../../..'); export const electronMainPath = join( @@ -581,7 +582,7 @@ export async function closeElectronApp( const childProcess = app.electronApp.process(); if (!childProcess.killed) { - childProcess.kill(); + terminateElectronProcess(childProcess); } await waitForPromiseWithTimeout( @@ -601,7 +602,7 @@ export async function closeElectronApp( console.warn( 'Electron app survived SIGTERM; escalating to SIGKILL' ); - childProcess.kill('SIGKILL'); + terminateElectronProcess(childProcess, 'SIGKILL'); await waitForPromiseWithTimeout( closePromise.catch(() => undefined), electronAppKillWaitMs diff --git a/apps/electron-backend-e2e/src/performance/electron-process-termination.spec.ts b/apps/electron-backend-e2e/src/performance/electron-process-termination.spec.ts new file mode 100644 index 000000000..ddb3cd95d --- /dev/null +++ b/apps/electron-backend-e2e/src/performance/electron-process-termination.spec.ts @@ -0,0 +1,112 @@ +import assert from 'node:assert/strict'; +import { spawn, type execFileSync } from 'node:child_process'; +import { once } from 'node:events'; +import { describe, it } from 'node:test'; +import { terminateElectronProcess } from '../electron-process-termination'; + +describe('Electron forced termination', () => { + it('terminates the Windows shell and its Electron descendants together', () => { + let shellAlive = true; + let electronAlive = true; + const child = { + pid: 1234, + kill: () => { + shellAlive = false; + return true; + }, + }; + const run = ((file, args, options) => { + assert.equal(file, 'taskkill.exe'); + assert.deepEqual(args, ['/pid', '1234', '/T', '/F']); + assert.equal(options?.timeout, 5000); + shellAlive = false; + electronAlive = false; + return Buffer.alloc(0); + }) as typeof execFileSync; + + terminateElectronProcess(child, 'SIGTERM', 'win32', run); + + assert.equal(shellAlive, false); + assert.equal( + electronAlive, + false, + 'Electron must release the profile before relaunch' + ); + }); + + it('preserves signal escalation on Unix', () => { + const signals: NodeJS.Signals[] = []; + const child = { + pid: 1234, + kill: (signal: NodeJS.Signals) => { + signals.push(signal); + return true; + }, + }; + terminateElectronProcess(child, 'SIGTERM', 'linux'); + terminateElectronProcess(child, 'SIGKILL', 'darwin'); + assert.deepEqual(signals, ['SIGTERM', 'SIGKILL']); + }); + + it('does not fall back to killing only the shell when tree termination fails', () => { + const failure = new Error('taskkill failed'); + const child = { + pid: 1234, + kill: () => assert.fail('would orphan the Electron descendant'), + }; + const run = (() => { + throw failure; + }) as typeof execFileSync; + assert.throws( + () => terminateElectronProcess(child, 'SIGTERM', 'win32', run), + (error) => error === failure + ); + }); + + it( + 'reaps a real Windows shell child instead of orphaning its descendant', + { + skip: process.platform !== 'win32', + timeout: 15000, + }, + async () => { + // Match Playwright's Windows launch topology: cmd.exe -> application. + const shell = spawn( + `"${process.execPath}"`, + [ + '-e', + '"setInterval(() => {}, 1000); console.log(process.pid)"', + ], + { shell: true, stdio: ['ignore', 'pipe', 'pipe'] } + ); + try { + const [output] = await once(shell.stdout, 'data', { + signal: AbortSignal.timeout(5000), + }); + const descendantPid = Number(String(output).trim()); + assert.ok( + Number.isSafeInteger(descendantPid) && descendantPid > 0 + ); + assert.notEqual(descendantPid, shell.pid); + process.kill(descendantPid, 0); + + terminateElectronProcess(shell); + await assert.rejects( + async () => process.kill(descendantPid, 0), + { + code: 'ESRCH', + } + ); + } finally { + if (shell.exitCode === null && shell.signalCode === null) { + // The successful taskkill can precede Node's exit event. + try { + terminateElectronProcess(shell); + } catch { + /* already exited */ + } + } + } + } + ); +}); diff --git a/docs/development/electron-debugging.md b/docs/development/electron-debugging.md index bf3a282a7..611ee15d1 100644 --- a/docs/development/electron-debugging.md +++ b/docs/development/electron-debugging.md @@ -67,6 +67,19 @@ agent-browser connect ws://127.0.0.1:9222/devtools/page/ agent-browser screenshot /tmp/iptvnator-cdp.png ``` +## E2E process cleanup and packaged diagnostics + +Playwright launches Electron through `cmd.exe` on Windows. If graceful E2E +shutdown times out, terminate that process tree with `taskkill /T /F`; killing +only `electronApp.process()` can leave Electron holding the temporary profile +and make the next launch exit on the single-instance lock. The E2E workflow +checks this against a real Windows shell and child process. Test cleanup must +target only its own launch PID, never all Electron or IPTVnator processes. + +The Linux portable build uploads `packaged-frame-copy-smoke` reports and traces +even when the smoke fails. Check the paused-frame screenshot and trace before +classifying a zero rendered-frame signal as an infrastructure flake. + ## Main-process ownership The entry point is `apps/electron-backend/src/main.ts`; it bootstraps the database,