shell(cat): restart the shared stdin reader for a later cat, notify each listener once - #37752
Open
robobun wants to merge 5 commits into
Open
shell(cat): restart the shared stdin reader for a later cat, notify each listener once#37752robobun wants to merge 5 commits into
robobun wants to merge 5 commits into
Claude / Claude Code Review
completed
Aug 12, 2026 in 23m 55s
Code review found 1 potential issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/runtime/shell/IOReader.rs:194-205 |
Stale SAFETY comment on reader() contradicts the fix's re-entry path |
Annotations
Check warning on line 205 in src/runtime/shell/IOReader.rs
claude / Claude Code Review
Stale SAFETY comment on reader() contradicts the fix's re-entry path
nit: the SAFETY comment on `reader()` (a few lines up, ~89-94) still says it "MUST NOT be invoked from within … `on_reader_done_cb`/`on_reader_error` … the read loop already holds a live `&mut ReaderImpl`". This fix now depends on exactly that re-entry (`on_reader_done_cb` → second cat's `start()` → `self.reader()` → `r.start(fd, true)`), and per `PipeReader.rs` it's sound — done/error are tail-dispatched through a copied vtable with no borrow of `*this` live. Worth narrowing the comment so a re
Loading