Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add back pressure to pty #536

Merged
merged 2 commits into from
May 28, 2021
Merged

Add back pressure to pty #536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ccf3d6
fix: use bounded queue between pty and screen
kxt May 25, 2021
ee4e619
fix(tests): Add SLEEP to top_and_quit test
kunalmohan May 28, 2021
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
6 changes: 5 additions & 1 deletion src/tests/integration/close_pane.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::CliArgs;
use crate::tests::utils::commands::{
CLOSE_PANE_IN_PANE_MODE, ESC, MOVE_FOCUS_IN_PANE_MODE, PANE_MODE, QUIT,
RESIZE_DOWN_IN_RESIZE_MODE, RESIZE_LEFT_IN_RESIZE_MODE, RESIZE_MODE, RESIZE_UP_IN_RESIZE_MODE,
SPLIT_DOWN_IN_PANE_MODE, SPLIT_RIGHT_IN_PANE_MODE,
SLEEP, SPLIT_DOWN_IN_PANE_MODE, SPLIT_RIGHT_IN_PANE_MODE,
};
use zellij_utils::input::config::Config;

Expand Down Expand Up @@ -404,6 +404,7 @@ pub fn close_pane_with_multiple_panes_above_it_away_from_screen_edges() {
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_RIGHT_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down Expand Up @@ -464,6 +465,7 @@ pub fn close_pane_with_multiple_panes_below_it_away_from_screen_edges() {
&PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_RIGHT_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down Expand Up @@ -530,6 +532,7 @@ pub fn close_pane_with_multiple_panes_to_the_left_away_from_screen_edges() {
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down Expand Up @@ -592,6 +595,7 @@ pub fn close_pane_with_multiple_panes_to_the_right_away_from_screen_edges() {
&PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down
4 changes: 2 additions & 2 deletions src/tests/integration/compatibility.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::tests::utils::{get_next_to_last_snapshot, get_output_frame_snapshots}
use crate::CliArgs;
use zellij_utils::pane_size::PositionAndSize;

use crate::tests::utils::commands::QUIT;
use crate::tests::utils::commands::{QUIT, SLEEP};
use zellij_utils::input::config::Config;

/*
Expand Down Expand Up @@ -607,7 +607,7 @@ pub fn top_and_quit() {
};
let fixture_name = "top_and_quit";
let mut fake_input_output = get_fake_os_input(&fake_win_size, fixture_name);
fake_input_output.add_terminal_input(&[&QUIT]);
fake_input_output.add_terminal_input(&[&SLEEP, &QUIT]);
start(
Box::new(fake_input_output.clone()),
CliArgs::default(),
Expand Down
5 changes: 5 additions & 0 deletions src/tests/integration/resize_down.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@ pub fn resize_down_with_panes_above_aligned_left_and_right_with_current_pane() {
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&RESIZE_MODE,
&RESIZE_DOWN_IN_RESIZE_MODE,
Expand Down Expand Up @@ -491,6 +492,7 @@ pub fn resize_down_with_panes_below_aligned_left_and_right_with_current_pane() {
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down Expand Up @@ -547,10 +549,12 @@ pub fn resize_down_with_panes_above_aligned_left_and_right_with_panes_to_the_lef
&RESIZE_LEFT_IN_RESIZE_MODE,
&RESIZE_LEFT_IN_RESIZE_MODE,
&PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down Expand Up @@ -619,6 +623,7 @@ pub fn resize_down_with_panes_below_aligned_left_and_right_with_to_the_left_and_
&RESIZE_LEFT_IN_RESIZE_MODE,
&RESIZE_LEFT_IN_RESIZE_MODE,
&PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down
3 changes: 3 additions & 0 deletions src/tests/integration/resize_left.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ pub fn resize_left_with_panes_to_the_left_aligned_top_and_bottom_with_current_pa
&SPLIT_RIGHT_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_RIGHT_IN_PANE_MODE,
&RESIZE_MODE,
&RESIZE_LEFT_IN_RESIZE_MODE,
Expand Down Expand Up @@ -469,6 +470,7 @@ pub fn resize_left_with_panes_to_the_right_aligned_top_and_bottom_with_current_p
&SPLIT_RIGHT_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_RIGHT_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down Expand Up @@ -529,6 +531,7 @@ pub fn resize_left_with_panes_to_the_left_aligned_top_and_bottom_with_panes_abov
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_RIGHT_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down
3 changes: 3 additions & 0 deletions src/tests/integration/resize_right.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ pub fn resize_right_with_panes_to_the_left_aligned_top_and_bottom_with_current_p
&SPLIT_RIGHT_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_RIGHT_IN_PANE_MODE,
&RESIZE_MODE,
&RESIZE_RIGHT_IN_RESIZE_MODE,
Expand Down Expand Up @@ -469,6 +470,7 @@ pub fn resize_right_with_panes_to_the_right_aligned_top_and_bottom_with_current_
&SPLIT_RIGHT_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_RIGHT_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down Expand Up @@ -529,6 +531,7 @@ pub fn resize_right_with_panes_to_the_left_aligned_top_and_bottom_with_panes_abo
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_RIGHT_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down
5 changes: 5 additions & 0 deletions src/tests/integration/resize_up.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ pub fn resize_up_with_panes_above_aligned_left_and_right_with_current_pane() {
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&RESIZE_MODE,
&RESIZE_UP_IN_RESIZE_MODE,
Expand Down Expand Up @@ -485,6 +486,7 @@ pub fn resize_up_with_panes_below_aligned_left_and_right_with_current_pane() {
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down Expand Up @@ -541,10 +543,12 @@ pub fn resize_up_with_panes_above_aligned_left_and_right_with_panes_to_the_left_
&RESIZE_LEFT_IN_RESIZE_MODE,
&RESIZE_LEFT_IN_RESIZE_MODE,
&PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down Expand Up @@ -613,6 +617,7 @@ pub fn resize_up_with_panes_below_aligned_left_and_right_with_to_the_left_and_ri
&RESIZE_LEFT_IN_RESIZE_MODE,
&RESIZE_LEFT_IN_RESIZE_MODE,
&PANE_MODE,
&SLEEP,
&SPLIT_DOWN_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
&MOVE_FOCUS_IN_PANE_MODE,
Expand Down
9 changes: 7 additions & 2 deletions zellij-server/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,11 @@ fn init_session(
) -> SessionMetaData {
let (to_screen, screen_receiver): ChannelWithContext<ScreenInstruction> = channels::unbounded();
let to_screen = SenderWithContext::new(to_screen);

let (to_screen_bounded, bounded_screen_receiver): ChannelWithContext<ScreenInstruction> =
channels::bounded(50);
let to_screen_bounded = SenderWithContext::new(to_screen_bounded);

let (to_plugin, plugin_receiver): ChannelWithContext<PluginInstruction> = channels::unbounded();
let to_plugin = SenderWithContext::new(to_plugin);
let (to_pty, pty_receiver): ChannelWithContext<PtyInstruction> = channels::unbounded();
Expand Down Expand Up @@ -334,7 +339,7 @@ fn init_session(
let pty = Pty::new(
Bus::new(
vec![pty_receiver],
Some(&to_screen),
Some(&to_screen_bounded),
None,
Some(&to_plugin),
Some(&to_server),
Expand All @@ -351,7 +356,7 @@ fn init_session(
.name("screen".to_string())
.spawn({
let screen_bus = Bus::new(
vec![screen_receiver],
vec![screen_receiver, bounded_screen_receiver],
None,
Some(&to_pty),
Some(&to_plugin),
Expand Down
26 changes: 15 additions & 11 deletions zellij-server/src/pty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ async fn deadline_read(
}
}

async fn async_send_to_screen(senders: ThreadSenders, screen_instruction: ScreenInstruction) {
task::spawn_blocking(move || senders.send_to_screen(screen_instruction))
.await
.unwrap()
}

fn stream_terminal_bytes(
pid: RawFd,
senders: ThreadSenders,
Expand All @@ -171,36 +177,34 @@ fn stream_terminal_bytes(
match deadline_read(async_reader.as_mut(), render_deadline, &mut buf).await {
ReadResult::Ok(0) | ReadResult::Err(_) => break, // EOF or error
ReadResult::Timeout => {
let _ = senders.send_to_screen(ScreenInstruction::Render);
async_send_to_screen(senders.clone(), ScreenInstruction::Render).await;
// next read does not need a deadline as we just rendered everything
render_deadline = None;

// yield so Screen thread has some time to render before send additional
// PtyBytes.
task::sleep(Duration::from_millis(10)).await;
}
ReadResult::Ok(n_bytes) => {
let bytes = &buf[..n_bytes];
if debug {
let _ = debug_to_file(bytes, pid);
}
let _ = senders
.send_to_screen(ScreenInstruction::PtyBytes(pid, bytes.to_vec()));
async_send_to_screen(
senders.clone(),
ScreenInstruction::PtyBytes(pid, bytes.to_vec()),
)
.await;
// if we already have a render_deadline we keep it, otherwise we set it
// to the duration of `render_pause`.
render_deadline.get_or_insert(Instant::now() + render_pause);
}
}
}
let _ = senders.send_to_screen(ScreenInstruction::Render);
async_send_to_screen(senders.clone(), ScreenInstruction::Render).await;

#[cfg(not(any(feature = "test", test)))]
// this is a little hacky, and is because the tests end the file as soon as
// we read everything, rather than hanging until there is new data
// a better solution would be to fix the test fakes, but this will do for now
senders
.send_to_screen(ScreenInstruction::ClosePane(PaneId::Terminal(pid)))
.unwrap();
async_send_to_screen(senders, ScreenInstruction::ClosePane(PaneId::Terminal(pid)))
.await;
}
})
}
Expand Down