Skip to content

Commit

Permalink
fix(ui): wait until task output is fully rendered on stop (#8588)
Browse files Browse the repository at this point in the history
### Description

Previously we weren't `app.stop()` would exit before the TUI cleanup
completed. This resulted in a race condition between the TUI cleanup and
the run summary getting printed.

This PR changes the call to now wait until cleanup completes before
returning ensuring that the run summary is written only after the TUI
exits.

### Testing Instructions

Run a task a few times and you should see the run summary either not get
rendered *or* getting rendered before the terminal is back in cooked
mode.

Before:
<img width="1106" alt="Screenshot 2024-06-24 at 12 13 12 PM"
src="https://github.com/vercel/turbo/assets/4131117/6fc678c3-36a8-4526-bd22-289aa5f1ca94">
After:
<img width="759" alt="Screenshot 2024-06-24 at 12 09 20 PM"
src="https://github.com/vercel/turbo/assets/4131117/8cb93df6-3310-4047-b460-30ab10e40df2">
  • Loading branch information
chris-olszewski committed Jun 24, 2024
1 parent 7518fba commit 8fd9865
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 12 deletions.
28 changes: 20 additions & 8 deletions crates/turborepo-ui/src/tui/app.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::{
io::{self, Stdout, Write},
sync::mpsc,
time::{Duration, Instant},
};

Expand Down Expand Up @@ -120,9 +121,12 @@ pub fn run_app(tasks: Vec<String>, receiver: AppReceiver) -> Result<(), Error> {
let mut app: App<Box<dyn io::Write + Send>> =
App::new(size.height, full_task_width.max(ratio_pane_width), tasks);

let result = run_app_inner(&mut terminal, &mut app, receiver);
let (result, callback) = match run_app_inner(&mut terminal, &mut app, receiver) {
Ok(callback) => (Ok(()), callback),
Err(err) => (Err(err), None),
};

cleanup(terminal, app)?;
cleanup(terminal, app, callback)?;

result
}
Expand All @@ -133,12 +137,13 @@ fn run_app_inner<B: Backend + std::io::Write>(
terminal: &mut Terminal<B>,
app: &mut App<Box<dyn io::Write + Send>>,
receiver: AppReceiver,
) -> Result<(), Error> {
) -> Result<Option<mpsc::SyncSender<()>>, Error> {
// Render initial state to paint the screen
terminal.draw(|f| view(app, f))?;
let mut last_render = Instant::now();
let mut callback = None;
while let Some(event) = poll(app.input_options, &receiver, last_render + FRAMERATE) {
update(app, event)?;
callback = update(app, event)?;
if app.done {
break;
}
Expand All @@ -148,7 +153,7 @@ fn run_app_inner<B: Backend + std::io::Write>(
}
}

Ok(())
Ok(callback)
}

/// Blocking poll for events, will only return None if app handle has been
Expand All @@ -158,7 +163,7 @@ fn poll(input_options: InputOptions, receiver: &AppReceiver, deadline: Instant)
Ok(Some(event)) => Some(event),
Ok(None) => receiver.recv(deadline).ok(),
// Unable to read from stdin, shut down and attempt to clean up
Err(_) => Some(Event::Stop),
Err(_) => Some(Event::InternalStop),
}
}

Expand Down Expand Up @@ -198,6 +203,7 @@ fn startup() -> io::Result<Terminal<CrosstermBackend<Stdout>>> {
fn cleanup<B: Backend + io::Write, I>(
mut terminal: Terminal<B>,
mut app: App<I>,
callback: Option<mpsc::SyncSender<()>>,
) -> io::Result<()> {
terminal.clear()?;
crossterm::execute!(
Expand All @@ -209,13 +215,15 @@ fn cleanup<B: Backend + io::Write, I>(
app.pane.persist_tasks(&started_tasks)?;
crossterm::terminal::disable_raw_mode()?;
terminal.show_cursor()?;
// We can close the channel now that terminal is back restored to a normal state
drop(callback);
Ok(())
}

fn update(
app: &mut App<Box<dyn io::Write + Send>>,
event: Event,
) -> Result<Option<Vec<u8>>, Error> {
) -> Result<Option<mpsc::SyncSender<()>>, Error> {
match event {
Event::StartTask { task } => {
app.table.start_task(&task)?;
Expand All @@ -227,8 +235,12 @@ fn update(
Event::Status { task, status } => {
app.pane.set_status(&task, status)?;
}
Event::Stop => {
Event::InternalStop => {
app.done = true;
}
Event::Stop(callback) => {
app.done = true;
return Ok(Some(callback));
}
Event::Tick => {
app.table.tick();
Expand Down
4 changes: 3 additions & 1 deletion crates/turborepo-ui/src/tui/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ pub enum Event {
task: String,
status: String,
},
Stop,
Stop(std::sync::mpsc::SyncSender<()>),
// Stop initiated by the TUI itself
InternalStop,
Tick,
Up,
Down,
Expand Down
5 changes: 4 additions & 1 deletion crates/turborepo-ui/src/tui/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@ impl AppSender {

/// Stop rendering TUI and restore terminal to default configuration
pub fn stop(&self) {
let (callback_tx, callback_rx) = mpsc::sync_channel(1);
// Send stop event, if receiver has dropped ignore error as
// it'll be a no-op.
self.primary.send(Event::Stop).ok();
self.primary.send(Event::Stop(callback_tx)).ok();
// Wait for callback to be sent or the channel closed.
callback_rx.recv().ok();
}

/// Update the list of tasks displayed in the TUI
Expand Down
4 changes: 2 additions & 2 deletions crates/turborepo-ui/src/tui/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ fn ctrl_c() -> Option<Event> {
match signal::raise(signal::SIGINT) {
Ok(_) => None,
// We're unable to send the signal, stop rendering to force shutdown
Err(_) => Some(Event::Stop),
Err(_) => Some(Event::InternalStop),
}
}

Expand All @@ -101,7 +101,7 @@ fn ctrl_c() -> Option<Event> {
None
} else {
// We're unable to send the Ctrl-C event, stop rendering to force shutdown
Some(Event::Stop)
Some(Event::InternalStop)
}
}

Expand Down

0 comments on commit 8fd9865

Please sign in to comment.