close the PTY master fd when the child exits naturally (T-360)
_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 <noreply@anthropic.com>
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<ffi.Int32>();
|
||||
_waitpid(pid, s, _kWnohang);
|
||||
calloc.free(s);
|
||||
|
||||
@@ -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<void>();
|
||||
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',
|
||||
|
||||
Reference in New Issue
Block a user