From a4e78605bc1c6c0372311308d6e59957b59f2d7c Mon Sep 17 00:00:00 2001 From: Haoxiang Fei Date: Mon, 15 Jun 2026 10:54:10 +0800 Subject: [PATCH 1/4] feat(tty): serialize terminal output writes with a lock Add an internal `lock` package wrapping a binary semaphore (Semaphore(1)) and route Tty::write / Tty::write_string through it, so a single write lands on the terminal atomically even when multiple tasks write concurrently. with_lock pairs acquire/release via defer, releasing on both the normal and failure paths. Upgrade moonbitlang/async 0.19.1 -> 0.19.4 for the semaphore API. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/lock/lock.mbt | 50 ++++++++++++++++++++++++++++++++ internal/lock/lock_test.mbt | 44 ++++++++++++++++++++++++++++ internal/lock/moon.pkg | 7 +++++ internal/lock/pkg.generated.mbti | 19 ++++++++++++ moon.mod | 2 +- moon.pkg | 1 + tty.mbt | 22 ++++++++++++-- 7 files changed, 142 insertions(+), 3 deletions(-) create mode 100644 internal/lock/lock.mbt create mode 100644 internal/lock/lock_test.mbt create mode 100644 internal/lock/moon.pkg create mode 100644 internal/lock/pkg.generated.mbti diff --git a/internal/lock/lock.mbt b/internal/lock/lock.mbt new file mode 100644 index 0000000..7913cd7 --- /dev/null +++ b/internal/lock/lock.mbt @@ -0,0 +1,50 @@ +///| +/// A mutual-exclusion lock. +/// +/// `Lock` wraps a binary semaphore: at most one task can hold it at a time. +/// Other tasks calling `acquire` block until the holder calls `release`, and +/// waiters are served first-come-first-serve. Prefer `with_lock`, which pairs +/// the acquire/release for you and releases even when the action fails. +struct Lock(@async/semaphore.Semaphore) + +///| +/// Create a new, unlocked `Lock`. +pub fn Lock::new() -> Lock { + Lock(Semaphore(1)) +} + +///| +/// Acquire the lock, blocking until it is available. +/// +/// As a blocking point, `acquire` may be cancelled, in which case the lock is +/// not held and the cancellation is raised. Every successful `acquire` must be +/// matched by exactly one `release`. +pub async fn Lock::acquire(self : Lock) -> Unit { + self.0.acquire() +} + +///| +/// Try to acquire the lock without blocking. +/// +/// Returns `true` and takes the lock if it was free, otherwise returns `false` +/// immediately. A successful acquisition must be matched by one `release`. +pub fn Lock::try_acquire(self : Lock) -> Bool { + self.0.try_acquire() +} + +///| +/// Release the lock, waking the next waiter if any. +pub fn Lock::release(self : Lock) -> Unit { + self.0.release() +} + +///| +/// Run `action` while holding the lock, returning its result. +/// +/// The lock is released whether `action` returns normally or fails. If +/// `acquire` is cancelled before the lock is taken, `action` does not run. +pub async fn[X] Lock::with_lock(self : Lock, action : async () -> X) -> X { + self.acquire() + defer self.release() + action() +} diff --git a/internal/lock/lock_test.mbt b/internal/lock/lock_test.mbt new file mode 100644 index 0000000..45c2129 --- /dev/null +++ b/internal/lock/lock_test.mbt @@ -0,0 +1,44 @@ +///| +/// A held lock keeps concurrent critical sections from overlapping, even when +/// the holder suspends. +async test "with_lock serializes concurrent critical sections" { + let lock = @lock.Lock::new() + let mut in_critical = false + let mut overlaps = 0 + let order = [] + @async.with_task_group(group => { + for id in 0..<5 { + group.spawn_bg(() => { + lock.with_lock(() => { + if in_critical { + overlaps += 1 + } + in_critical = true + // Suspend while holding the lock; a missing lock would let another task + // enter here and trip the `overlaps` counter. + @async.sleep(1) + order.push(id) + in_critical = false + }) + }) + } + }) + assert_eq(overlaps, 0) + assert_eq(order.length(), 5) +} + +///| +/// `with_lock` releases the lock even when the action fails. +async test "with_lock releases on failure" { + let lock = @lock.Lock::new() + let failed = try { + lock.with_lock(() => fail("boom")) + false + } catch { + _ => true + } + assert_eq(failed, true) + // The lock must be free again, so a follow-up acquire succeeds immediately. + assert_eq(lock.try_acquire(), true) + lock.release() +} diff --git a/internal/lock/moon.pkg b/internal/lock/moon.pkg new file mode 100644 index 0000000..ed1091f --- /dev/null +++ b/internal/lock/moon.pkg @@ -0,0 +1,7 @@ +import { + "moonbitlang/async/semaphore" @async/semaphore, +} + +import { + "moonbitlang/async", +} for "test" diff --git a/internal/lock/pkg.generated.mbti b/internal/lock/pkg.generated.mbti new file mode 100644 index 0000000..39c98ad --- /dev/null +++ b/internal/lock/pkg.generated.mbti @@ -0,0 +1,19 @@ +// Generated using `moon info`, DON'T EDIT IT +package "moonbit-community/tty/internal/lock" + +// Values + +// Errors + +// Types and methods +type Lock +pub async fn Lock::acquire(Self) -> Unit +pub fn Lock::new() -> Self +pub fn Lock::release(Self) -> Unit +pub fn Lock::try_acquire(Self) -> Bool +pub async fn[X] Lock::with_lock(Self, async () -> X) -> X + +// Type aliases + +// Traits + diff --git a/moon.mod b/moon.mod index d453eb5..3dab2d3 100644 --- a/moon.mod +++ b/moon.mod @@ -3,7 +3,7 @@ name = "moonbit-community/tty" version = "0.2.5" import { - "moonbitlang/async@0.19.1", + "moonbitlang/async@0.19.4", } readme = "README.md" diff --git a/moon.pkg b/moon.pkg index acdc000..340a859 100644 --- a/moon.pkg +++ b/moon.pkg @@ -10,6 +10,7 @@ import { "moonbit-community/tty/color", "moonbit-community/tty/input" @public_input, "moonbit-community/tty/internal/io" @internal_io, + "moonbit-community/tty/internal/lock", "moonbit-community/tty/internal/input", "moonbit-community/tty/internal/win32", "moonbit-community/tty/internal/vt", diff --git a/tty.mbt b/tty.mbt index cec6449..e150eb0 100644 --- a/tty.mbt +++ b/tty.mbt @@ -26,6 +26,9 @@ struct Tty { reader : @input.EventReader input_backend : WindowsInput pending_input : Array[@public_input.InputEvent] + // Serializes output writes so a single `write`/`write_string` call lands on + // the terminal atomically with respect to other concurrent writers. + write_lock : @lock.Lock } ///| @@ -35,6 +38,9 @@ struct Tty { output : &Writer reader : @input.EventReader pending_input : Array[@public_input.InputEvent] + // Serializes output writes so a single `write`/`write_string` call lands on + // the terminal atomically with respect to other concurrent writers. + write_lock : @lock.Lock } ///| @@ -50,6 +56,7 @@ pub fn[I : Reader, O : Writer] Tty::new(input : I, output : O) -> Tty { reader: input_backend.event_reader(input as &@async/io.Reader), input_backend, pending_input: [], + write_lock: @lock.Lock::new(), } } @@ -64,6 +71,7 @@ pub fn[I : Reader, O : Writer] Tty::new(input : I, output : O) -> Tty { output, reader: @input.EventReader::new(input as &@async/io.Reader), pending_input: [], + write_lock: @lock.Lock::new(), } } @@ -132,14 +140,24 @@ async fn Tty::read_internal_event( ///| /// Write bytes to the terminal output handle. +/// +/// The write is serialized through an internal lock, so a single call lands on +/// the terminal atomically even when multiple tasks write concurrently. To make +/// a multi-call sequence atomic, assemble it into one buffer and issue a single +/// `write`. pub async fn Tty::write(self : Self, data : &@async/io.Data) -> Unit { - self.output.write(data) + self.write_lock.with_lock(() => self.output.write(data)) } ///| /// Write a string to the terminal output handle. +/// +/// The write is serialized through an internal lock, so a single call lands on +/// the terminal atomically even when multiple tasks write concurrently. To make +/// a multi-call sequence atomic, assemble it into one string and issue a single +/// `write_string`. pub async fn Tty::write_string(self : Self, string : String) -> Unit { - self.output.write(string) + self.write_lock.with_lock(() => self.output.write(string)) } ///| From 27706946f6bf76e04951953438e19354ab3ac4b6 Mon Sep 17 00:00:00 2001 From: Haoxiang Fei Date: Mon, 15 Jun 2026 10:58:36 +0800 Subject: [PATCH 2/4] fix(tty): add write_lock to Windows white-box Tty literal The win32 console white-box test builds a Tty record literal directly, which must include the new write_lock field. Fixes the windows-latest CI check. Co-Authored-By: Claude Opus 4.8 (1M context) --- win32_input_wbtest.mbt | 1 + 1 file changed, 1 insertion(+) diff --git a/win32_input_wbtest.mbt b/win32_input_wbtest.mbt index c846d8b..e400976 100644 --- a/win32_input_wbtest.mbt +++ b/win32_input_wbtest.mbt @@ -64,6 +64,7 @@ fn[I : Reader, O : Writer] win32_test_console_tty(input : I, output : O) -> Tty reader: input_backend.event_reader(input as &@async/io.Reader), input_backend, pending_input: [], + write_lock: @lock.Lock::new(), } } From 67ba977130f314ac6b180a939771694b643f95db Mon Sep 17 00:00:00 2001 From: Haoxiang Fei Date: Mon, 15 Jun 2026 11:03:07 +0800 Subject: [PATCH 3/4] fix(tty): keep moonbitlang/async at 0.19.1 The Semaphore API the write lock needs already exists in 0.19.1 with an identical signature, so the bump to 0.19.4 was unnecessary. 0.19.4 also changed Windows-affecting internals and regressed the Tty write path on windows-latest (Tty::open test timed out). Staying on 0.19.1 keeps the lock feature while matching the version the tests module already pins. Co-Authored-By: Claude Opus 4.8 (1M context) --- moon.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/moon.mod b/moon.mod index 3dab2d3..d453eb5 100644 --- a/moon.mod +++ b/moon.mod @@ -3,7 +3,7 @@ name = "moonbit-community/tty" version = "0.2.5" import { - "moonbitlang/async@0.19.4", + "moonbitlang/async@0.19.1", } readme = "README.md" From 9dc7503270a055de3763221cbe19b5b13872408d Mon Sep 17 00:00:00 2001 From: Haoxiang Fei Date: Mon, 15 Jun 2026 11:07:58 +0800 Subject: [PATCH 4/4] docs(plan): record output write lock task (TTY-23) Add docs/plans/2026-06-15-output-write-lock.md with goal, target files, public API changes, invariants, acceptance criteria, validation, and the public API audit notes required when .mbti files change, and add the TTY-23 row to the execution board. Addresses the plan-discipline review. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/plan.md | 1 + docs/plans/2026-06-15-output-write-lock.md | 143 +++++++++++++++++++++ 2 files changed, 144 insertions(+) create mode 100644 docs/plans/2026-06-15-output-write-lock.md diff --git a/docs/plan.md b/docs/plan.md index 4095df9..0f212e3 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -58,6 +58,7 @@ This board tracks implementation direction for `moonbit-community/tty`. Use | TTY-21 | done | Windows console input polling | `docs/plans/2026-06-08-windows-console-input-polling.md`, root package | Windows `Tty::read_event` reads console input records directly and reports resize events without public API changes | `moon fmt`, `moon check`, `moon test .`, `moon test internal/input`, `moon test`, `moon info`, `git diff --check`, Windows manual resize smoke pending | | TTY-22 | done | Windows input record enum | `docs/plans/2026-06-09-windows-input-record-enum.md`, root package | Windows `INPUT_RECORD` bytes parse into private typed variants, including native mouse records | `moon fmt`, targeted Windows wbtests, `moon test .`, `moon test`, `moon check`, `moon info`, `git diff --check` | | MVP-2 | done | queued input and shell commands in agent demo | `docs/plans/2026-05-21-agent-queued-shell.md`, `examples/agent` | `Tab` queues input for delayed submission and `!cmd` runs through `moonbitlang/async/process` without background tasks writing directly to tty | `moon fmt`, `moon check examples/agent`, `moon test examples/agent`, `moon check`, `moon test`, `moon info`, `git diff --check` | +| TTY-23 | done | serialize root `Tty` output writes | `docs/plans/2026-06-15-output-write-lock.md`, `internal/lock/`, root package | a single `Tty::write`/`Tty::write_string` call lands atomically under concurrent writers via an internal `Lock`, with no public API change | `moon fmt`, `moon test internal/lock`, `moon test .`, `moon test`, `moon check`, `moon info`, `git diff --check` | ## Current Rules diff --git a/docs/plans/2026-06-15-output-write-lock.md b/docs/plans/2026-06-15-output-write-lock.md new file mode 100644 index 0000000..73abfe2 --- /dev/null +++ b/docs/plans/2026-06-15-output-write-lock.md @@ -0,0 +1,143 @@ +# Output Write Lock + +## Goal + +Serialize `Tty` output writes so a single `Tty::write` / `Tty::write_string` +call lands on the terminal atomically with respect to other concurrent writers, +without the caller having to coordinate access. Keep the locking primitive +internal and the public `Tty` shape opaque. + +## Status + +Done. + +## Context And Decisions + +- `moonbitlang/async` is a single-threaded cooperative runtime, so there are no + data races, but a logical write can still interleave: `@async/io.Writer.write` + loops over `write_once` syscalls with suspension points between them, letting a + second writer splice its bytes into the first writer's escape sequences. +- A binary semaphore (`Semaphore(1)`) is the smallest primitive that makes a + single write critical section atomic. The `Semaphore` type already exists in + the pinned `moonbitlang/async@0.19.1`, so no dependency bump is required. +- Wrap the semaphore in a small internal `lock` package (`Lock`) instead of + exposing the async semaphore directly, so the locking contract is named and + documented in one place and the `with_lock` combinator owns acquire/release + pairing. +- `with_lock` releases via `defer`, registered only after `acquire` succeeds, so + a cancelled `acquire` never releases a lock it does not hold and a failing + action still releases. +- Locking granularity is per call. A method that issues several `write`s (for + example `enable_mouse`) is a sequence of atomic writes, not one atomic unit; + frame-level atomicity remains the caller's responsibility (assemble one buffer + per write). This is documented on `write` / `write_string`. +- A bump to `moonbitlang/async@0.19.4` was attempted and reverted: it regressed + the Windows `Tty` write path (`tests/open` `Tty::open` timed out on + windows-latest) and is unnecessary because `0.19.1` already provides the + `Semaphore` API used here. + +## References Or Standards + +- `moonbitlang/async/semaphore` `Semaphore` (binary semaphore as a mutex). + +## Target Files + +- `internal/lock/moon.pkg` +- `internal/lock/lock.mbt` +- `internal/lock/lock_test.mbt` +- `tty.mbt` +- `win32_input_wbtest.mbt` +- `moon.pkg` +- `docs/plan.md` +- generated `.mbti` files from `moon info` + +## Public API Changes + +New internal package `moonbit-community/tty/internal/lock`: + +- `Lock` (opaque struct) +- `Lock::new() -> Lock` +- `Lock::acquire(Self) -> Unit` (async) +- `Lock::try_acquire(Self) -> Bool` +- `Lock::release(Self) -> Unit` +- `Lock::with_lock[X](Self, async () -> X) -> X` (async) + +Root package: no public API change. `Tty` gains a private `write_lock : @lock.Lock` +field; `Tty::write` and `Tty::write_string` keep their existing signatures. + +No new public type, parser state, input event, platform backend, dependency, or +capability query is added. + +## Invariants + +- Every byte written to the terminal output stream goes through `Tty::write` or + `Tty::write_string`, both of which hold `write_lock` for the duration of the + underlying writer call. +- `fd()`-based operations (`size.mbt`, `state.mbt`) are independent atomic + syscalls and are intentionally not serialized by `write_lock`. +- `with_lock` releases the lock on both the normal and the failure path, and + never releases when `acquire` was cancelled before taking the lock. +- The `lock` package is internal and exposes no public mutable fields; `Lock` is + opaque. + +## Acceptance Criteria + +- Concurrent critical sections guarded by `with_lock` never overlap, even when + the holder suspends. +- `with_lock` releases the lock when its action fails. +- Root `.mbti` is unchanged (the new field is private). +- Generated `.mbti` for the `lock` package exposes only the intended methods and + no fields. + +## Validation Commands + +- `moon fmt` +- `moon test internal/lock` +- `moon test .` +- `moon test` +- `moon check` +- `moon info` +- review generated `.mbti` diff +- `git diff --check` + +## Public API Audit + +- New `internal/lock/pkg.generated.mbti` exposes `Lock` as an opaque struct + (`// private fields`) plus `new`, `acquire`, `try_acquire`, `release`, and + `with_lock`. No public fields are exposed. +- Root `pkg.generated.mbti` is unchanged: `write_lock` is a private field and + `Tty::write` / `Tty::write_string` signatures are unchanged. +- No public type, mutable field, parser state, input event, platform handle, + backend selection, dependency, or capability query was added. +- `moonbitlang/async` stays pinned at `0.19.1`; no dependency version change. +- Generated `.mbti` files were reviewed after `moon info`. + +## Result Notes + +- Added the internal `lock` package wrapping `Semaphore(1)` with a `with_lock` + combinator that pairs acquire/release through `defer`. +- Routed `Tty::write` / `Tty::write_string` through `write_lock.with_lock(...)` + and initialized `write_lock` in both the Windows and non-Windows constructors. +- Updated the Windows white-box `Tty` literal in `win32_input_wbtest.mbt` to set + the new field. +- Per-call atomicity only; multi-write command methods are not wrapped as a + single unit. Documented the build-one-buffer idiom for frame-level atomicity. + +## Validation Results + +- `moon fmt` passed. +- `moon test internal/lock` passed: 2 tests. +- `moon test` passed: 165 tests. +- `moon check` passed. +- `moon info` passed and regenerated the intended `.mbti` entries. +- CI passed on ubuntu-latest, macos-latest, and windows-latest. +- generated `.mbti` diff reviewed. +- `git diff --check` passed. + +## Open Questions + +- Whether selected multi-write command methods (for example mouse enable/disable) + should be wrapped in `with_lock` for group atomicity, or batched into a single + buffer. Deferred; current contract is per-call atomicity. +- Revisiting `moonbitlang/async@0.19.4`: it regresses the Windows `Tty` write + path and needs separate investigation before any future bump.