From 466383671d2894f78c3bdcecc5f9cd9fcd1758ae Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Fri, 12 Jun 2026 00:18:47 +0200 Subject: [PATCH] close the PTY master fd when the child exits naturally (T-360) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _reap() flipped _dead without releasing the master fd, and close() short-circuits on _dead — so every naturally-exited child leaked its fd and pty device for the life of the app. The reader isolate sends EOF only after leaving its poll loop, so releasing the fd inside _reap() cannot race the reader. Regression test counts /dev/ptmx entries in /proc/self/fd across a natural exit; verified to fail against the unfixed code. Co-Authored-By: Claude Fable 5 --- .pql/changelog/ticket_history/2026-06.sql | 2 ++ .pql/changelog/tickets/2026-06.sql | 6 ++++ CHANGELOG.md | 4 +++ lib/src/pty/native_pty.dart | 5 ++++ test/pty/session_test.dart | 36 +++++++++++++++++++++++ 5 files changed, 53 insertions(+) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index accf3837..c26e7afd 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -3937,3 +3937,5 @@ Bug this split must fix (verified): _consumeCsi DISCARDS intermediate bytes — Related bug with its own ticket (T-369): unguarded params[i+1] lookahead in SGR 38/48 at lines ~502/512/547/557 (RangeError on truncated sequences) + colon-form sub-parameters unhandled. The split makes the fix natural: sgr_handler.dart owns guarded lookahead helpers; if T-369 lands first, carry its tests over; if this lands first, fix it inside sgr_handler.dart and close T-369 with it. Tests: test/terminal/escape/parser_test.dart (786 LOC) splits along the same seams — keep parser_test.dart for top-level dispatch/SBC/rollback, add csi_parser_test.dart (intermediates capture, DECSCUSR), sgr_handler_test.dart (bounds + colon form + 256/RGB), mode_handler_test.dart, osc_parser_test.dart, window/DA splits as convenient. The _RecordingHandler fixture is reusable across all of them.', NULL, '2026-06-11 22:10:19', '2026-06-11 22:10:19', '2026-06-11 22:10:19', NULL, '2942deccaff5b534ac3e33ac23b4d9fa', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBJ5T7HAQ9CA8XQMX43A2C', 'status', 'backlog', 'in_progress', NULL, '2026-06-11 22:14:48', '2026-06-11 22:14:48', '2026-06-11 22:14:48', NULL, '6f607da089f1326c07f4c17a6153c014', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBJ5T7HAQ9CA8XQMX43A2C', 'status', 'in_progress', 'done', NULL, '2026-06-11 22:18:25', '2026-06-11 22:18:25', '2026-06-11 22:18:25', NULL, '307a819cf82cf2ed49b3fd9e192992b6', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index ff5bf31a..d2c673aa 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -3973,3 +3973,9 @@ Bug this split must fix (verified): _consumeCsi DISCARDS intermediate bytes — Related bug with its own ticket (T-369): unguarded params[i+1] lookahead in SGR 38/48 at lines ~502/512/547/557 (RangeError on truncated sequences) + colon-form sub-parameters unhandled. The split makes the fix natural: sgr_handler.dart owns guarded lookahead helpers; if T-369 lands first, carry its tests over; if this lands first, fix it inside sgr_handler.dart and close T-369 with it. Tests: test/terminal/escape/parser_test.dart (786 LOC) splits along the same seams — keep parser_test.dart for top-level dispatch/SBC/rollback, add csi_parser_test.dart (intermediates capture, DECSCUSR), sgr_handler_test.dart (bounds + colon form + 256/RGB), mode_handler_test.dart, osc_parser_test.dart, window/DA splits as convenient. The _RecordingHandler fixture is reusable across all of them.', 'backlog', 'low', NULL, NULL, NULL, '2026-05-18 10:29:02', '2026-06-11 22:10:19', NULL, '90450ed5bd162fe65bf0426662d65ee4', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBJ5T7HAQ9CA8XQMX43A2C', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'PTY master fd leaks on every natural child exit', 'lib/src/pty/native_pty.dart:444-450 — on child EOF, _reap() sets _dead = true but never closes _fd; a later close() short-circuits at `if (_dead) return;` (line ~460) so _nativeClose(_fd) (line ~477) never runs. Every terminal/Claude pane whose child exits on its own leaks an fd and a pty device for the life of the app. Two independent reviewers confirmed. + +Fix: close the master fd in the natural-exit path (or let close() proceed to fd teardown when dead). Add a test asserting the fd is released after child EOF.', 'in_progress', 'critical', NULL, NULL, NULL, '2026-06-11 21:54:49', '2026-06-11 22:14:48', NULL, '3356972c53b2ba4079a3c86e7d836c09', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBHBJ5T7HAQ9CA8XQMX43A2C', 'bug', '06FBHBGHNEQTAEPGNJKN42C1E8', 'PTY master fd leaks on every natural child exit', 'lib/src/pty/native_pty.dart:444-450 — on child EOF, _reap() sets _dead = true but never closes _fd; a later close() short-circuits at `if (_dead) return;` (line ~460) so _nativeClose(_fd) (line ~477) never runs. Every terminal/Claude pane whose child exits on its own leaks an fd and a pty device for the life of the app. Two independent reviewers confirmed. + +Fix: close the master fd in the natural-exit path (or let close() proceed to fd teardown when dead). Add a test asserting the fd is released after child EOF.', 'done', 'critical', NULL, NULL, NULL, '2026-06-11 21:54:49', '2026-06-11 22:18:25', NULL, '62168ec7a6bcdd154adc173c9cf9275f', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f7e1d23..86639603 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ### Fixed +- **PTY master fd no longer leaks when a child exits on its own.** Every + terminal or Claude pane whose process ended naturally left its pty device + open for the life of the app; natural exit now releases the fd. (T-360) + - **Accepting ExitPlanMode now leaves plan mode in the conversation panel.** Approving Claude's plan (the ExitPlanMode tool) transitioned the underlying session out of plan mode, but clide's tracked permission mode didn't follow, diff --git a/lib/src/pty/native_pty.dart b/lib/src/pty/native_pty.dart index 27dc1108..70d74bdf 100644 --- a/lib/src/pty/native_pty.dart +++ b/lib/src/pty/native_pty.dart @@ -444,6 +444,11 @@ class NativePty { void _reap() { if (_dead) return; _dead = true; + // The reader isolate sends EOF only after exiting its poll loop, so + // nothing touches the master fd anymore. Release it here — close() + // short-circuits on _dead, so skipping this leaks the fd and its pty + // device for the life of the app on every natural child exit (T-360). + _nativeClose(_fd); final s = calloc(); _waitpid(pid, s, _kWnohang); calloc.free(s); diff --git a/test/pty/session_test.dart b/test/pty/session_test.dart index e47a0a59..d78aabdd 100644 --- a/test/pty/session_test.dart +++ b/test/pty/session_test.dart @@ -154,6 +154,42 @@ void main() { expect(s.pid, greaterThan(0)); }); + test('master fd is released after natural child exit', tags: ['pty'], () async { + // Linux-only: counts open fds resolving to /dev/ptmx via /proc. + // A naturally-exited child must not leave the master fd open — + // _reap() owns the release because close() short-circuits on + // _dead (T-360). + if (!Platform.isLinux) return; + + int ptmxCount() => Directory('/proc/self/fd').listSync().where((e) { + try { + return Link(e.path).targetSync() == '/dev/ptmx'; + } on FileSystemException { + return false; // fd vanished between list and readlink + } + }).length; + + final baseline = ptmxCount(); + final s = NativePty.start( + executable: '/bin/sh', + arguments: ['-c', 'exit 0'], + columns: 80, + rows: 24, + workingDirectory: '/', + environment: {...Platform.environment, 'TERM': 'xterm-256color'}, + ); + addTearDown(s.close); + + final done = Completer(); + s.output.listen((_) {}, onDone: () => done.complete()); + await done.future.timeout(ioTimeout, onTimeout: () => fail('output stream did not close within ${ioTimeout.inSeconds}s after child exit')); + + // EOF closes the output stream from the same listener callback + // that runs _reap(), so the fd is already released here. + expect(s.isClosed, isTrue); + expect(ptmxCount(), baseline); + }); + test('resize on a live PTY does not throw', () async { final s = NativePty.start( executable: '/bin/sh',