diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c6bef1..b91ca8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -97,6 +97,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 now prints only `KeyboardInterrupt`, as Python does. GemStone's own messages from a `gemdb` run are shown only when the program could not run to the end, such as a refused login, and a `.topazini` file is no longer read. +- **Closing the terminal ends a `gemdb` run.** Closing the terminal tab of `gemdb app.py`, or of + **Run Python File in GemDB**, left the program running out of sight, holding a database session + and, for a web app, its port. It is now stopped as Ctrl+C would stop it: its `finally` blocks run + and its session is given back. ## [1.6.0] - 2026-10-07 diff --git a/docs/grail.md b/docs/grail.md index cd6429a..8fd1429 100644 --- a/docs/grail.md +++ b/docs/grail.md @@ -85,6 +85,18 @@ commands from stdin, and a refused login with input piped in ran that input as topaz commands (measured, and covered by `cli.test.ts`). Under `-S` the same `exit` was ignored, and on a tty it printed four lines saying so. +**Topaz ignores SIGHUP, so the wrapper ends it.** Measured on 4.0.0.a4 by +closing a pty under topaz: the wrapper exits, but topaz carries on running +the program with its session, whether the program is quiet or keeps printing +to the dead terminal, and its `finally` blocks never run (#136). The +wrapper's `EXIT` trap, which bash runs on a hangup, therefore sends a topaz +still running SIGINT, which is what Ctrl-C sends. The driver turns that into +`KeyboardInterrupt`, the `finally` blocks run, and topaz logs out and exits, +within a quarter of a second in the measurement. A topaz still running after +ten seconds gets SIGTERM, which skips `finally`. The temporary files are +removed only after topaz has gone, because the driver writes the status file +as it unwinds and would otherwise recreate one the trap had just removed. + Errors in the driver are caught as `AbstractException`, not `Error`: Grail's Python exceptions live outside the `Error` branch, which is why grail.tpz's own file mode exits 0 on a Python diff --git a/src/__integration__/cli.test.ts b/src/__integration__/cli.test.ts index 06c247b..81eae7b 100644 --- a/src/__integration__/cli.test.ts +++ b/src/__integration__/cli.test.ts @@ -311,35 +311,53 @@ exit [lindex $result 3] expect(ran.code).toBe(1); }); - it('leaves no temporary files behind, even when its terminal closes mid-run', async () => { + it('ends the program, finally blocks and all, when its terminal closes', async () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gemdb-cli-tmp-')); const env = { ...process.env, TMPDIR: tmp }; + fs.rmSync(path.join(workDir, 'hung-up.txt'), { force: true }); fs.writeFileSync( - path.join(workDir, 'forever.py'), - 'import time\nprint("serving", flush=True)\nwhile True:\n time.sleep(0.2)\n', + path.join(workDir, 'quiet.py'), + 'import time\nprint("serving", flush=True)\ntry:\n while True:\n time.sleep(0.2)\n' + + 'finally:\n open("hung-up.txt", "w").write("finally ran")\n', ); await new Promise((resolve) => execFile('bash', [cliPath(), '-c', 'print(1)'], { cwd: workDir, env }, resolve), ); - // Its own process group, so the hangup reaches the wrapper and topaz - // together, as closing a terminal tab does. - const child = spawn('bash', [cliPath(), 'forever.py'], { cwd: workDir, env, detached: true }); - const exited = new Promise((resolve) => child.on('exit', resolve)); - await new Promise((resolve) => { - child.stdout.on('data', (chunk: Buffer) => { - if (chunk.toString().includes('serving')) resolve(); - }); - }); - - process.kill(-(child.pid ?? 0), 'SIGHUP'); - await exited; + // Closing expect's side of the pty is what closing a terminal tab does: + // the kernel hangs up the foreground process group, wrapper and topaz. + const scriptPath = path.join(workDir, 'hangup.exp'); + fs.writeFileSync( + scriptPath, + `set timeout 90 +spawn bash ${cliPath()} quiet.py +expect { + -ex "serving" {} + timeout { puts stderr "TIMEOUT waiting to serve"; exit 9 } +} +puts "WRAPPER=[exp_pid] TOPAZ=[exec pgrep -P [exp_pid] -x topaz]" +close +`, + ); + // On 'exit', not when its output closes: a topaz left running would hold + // expect's pipes open, and the test would time out instead of failing. + const expectRun = spawn('/usr/bin/expect', ['-f', scriptPath], { cwd: workDir, env }); + let transcript = ''; + expectRun.stdout.on('data', (chunk: Buffer) => (transcript += chunk.toString())); + await new Promise((resolve) => expectRun.on('exit', resolve)); + const topaz = Number(/TOPAZ=(\d+)/.exec(transcript)?.[1]); + const wrapper = Number(/WRAPPER=(\d+)/.exec(transcript)?.[1]); + + // topaz ignores SIGHUP, so this is the wrapper's doing: what Ctrl-C sends. + // The wrapper goes last, once it has seen topaz go and removed its files. + const ended = await waitForProcessToEnd(topaz, 15_000); + await waitForProcessToEnd(wrapper, 15_000); const left = fs.readdirSync(tmp); - // topaz outlives a hangup, as it did before the wrapper had a trap, so it - // is ended here: left running it holds a session the next test needs gone. - process.kill(-(child.pid ?? 0), 'SIGTERM'); - await waitForGroupToEnd(child.pid ?? 0); + if (!ended) process.kill(topaz, 'SIGTERM'); fs.rmSync(tmp, { recursive: true, force: true }); + expect(topaz).toBeGreaterThan(0); + expect(ended).toBe(true); + expect(fs.readFileSync(path.join(workDir, 'hung-up.txt'), 'utf8')).toBe('finally ran'); expect(left).toEqual([]); }); @@ -386,15 +404,15 @@ function canMakeFixture(): boolean { return probe !== undefined; } -/** Resolves once no process in the group is left, polling as `kill -0` does. */ -async function waitForGroupToEnd(pgid: number): Promise { - for (let i = 0; i < 100; i++) { +/** Whether `pid` has exited within `ms`, polling as `kill -0` does. */ +async function waitForProcessToEnd(pid: number, ms: number): Promise { + for (let waited = 0; waited < ms; waited += 100) { try { - process.kill(-pgid, 0); + process.kill(pid, 0); } catch { - return; + return true; } await new Promise((resolve) => setTimeout(resolve, 100)); } - throw new Error(`process group ${pgid} still running after 10 s`); + return false; } diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts index 99e8a13..df3bf30 100644 --- a/src/__tests__/cli.test.ts +++ b/src/__tests__/cli.test.ts @@ -123,16 +123,21 @@ describe('writeCliScripts', () => { expect(script).toMatch(/if \[ -z "\$STATUS" \]; then\n\s+sed .* "\$GEMDB_TOPAZ_LOG" >&2\n/); }); - it('removes its temporary files however the run ends', () => { + it('tidies up after the run however it ends, topaz included', () => { writeCliScripts(ext); const script = fs.readFileSync(cliPath(), 'utf8'); // Armed before the first file is made, so no exit path — a hangup // included — can skip it, and nothing after the run removes them itself. - expect(script.indexOf('trap remove_temp_files EXIT')).toBeGreaterThan(-1); - expect(script.indexOf('trap remove_temp_files EXIT')).toBeLessThan(script.indexOf('mktemp')); + expect(script.indexOf('trap end_run EXIT')).toBeGreaterThan(-1); + expect(script.indexOf('trap end_run EXIT')).toBeLessThan(script.indexOf('mktemp')); expect(script).not.toMatch(/^rm -f "\$GEMDB_STATUS_FILE"/m); + // topaz ignores SIGHUP, so a wrapper ending first sends it Ctrl-C — but + // only while topaz is running, so a finished run looks for nothing. + expect(script).toMatch(/TOPAZ_RUNNING=yes\ntopaz -L /); + expect(script).toMatch(/\nRC=\$\?\nTOPAZ_RUNNING=""\n/); + expect(script).toContain('kill -INT "$pid"'); }); it('never leaves topaz reading commands from stdin', () => { diff --git a/src/cli.ts b/src/cli.ts index 7f7da97..40bb8a9 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -624,21 +624,50 @@ fi fi fi -# The run's temporary files, removed however it ends: bash runs an EXIT trap -# on a normal exit, and on SIGHUP (a terminal tab closed mid-run) and SIGTERM -# too — measured with macOS's /bin/bash 3.2, the oldest this runs under. A -# function rather than an array, because 3.2 calls an empty array unbound -# under set -u. +# However the run ends, end_run tidies up after it: bash runs an EXIT trap on +# a normal exit, and on SIGHUP (a terminal tab closed mid-run) and SIGTERM +# too — measured with macOS's /bin/bash 3.2, the oldest this runs under. +# +# A hangup ends this script but not topaz, which ignores SIGHUP: closing the +# terminal left it running the program, holding a session (and a web app its +# port) with nobody to see it, and a program that kept printing was not ended +# by its writes failing either (#136). So if topaz is still running, it is sent +# what Ctrl-C sends — measured after closing the terminal, the program's +# finally blocks ran and topaz logged out and exited within a quarter of a +# second — and SIGTERM, which skips finally, if it has not gone in ten +# seconds. Without pgrep there is no topaz to find, and the run is left as it +# was before. The files go last, because the driver writes the status file as +# it unwinds. They are listed in a loop rather than kept in an array, because +# 3.2 calls an empty array unbound under set -u. CODE_FILE="" GEMDB_STATUS_FILE="" GEMDB_TOPAZ_LOG="" -remove_temp_files() { +TOPAZ_RUNNING="" +wait_for_exit() { + local tries=0 + while kill -0 "$1" 2>/dev/null && [ "$tries" -lt "$2" ]; do + sleep 0.1 + tries=$((tries + 1)) + done + ! kill -0 "$1" 2>/dev/null +} +stop_topaz() { + local pid + pid="$(pgrep -P $$ -x topaz 2>/dev/null | head -n 1)" + [ -n "$pid" ] || return 0 + kill -INT "$pid" 2>/dev/null + wait_for_exit "$pid" 100 && return 0 + kill -TERM "$pid" 2>/dev/null + wait_for_exit "$pid" 50 || true +} +end_run() { + if [ -n "$TOPAZ_RUNNING" ]; then stop_topaz; fi local f for f in "$CODE_FILE" "\${CODE_FILE%.py}" "$GEMDB_STATUS_FILE" "$GEMDB_TOPAZ_LOG"; do if [ -n "$f" ]; then rm -f "$f"; fi done } -trap remove_temp_files EXIT +trap end_run EXIT # -c 'code': materialise the code as a file, like CPython's -c but visibly so. # mktemp is given an explicit template — BSD and GNU disagree about -t. @@ -667,8 +696,10 @@ fi GEMDB_STATUS_FILE="$(mktemp "\${TMPDIR:-/tmp}/gemdb-status-XXXXXX")" GEMDB_TOPAZ_LOG="$(mktemp "\${TMPDIR:-/tmp}/gemdb-topaz-XXXXXX")" export GEMDB_STATUS_FILE GEMDB_TOPAZ_LOG +TOPAZ_RUNNING=yes topaz -L -q -I "$ROOT/bin/gemdb-run-init.tpz" -P "$ROOT/bin/gemdb-run.tpz" ${TOPAZ_TUNING} -- "$@" RC=$? +TOPAZ_RUNNING="" STATUS="$(cat "$GEMDB_STATUS_FILE" 2>/dev/null || true)" if [ -z "$STATUS" ]; then sed -e '/^topaz[^>]*> run$/,/^%$/d' -e '/^topaz[^>]*> /d' "$GEMDB_TOPAZ_LOG" >&2