Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
### Added
- activity indicator: tabs with output while not focused now show the activity dot in the tab bar; the marker clears as soon as the tab is focused

### Documentation
- document the per-pane lock hierarchy (`log_file` > `grid` > `pending_resize`) and the never-hold-two-at-once invariant in `drain.rs`

### Fixed
- scroll the tab bar to keep the active tab visible when many tabs are open; previously tabs overflowed the window width and the header rendered garbled
- avoid a ghost pane and a crash when a PTY spawn fails: a failed split/tab/restore no longer wires the layout or focus to a non-existent pane, and closing a pane in that state no longer panics
Expand Down
34 changes: 34 additions & 0 deletions src/drain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,40 @@ use crate::terminal::grid::Grid;

use super::App;

// ── Lock hierarchy (per pane) ─────────────────────────────────────────────────
//
// Each `PaneEntry` carries three locks shared between the parser thread (this
// module) and the main thread (`renderer/render_ops.rs`, `pane_ops.rs`):
//
// * `grid` — `Arc<RwLock<Grid>>` (cell grid + scrollback)
// * `log_file` — `Arc<Mutex<Option<File>>>` (raw-byte session log)
// * `pending_resize` — `Arc<Mutex<Option<(cols, rows)>>>` (main → parser signal)
//
// PRIMARY INVARIANT — never hold more than one of these at a time.
// Every site acquires one lock, does the minimum work, and releases it before
// acquiring the next. This is the rule that actually prevents deadlock and it
// holds everywhere today:
// - parser thread (below): `log_file` is taken and dropped (~:130), then
// `pending_resize.take()` is taken and dropped (~:137), and only then is the
// single `grid.write()` held (~:143). The resize is applied *inside* that
// grid write lock so the main thread never needs `grid.write()` — it only
// sets `pending_resize` and returns.
// - `render_ops::redraw` reads `grid` in a scoped block, drops it, then checks
// `log_file` separately.
// - `pane_ops::sync_pane_sizes_tab` reads `grid` in a scoped block, drops it,
// then sets `pending_resize`.
//
// SECONDARY RULE — should future code ever be forced to hold two at once,
// acquire them outermost-to-innermost in this order, and never acquire an outer
// lock while already holding an inner one:
//
// log_file > grid > pending_resize
//
// The `log_file > grid` half is load-bearing: the parser reaches `log_file`
// before `grid`, so the main thread must never hold `grid` while waiting on
// `log_file` (it would stall both threads — see the note in
// `renderer/render_ops.rs::redraw`).

#[cfg(test)]
#[path = "drain_test.rs"]
mod tests;
Expand Down
3 changes: 3 additions & 0 deletions src/pane_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,9 @@ impl App {
let (cols, rows) = entry
.metrics
.grid_size_for(w.saturating_sub(pad2), h.saturating_sub(pad2));
// Lock hierarchy (see drain.rs): read `grid` in this scoped block
// and drop it before taking `pending_resize` below — the two are
// never held at once.
let (grid_cols, grid_rows) = {
let g = entry.pane.grid.read().unwrap();
(g.cols, g.rows)
Expand Down