From 1787147e82908a2e8c88fb333a62dfdb7a7f2e7f Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 5 May 2026 15:10:43 +0200 Subject: [PATCH] PTY: fix resource leaks and reader-isolate races (T-76) NativePty.close() now awaits the reader-isolate spawn, kills the child first to drive EOF on the master fd, awaits the isolate's EOF acknowledgement, and only then closes the fd. Previously the fd-close racing with the polling isolate left a window where the fd number could be reused and the isolate would briefly target the wrong file. Both NativePty and PtySession now surface reader-isolate spawn errors via the output stream's addError instead of silently swallowing them. PtySession.spawn closes the master fd on any post-receive failure, closes parentSock in finally (was leaking on every spawn), and kills the ptyc process if recvFd fails. PtySession._recvFdAsync uses try/finally to close the ReceivePort and kill the spawn isolate even when Isolate.spawn itself throws. Co-Authored-By: Claude --- .pql/pql-plan.json | 234 +++++++++++++++++++----------------- CHANGELOG.md | 9 ++ lib/src/pty/native_pty.dart | 59 ++++++++- lib/src/pty/session.dart | 81 +++++++++---- 4 files changed, 244 insertions(+), 139 deletions(-) diff --git a/.pql/pql-plan.json b/.pql/pql-plan.json index 295dc4ec..115db751 100644 --- a/.pql/pql-plan.json +++ b/.pql/pql-plan.json @@ -1,5 +1,5 @@ { - "exported_at": "2026-05-05T13:07:25Z", + "exported_at": "2026-05-05T13:10:43Z", "decisions": [ { "id": "D-1", @@ -9,7 +9,7 @@ "status": "active", "date": "2026-04-20", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-3", @@ -19,7 +19,7 @@ "status": "active", "date": "2026-04-20", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-4", @@ -29,7 +29,7 @@ "status": "active", "date": "2026-04-20", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-5", @@ -39,7 +39,7 @@ "status": "active", "date": "2026-04-20", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-6", @@ -49,7 +49,7 @@ "status": "active", "date": "2026-04-20", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-7", @@ -59,7 +59,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-8", @@ -69,7 +69,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-9", @@ -79,7 +79,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-10", @@ -89,7 +89,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-11", @@ -99,7 +99,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-12", @@ -109,7 +109,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-13", @@ -119,7 +119,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-14", @@ -129,7 +129,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-15", @@ -139,7 +139,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/extensions.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-16", @@ -149,7 +149,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/extensions.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-17", @@ -159,7 +159,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/extensions.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-18", @@ -169,7 +169,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/extensions.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-19", @@ -179,7 +179,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/extensions.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-20", @@ -189,7 +189,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/accessibility.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-21", @@ -199,7 +199,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/accessibility.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-22", @@ -209,7 +209,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/accessibility.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-23", @@ -219,7 +219,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/testing.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-24", @@ -229,7 +229,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/testing.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-25", @@ -239,7 +239,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/testing.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-26", @@ -249,7 +249,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/testing.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-27", @@ -259,7 +259,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/testing.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-28", @@ -269,7 +269,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/testing.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-29", @@ -279,7 +279,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/testing.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-30", @@ -289,7 +289,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/testing.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-31", @@ -299,7 +299,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/tooling.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-32", @@ -309,7 +309,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/tooling.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-33", @@ -319,7 +319,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/tooling.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-34", @@ -329,7 +329,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-35", @@ -339,7 +339,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-36", @@ -349,7 +349,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-37", @@ -359,7 +359,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-38", @@ -369,7 +369,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-39", @@ -379,7 +379,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-40", @@ -389,7 +389,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-41", @@ -399,7 +399,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-42", @@ -409,7 +409,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/tooling.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-43", @@ -419,7 +419,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-44", @@ -429,7 +429,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-45", @@ -439,7 +439,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-46", @@ -449,7 +449,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/extensions.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-47", @@ -459,7 +459,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-48", @@ -469,7 +469,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-49", @@ -479,7 +479,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-50", @@ -489,7 +489,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-51", @@ -499,7 +499,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-52", @@ -509,7 +509,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-53", @@ -519,7 +519,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-54", @@ -529,7 +529,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-55", @@ -539,7 +539,7 @@ "status": "active", "date": "2026-04-23", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-56", @@ -549,7 +549,7 @@ "status": "active", "date": "2026-04-23", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-57", @@ -559,7 +559,7 @@ "status": "active", "date": "2026-04-23", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-58", @@ -569,7 +569,7 @@ "status": "active", "date": "2026-04-23", "file_path": "decisions/tooling.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-59", @@ -579,7 +579,7 @@ "status": "active", "date": "2026-04-25", "file_path": "decisions/tooling.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-60", @@ -589,7 +589,7 @@ "status": "active", "date": "2026-04-26", "file_path": "decisions/tooling.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-61", @@ -599,7 +599,7 @@ "status": "active", "date": "2026-04-26", "file_path": "decisions/tooling.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-62", @@ -609,7 +609,7 @@ "status": "active", "date": "2026-04-26", "file_path": "decisions/tooling.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-63", @@ -619,7 +619,7 @@ "status": "active", "date": "2026-04-26", "file_path": "decisions/tooling.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-64", @@ -629,7 +629,7 @@ "status": "active", "date": "2026-05-03", "file_path": "decisions/architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "D-65", @@ -639,7 +639,7 @@ "status": "active", "date": "2026-04-26", "file_path": "decisions/tooling.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-1", @@ -648,7 +648,7 @@ "title": "Authorisation granularity on the IPC socket", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-2", @@ -657,7 +657,7 @@ "title": "Back-pressure on event streams", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-3", @@ -666,7 +666,7 @@ "title": "Event persistence + audit/undo", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-4", @@ -675,7 +675,7 @@ "title": "`.canvas` schema compatibility with Obsidian", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-5", @@ -684,7 +684,7 @@ "title": "IPC wire-format stability + `schema_version:`", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-6", @@ -693,7 +693,7 @@ "title": "Window chrome — native frame vs frameless custom", "status": "resolved", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-7", @@ -702,7 +702,7 @@ "title": "macOS app bundle signing / notarisation", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-8", @@ -711,7 +711,7 @@ "title": "Extension API shape — widgets, subcommands, both?", "status": "open", "file_path": "decisions/questions-extensions.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-9", @@ -720,7 +720,7 @@ "title": "Lua runtime vendoring", "status": "open", "file_path": "decisions/questions-extensions.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-10", @@ -729,7 +729,7 @@ "title": "Extension manifest `schema_version:`", "status": "open", "file_path": "decisions/questions-extensions.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-11", @@ -738,7 +738,7 @@ "title": "Coverage gates — hard thresholds vs soft reporting", "status": "open", "file_path": "decisions/questions-testing.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-12", @@ -747,7 +747,7 @@ "title": "Screen-reader automation (axe-core via Playwright)", "status": "open", "file_path": "decisions/questions-testing.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-13", @@ -756,7 +756,7 @@ "title": "Web production-mode a11y", "status": "open", "file_path": "decisions/questions-accessibility.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-14", @@ -765,7 +765,7 @@ "title": "i18n plurals / gender / date-format tooling", "status": "open", "file_path": "decisions/questions-accessibility.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-15", @@ -774,7 +774,7 @@ "title": "Editor tab — full LSP vs tree-sitter-only highlight", "status": "open", "file_path": "decisions/questions-process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-16", @@ -783,7 +783,7 @@ "title": "`tree-sitter-dart` grammar maintenance", "status": "open", "file_path": "decisions/questions-process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-17", @@ -792,7 +792,7 @@ "title": "Icon set growth", "status": "open", "file_path": "decisions/questions-process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-18", @@ -801,7 +801,7 @@ "title": "Theme hot-reload in release builds", "status": "open", "file_path": "decisions/questions-process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-19", @@ -810,7 +810,7 @@ "title": "(withdrawn)", "status": "resolved", "file_path": "decisions/questions-process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-20", @@ -819,7 +819,7 @@ "title": "Kernel DB service — namespaced SQL access?", "status": "open", "file_path": "decisions/questions-process.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-21", @@ -828,7 +828,7 @@ "title": "Pql absorbs planning vs keeps separate", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-22", @@ -837,7 +837,7 @@ "title": "Ticket persistence strategy", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-23", @@ -846,7 +846,7 @@ "title": "SSH-remote development — run clide against a remote workspace", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-25", @@ -855,7 +855,7 @@ "title": "Body text face — mono everywhere vs Josefin Sans UI + mono code", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-26", @@ -864,7 +864,7 @@ "title": "Small screen layout (\u003c 1000px)", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-27", @@ -873,7 +873,7 @@ "title": "Two-editor split", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-28", @@ -882,7 +882,7 @@ "title": "Terminal strip scope — shell only or logs/errors/tests", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-29", @@ -891,7 +891,7 @@ "title": "Branch picker location", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-30", @@ -900,7 +900,7 @@ "title": "Focus behavior when editor is dirty and viewer is peeked", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "Q-31", @@ -909,7 +909,7 @@ "title": "XWayland fallback for frameless — proper Wayland protocol needed", "status": "open", "file_path": "decisions/questions-architecture.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "R-2", @@ -919,7 +919,7 @@ "status": "active", "date": "2026-04-20", "file_path": "decisions/rejected.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "R-3", @@ -929,7 +929,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/rejected.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "R-4", @@ -939,7 +939,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/rejected.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "R-5", @@ -949,7 +949,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/rejected.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "R-6", @@ -959,7 +959,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/rejected.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "R-7", @@ -969,7 +969,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/rejected.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "R-8", @@ -979,7 +979,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/rejected.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "R-9", @@ -989,7 +989,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/rejected.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "R-10", @@ -999,7 +999,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/rejected.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "R-11", @@ -1009,7 +1009,7 @@ "status": "active", "date": "2026-04-21", "file_path": "decisions/rejected.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" }, { "id": "R-12", @@ -1019,7 +1019,7 @@ "status": "active", "date": "2026-04-22", "file_path": "decisions/rejected.md", - "synced_at": "2026-05-05 13:07:03" + "synced_at": "2026-05-05 13:10:16" } ], "decision_refs": [ @@ -2606,19 +2606,19 @@ "id": "T-75", "type": "task", "title": "PTY: surface errno from forkpty/execve/write/ioctl failures", - "status": "in_progress", + "status": "done", "priority": "high", "created_at": "2026-05-05 12:58:59", - "updated_at": "2026-05-05 13:02:40" + "updated_at": "2026-05-05 13:07:29" }, { "id": "T-76", "type": "task", "title": "PTY: fix resource leaks and reader-isolate races", - "status": "backlog", + "status": "in_progress", "priority": "high", "created_at": "2026-05-05 12:58:59", - "updated_at": "2026-05-05 12:58:59" + "updated_at": "2026-05-05 13:07:34" }, { "id": "T-77", @@ -3947,6 +3947,20 @@ "old_value": "backlog", "new_value": "in_progress", "changed_at": "2026-05-05 13:02:40" + }, + { + "ticket_id": "T-75", + "field": "status", + "old_value": "in_progress", + "new_value": "done", + "changed_at": "2026-05-05 13:07:29" + }, + { + "ticket_id": "T-76", + "field": "status", + "old_value": "backlog", + "new_value": "in_progress", + "changed_at": "2026-05-05 13:07:34" } ] } diff --git a/CHANGELOG.md b/CHANGELOG.md index afbe4397..cc021fb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,15 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. immediate EOF that looked indistinguishable from clean exit. PTY `write` loops on short writes; both `NativePty.write` and `PtySession.write` now throw `PtyException` on hard errors. +- PTY teardown order fixed — kill the child first so the master + fd returns EOF, await the reader isolate exit, then close the + fd. Previously closing the fd while the isolate still polled it + could briefly target a reused fd. Reader isolate spawn errors + in both `NativePty` and `PtySession` are now surfaced via the + output stream instead of silently dropped. `_recvFdAsync` no + longer leaks the `ReceivePort` when `Isolate.spawn` throws, and + `PtySession.spawn` closes the master fd if any post-receive + step fails. ### Security diff --git a/lib/src/pty/native_pty.dart b/lib/src/pty/native_pty.dart index d6e67b9a..3a3ba018 100644 --- a/lib/src/pty/native_pty.dart +++ b/lib/src/pty/native_pty.dart @@ -90,6 +90,13 @@ class NativePty { final _out = StreamController.broadcast(); bool _dead = false; + /// Tracks the reader isolate's spawn — close() awaits this before + /// tearing down so we never race a still-spawning isolate. + Future? _readerReady; + Isolate? _readerIsolate; + ReceivePort? _readerPort; + Completer? _readerExited; + NativePty._(this._fd, this.pid); /// Byte stream of data produced by the child. @@ -222,13 +229,32 @@ class NativePty { // -- I/O ------------------------------------------------------------------ - void _spawnReader() async { + void _spawnReader() { + _readerReady = _spawnReaderAsync(); + } + + Future _spawnReaderAsync() async { final rp = ReceivePort(); - await Isolate.spawn(_readLoop, (rp.sendPort, _fd)); + _readerPort = rp; + _readerExited = Completer(); + try { + _readerIsolate = await Isolate.spawn(_readLoop, (rp.sendPort, _fd)); + } catch (e) { + // Surface the spawn failure instead of leaving the PTY in a + // half-alive state where output never flows but isClosed=false. + _dead = true; + if (!_out.isClosed) _out.addError(PtyException('reader-spawn', '$e')); + rp.close(); + _readerPort = null; + if (!_readerExited!.isCompleted) _readerExited!.complete(); + return; + } rp.listen((msg) { if (msg == null) { if (!_out.isClosed) _out.close(); rp.close(); + _readerPort = null; + if (!_readerExited!.isCompleted) _readerExited!.complete(); _reap(); } else { if (!_out.isClosed) _out.add(msg as Uint8List); @@ -330,12 +356,39 @@ class NativePty { } /// Kill the child and release resources. + /// + /// Order matters: kill the child first so its slave PTY closes, + /// causing the master fd to return EOF. The reader isolate sees + /// EOF and exits cleanly. Only then do we close the master fd — + /// closing it before the isolate exits creates a window where the + /// fd number could be reused and the isolate would briefly poll + /// the wrong file. Future close() async { if (_dead) return; _dead = true; - _nativeClose(_fd); + + // Make sure the reader is fully spawned before we tear it down — + // otherwise close() racing with start() leaves an orphan isolate. + await _readerReady; + _nativeKill(pid, _kSighup); _nativeKill(pid, 9); + + // Wait for the isolate to send `null` (EOF) — confirms it has + // exited its poll loop and won't touch the fd again. + if (_readerExited != null) { + await _readerExited!.future.timeout( + const Duration(milliseconds: 500), + onTimeout: () {}, + ); + } + + _nativeClose(_fd); + _readerIsolate?.kill(priority: Isolate.immediate); + _readerIsolate = null; + _readerPort?.close(); + _readerPort = null; + final s = calloc(); _waitpid(pid, s, 0); calloc.free(s); diff --git a/lib/src/pty/session.dart b/lib/src/pty/session.dart index f6792ecf..3b67006f 100644 --- a/lib/src/pty/session.dart +++ b/lib/src/pty/session.dart @@ -132,31 +132,44 @@ class PtySession { // Receive the master fd over the parent side of the socketpair. // recvFd blocks until ptyc sends — run in a child isolate so the // calling isolate's event loop stays responsive. - final masterFd = await _recvFdAsync(parentSock); - - // Apply initial winsize (ptyc already did this, but doing it - // again from Dart confirms the wire + gives a place to call it - // when resize() lands). - libc.setWinsize(masterFd, cols, rows); - - // Drain ptyc's stdout to parse the success envelope. We don't - // strictly need it — the fd arriving is proof-of-life — but - // draining avoids a PIPE accumulating. - final stdoutLine = await proc.stdout.transform(const Utf8Decoder()).transform(const LineSplitter()).first.timeout(const Duration(seconds: 5)); - final pid = _extractPid(stdoutLine); - - final code = await proc.exitCode; - if (code != 0) { - final stderr = await proc.stderr.transform(const Utf8Decoder()).join(); - libc.close(masterFd); - throw PtyException('ptyc', 'ptyc exited with code $code: $stderr'); + final int masterFd; + try { + masterFd = await _recvFdAsync(parentSock); + } catch (_) { + proc.kill(); + rethrow; } - return PtySession._(pid: pid, masterFd: masterFd); + // Once we own masterFd, every error path below must close it + // before rethrowing. Wrap the rest of the spawn in its own + // try/catch so the cleanup is centralized. + try { + libc.setWinsize(masterFd, cols, rows); + + final stdoutLine = await proc.stdout + .transform(const Utf8Decoder()) + .transform(const LineSplitter()) + .first + .timeout(const Duration(seconds: 5)); + final pid = _extractPid(stdoutLine); + + final code = await proc.exitCode; + if (code != 0) { + final stderr = await proc.stderr.transform(const Utf8Decoder()).join(); + libc.close(masterFd); + throw PtyException('ptyc', 'ptyc exited with code $code: $stderr'); + } + + return PtySession._(pid: pid, masterFd: masterFd); + } catch (_) { + libc.close(masterFd); + rethrow; + } } finally { // parent keeps its own fd until the session is closed; ptyc-side // fd is released either way (ptyc has exited by now). if (childSock >= 0) libc.close(childSock); + if (parentSock >= 0) libc.close(parentSock); pkg_ffi.calloc.free(sv); } } @@ -254,12 +267,16 @@ class PtySession { /// stall the calling isolate's event loop. static Future _recvFdAsync(int socketFd) async { final port = ReceivePort(); - final iso = await Isolate.spawn(_recvFdEntry, _RecvFdArgs(socketFd, port.sendPort)); - final result = await port.first; - iso.kill(priority: Isolate.immediate); - port.close(); - if (result is int) return result; - throw PtyException('recvFd', '$result'); + Isolate? iso; + try { + iso = await Isolate.spawn(_recvFdEntry, _RecvFdArgs(socketFd, port.sendPort)); + final result = await port.first; + if (result is int) return result; + throw PtyException('recvFd', '$result'); + } finally { + iso?.kill(priority: Isolate.immediate); + port.close(); + } } static void _recvFdEntry(_RecvFdArgs args) { @@ -288,7 +305,19 @@ class PtySession { Isolate.spawn<_ReaderArgs>( _readerEntrypoint, _ReaderArgs(fd: _masterFd, sendPort: port.sendPort), - ).then((iso) => _readerIsolate = iso); + ).then( + (iso) => _readerIsolate = iso, + onError: (Object e) { + // Spawn failure leaves the session unable to ever produce + // output. Surface the error and mark the controller closed + // so consumers don't hang waiting on the stream. + if (!_outputCtrl.isClosed) { + _outputCtrl.addError(PtyException('reader-spawn', '$e')); + _outputCtrl.close(); + } + if (!_readerExited.isCompleted) _readerExited.complete(); + }, + ); } // -- request builder ------------------------------------------------------