project: Spawn terminal process on background executor (#41216)

Lukas Wirth created

Attempt 2 for https://github.com/zed-industries/zed/pull/40774

We were spawning the process on the foreground thread before which can
block an arbitrary amount of time. Likewise we no longer block
deserialization on the terminal loading.

Release Notes:

- Improved startup time on systems with slow process spawning
capabilities

Change summary

crates/project/src/terminals.rs           | 356 ++++++++--------
crates/terminal/src/terminal.rs           | 541 ++++++++++++------------
crates/terminal_view/src/persistence.rs   | 102 ++--
crates/terminal_view/src/terminal_view.rs |  34 
4 files changed, 525 insertions(+), 508 deletions(-)

Detailed changes

crates/project/src/terminals.rs 🔗

@@ -139,142 +139,145 @@ impl Project {
             .await
             .unwrap_or_default();
 
-            project.update(cx, move |this, cx| {
-                let format_to_run = || {
-                    if let Some(command) = &spawn_task.command {
-                        let mut command: Option<Cow<str>> = shell_kind.try_quote(command);
-                        if let Some(command) = &mut command
-                            && command.starts_with('"')
-                            && let Some(prefix) = shell_kind.command_prefix()
-                        {
-                            *command = Cow::Owned(format!("{prefix}{command}"));
-                        }
+            let builder = project
+                .update(cx, move |_, cx| {
+                    let format_to_run = || {
+                        if let Some(command) = &spawn_task.command {
+                            let mut command: Option<Cow<str>> = shell_kind.try_quote(command);
+                            if let Some(command) = &mut command
+                                && command.starts_with('"')
+                                && let Some(prefix) = shell_kind.command_prefix()
+                            {
+                                *command = Cow::Owned(format!("{prefix}{command}"));
+                            }
 
-                        let args = spawn_task
-                            .args
-                            .iter()
-                            .filter_map(|arg| shell_kind.try_quote(&arg));
+                            let args = spawn_task
+                                .args
+                                .iter()
+                                .filter_map(|arg| shell_kind.try_quote(&arg));
 
-                        command.into_iter().chain(args).join(" ")
-                    } else {
-                        // todo: this breaks for remotes to windows
-                        format!("exec {shell} -l")
-                    }
-                };
-
-                let (shell, env) = {
-                    env.extend(spawn_task.env);
-                    match remote_client {
-                        Some(remote_client) => match activation_script.clone() {
-                            activation_script if !activation_script.is_empty() => {
-                                let separator = shell_kind.sequential_commands_separator();
-                                let activation_script =
-                                    activation_script.join(&format!("{separator} "));
-                                let to_run = format_to_run();
-                                let shell = remote_client
-                                    .read(cx)
-                                    .shell()
-                                    .unwrap_or_else(get_default_system_shell);
-                                let arg = format!("{activation_script}{separator} {to_run}");
-                                let args = shell_kind.args_for_shell(false, arg);
-
-                                create_remote_shell(
-                                    Some((&shell, &args)),
+                            command.into_iter().chain(args).join(" ")
+                        } else {
+                            // todo: this breaks for remotes to windows
+                            format!("exec {shell} -l")
+                        }
+                    };
+
+                    let (shell, env) = {
+                        env.extend(spawn_task.env);
+                        match remote_client {
+                            Some(remote_client) => match activation_script.clone() {
+                                activation_script if !activation_script.is_empty() => {
+                                    let separator = shell_kind.sequential_commands_separator();
+                                    let activation_script =
+                                        activation_script.join(&format!("{separator} "));
+                                    let to_run = format_to_run();
+                                    let shell = remote_client
+                                        .read(cx)
+                                        .shell()
+                                        .unwrap_or_else(get_default_system_shell);
+                                    let arg = format!("{activation_script}{separator} {to_run}");
+                                    let args = shell_kind.args_for_shell(false, arg);
+
+                                    create_remote_shell(
+                                        Some((&shell, &args)),
+                                        env,
+                                        path,
+                                        remote_client,
+                                        cx,
+                                    )?
+                                }
+                                _ => create_remote_shell(
+                                    spawn_task
+                                        .command
+                                        .as_ref()
+                                        .map(|command| (command, &spawn_task.args)),
                                     env,
                                     path,
                                     remote_client,
                                     cx,
-                                )?
-                            }
-                            _ => create_remote_shell(
-                                spawn_task
-                                    .command
-                                    .as_ref()
-                                    .map(|command| (command, &spawn_task.args)),
-                                env,
-                                path,
-                                remote_client,
-                                cx,
-                            )?,
-                        },
-                        None => match activation_script.clone() {
-                            activation_script if !activation_script.is_empty() => {
-                                let separator = shell_kind.sequential_commands_separator();
-                                let activation_script =
-                                    activation_script.join(&format!("{separator} "));
-                                let to_run = format_to_run();
-
-                                let mut arg = format!("{activation_script}{separator} {to_run}");
-                                if shell_kind == ShellKind::Cmd {
-                                    // We need to put the entire command in quotes since otherwise CMD tries to execute them
-                                    // as separate commands rather than chaining one after another.
-                                    arg = format!("\"{arg}\"");
-                                }
+                                )?,
+                            },
+                            None => match activation_script.clone() {
+                                activation_script if !activation_script.is_empty() => {
+                                    let separator = shell_kind.sequential_commands_separator();
+                                    let activation_script =
+                                        activation_script.join(&format!("{separator} "));
+                                    let to_run = format_to_run();
+
+                                    let mut arg =
+                                        format!("{activation_script}{separator} {to_run}");
+                                    if shell_kind == ShellKind::Cmd {
+                                        // We need to put the entire command in quotes since otherwise CMD tries to execute them
+                                        // as separate commands rather than chaining one after another.
+                                        arg = format!("\"{arg}\"");
+                                    }
 
-                                let args = shell_kind.args_for_shell(false, arg);
+                                    let args = shell_kind.args_for_shell(false, arg);
 
-                                (
-                                    Shell::WithArguments {
-                                        program: shell,
-                                        args,
-                                        title_override: None,
+                                    (
+                                        Shell::WithArguments {
+                                            program: shell,
+                                            args,
+                                            title_override: None,
+                                        },
+                                        env,
+                                    )
+                                }
+                                _ => (
+                                    if let Some(program) = spawn_task.command {
+                                        Shell::WithArguments {
+                                            program,
+                                            args: spawn_task.args,
+                                            title_override: None,
+                                        }
+                                    } else {
+                                        Shell::System
                                     },
                                     env,
-                                )
-                            }
-                            _ => (
-                                if let Some(program) = spawn_task.command {
-                                    Shell::WithArguments {
-                                        program,
-                                        args: spawn_task.args,
-                                        title_override: None,
-                                    }
-                                } else {
-                                    Shell::System
-                                },
-                                env,
-                            ),
-                        },
-                    }
-                };
-                TerminalBuilder::new(
-                    local_path.map(|path| path.to_path_buf()),
-                    task_state,
-                    shell,
-                    env,
-                    settings.cursor_shape,
-                    settings.alternate_scroll,
-                    settings.max_scroll_history_lines,
-                    is_via_remote,
-                    cx.entity_id().as_u64(),
-                    Some(completion_tx),
-                    cx,
-                    activation_script,
-                )
-                .map(|builder| {
-                    let terminal_handle = cx.new(|cx| builder.subscribe(cx));
-
-                    this.terminals
-                        .local_handles
-                        .push(terminal_handle.downgrade());
-
-                    let id = terminal_handle.entity_id();
-                    cx.observe_release(&terminal_handle, move |project, _terminal, cx| {
-                        let handles = &mut project.terminals.local_handles;
-
-                        if let Some(index) = handles
-                            .iter()
-                            .position(|terminal| terminal.entity_id() == id)
-                        {
-                            handles.remove(index);
-                            cx.notify();
+                                ),
+                            },
                         }
-                    })
-                    .detach();
+                    };
+                    anyhow::Ok(TerminalBuilder::new(
+                        local_path.map(|path| path.to_path_buf()),
+                        task_state,
+                        shell,
+                        env,
+                        settings.cursor_shape,
+                        settings.alternate_scroll,
+                        settings.max_scroll_history_lines,
+                        is_via_remote,
+                        cx.entity_id().as_u64(),
+                        Some(completion_tx),
+                        cx,
+                        activation_script,
+                    ))
+                })??
+                .await?;
+            project.update(cx, move |this, cx| {
+                let terminal_handle = cx.new(|cx| builder.subscribe(cx));
+
+                this.terminals
+                    .local_handles
+                    .push(terminal_handle.downgrade());
+
+                let id = terminal_handle.entity_id();
+                cx.observe_release(&terminal_handle, move |project, _terminal, cx| {
+                    let handles = &mut project.terminals.local_handles;
 
-                    terminal_handle
+                    if let Some(index) = handles
+                        .iter()
+                        .position(|terminal| terminal.entity_id() == id)
+                    {
+                        handles.remove(index);
+                        cx.notify();
+                    }
                 })
-            })?
+                .detach();
+
+                terminal_handle
+            })
         })
     }
 
@@ -355,53 +358,55 @@ impl Project {
             })
             .await
             .unwrap_or_default();
-            project.update(cx, move |this, cx| {
-                let (shell, env) = {
-                    match remote_client {
-                        Some(remote_client) => {
-                            create_remote_shell(None, env, path, remote_client, cx)?
-                        }
-                        None => (settings.shell, env),
-                    }
-                };
-                TerminalBuilder::new(
-                    local_path.map(|path| path.to_path_buf()),
-                    None,
-                    shell,
-                    env,
-                    settings.cursor_shape,
-                    settings.alternate_scroll,
-                    settings.max_scroll_history_lines,
-                    is_via_remote,
-                    cx.entity_id().as_u64(),
-                    None,
-                    cx,
-                    activation_script,
-                )
-                .map(|builder| {
-                    let terminal_handle = cx.new(|cx| builder.subscribe(cx));
-
-                    this.terminals
-                        .local_handles
-                        .push(terminal_handle.downgrade());
-
-                    let id = terminal_handle.entity_id();
-                    cx.observe_release(&terminal_handle, move |project, _terminal, cx| {
-                        let handles = &mut project.terminals.local_handles;
-
-                        if let Some(index) = handles
-                            .iter()
-                            .position(|terminal| terminal.entity_id() == id)
-                        {
-                            handles.remove(index);
-                            cx.notify();
+            let builder = project
+                .update(cx, move |_, cx| {
+                    let (shell, env) = {
+                        match remote_client {
+                            Some(remote_client) => {
+                                create_remote_shell(None, env, path, remote_client, cx)?
+                            }
+                            None => (settings.shell, env),
                         }
-                    })
-                    .detach();
+                    };
+                    anyhow::Ok(TerminalBuilder::new(
+                        local_path.map(|path| path.to_path_buf()),
+                        None,
+                        shell,
+                        env,
+                        settings.cursor_shape,
+                        settings.alternate_scroll,
+                        settings.max_scroll_history_lines,
+                        is_via_remote,
+                        cx.entity_id().as_u64(),
+                        None,
+                        cx,
+                        activation_script,
+                    ))
+                })??
+                .await?;
+            project.update(cx, move |this, cx| {
+                let terminal_handle = cx.new(|cx| builder.subscribe(cx));
+
+                this.terminals
+                    .local_handles
+                    .push(terminal_handle.downgrade());
+
+                let id = terminal_handle.entity_id();
+                cx.observe_release(&terminal_handle, move |project, _terminal, cx| {
+                    let handles = &mut project.terminals.local_handles;
 
-                    terminal_handle
+                    if let Some(index) = handles
+                        .iter()
+                        .position(|terminal| terminal.entity_id() == id)
+                    {
+                        handles.remove(index);
+                        cx.notify();
+                    }
                 })
-            })?
+                .detach();
+
+                terminal_handle
+            })
         })
     }
 
@@ -422,13 +427,14 @@ impl Project {
             cwd
         };
 
-        let new_terminal = terminal
-            .read(cx)
-            .clone_builder(cx, local_path)
-            .map(|builder| {
-                let terminal_handle = cx.new(|cx| builder.subscribe(cx));
+        let builder = terminal.read(cx).clone_builder(cx, local_path);
+        cx.spawn(async |project, cx| {
+            let terminal = builder.await?;
+            project.update(cx, |project, cx| {
+                let terminal_handle = cx.new(|cx| terminal.subscribe(cx));
 
-                self.terminals
+                project
+                    .terminals
                     .local_handles
                     .push(terminal_handle.downgrade());
 
@@ -447,8 +453,8 @@ impl Project {
                 .detach();
 
                 terminal_handle
-            });
-        Task::ready(new_terminal)
+            })
+        })
     }
 
     pub fn terminal_settings<'a>(

crates/terminal/src/terminal.rs 🔗

@@ -402,6 +402,7 @@ impl TerminalBuilder {
                 window_id,
             },
             child_exited: None,
+            event_loop_task: Task::ready(Ok(())),
         };
 
         Ok(TerminalBuilder {
@@ -423,236 +424,236 @@ impl TerminalBuilder {
         completion_tx: Option<Sender<Option<ExitStatus>>>,
         cx: &App,
         activation_script: Vec<String>,
-    ) -> Result<TerminalBuilder> {
-        // If the parent environment doesn't have a locale set
-        // (As is the case when launched from a .app on MacOS),
-        // and the Project doesn't have a locale set, then
-        // set a fallback for our child environment to use.
-        if std::env::var("LANG").is_err() {
-            env.entry("LANG".to_string())
-                .or_insert_with(|| "en_US.UTF-8".to_string());
-        }
-
-        env.insert("ZED_TERM".to_string(), "true".to_string());
-        env.insert("TERM_PROGRAM".to_string(), "zed".to_string());
-        env.insert("TERM".to_string(), "xterm-256color".to_string());
-        env.insert("COLORTERM".to_string(), "truecolor".to_string());
-        env.insert(
-            "TERM_PROGRAM_VERSION".to_string(),
-            release_channel::AppVersion::global(cx).to_string(),
-        );
-
-        #[derive(Default)]
-        struct ShellParams {
-            program: String,
-            args: Option<Vec<String>>,
-            title_override: Option<String>,
-        }
-
-        impl ShellParams {
-            fn new(
+    ) -> Task<Result<TerminalBuilder>> {
+        let version = release_channel::AppVersion::global(cx);
+        cx.background_spawn(async move {
+            // If the parent environment doesn't have a locale set
+            // (As is the case when launched from a .app on MacOS),
+            // and the Project doesn't have a locale set, then
+            // set a fallback for our child environment to use.
+            if std::env::var("LANG").is_err() {
+                env.entry("LANG".to_string())
+                    .or_insert_with(|| "en_US.UTF-8".to_string());
+            }
+
+            env.insert("ZED_TERM".to_string(), "true".to_string());
+            env.insert("TERM_PROGRAM".to_string(), "zed".to_string());
+            env.insert("TERM".to_string(), "xterm-256color".to_string());
+            env.insert("COLORTERM".to_string(), "truecolor".to_string());
+            env.insert("TERM_PROGRAM_VERSION".to_string(), version.to_string());
+
+            #[derive(Default)]
+            struct ShellParams {
                 program: String,
                 args: Option<Vec<String>>,
                 title_override: Option<String>,
-            ) -> Self {
-                log::debug!("Using {program} as shell");
-                Self {
-                    program,
-                    args,
-                    title_override,
-                }
             }
-        }
 
-        let shell_params = match shell.clone() {
-            Shell::System => {
-                if cfg!(windows) {
-                    Some(ShellParams::new(
-                        util::shell::get_windows_system_shell(),
-                        None,
-                        None,
-                    ))
-                } else {
-                    None
+            impl ShellParams {
+                fn new(
+                    program: String,
+                    args: Option<Vec<String>>,
+                    title_override: Option<String>,
+                ) -> Self {
+                    log::debug!("Using {program} as shell");
+                    Self {
+                        program,
+                        args,
+                        title_override,
+                    }
                 }
             }
-            Shell::Program(program) => Some(ShellParams::new(program, None, None)),
-            Shell::WithArguments {
-                program,
-                args,
-                title_override,
-            } => Some(ShellParams::new(program, Some(args), title_override)),
-        };
-        let terminal_title_override = shell_params.as_ref().and_then(|e| e.title_override.clone());
 
-        #[cfg(windows)]
-        let shell_program = shell_params.as_ref().map(|params| {
-            use util::ResultExt;
+            let shell_params = match shell.clone() {
+                Shell::System => {
+                    if cfg!(windows) {
+                        Some(ShellParams::new(
+                            util::shell::get_windows_system_shell(),
+                            None,
+                            None,
+                        ))
+                    } else {
+                        None
+                    }
+                }
+                Shell::Program(program) => Some(ShellParams::new(program, None, None)),
+                Shell::WithArguments {
+                    program,
+                    args,
+                    title_override,
+                } => Some(ShellParams::new(program, Some(args), title_override)),
+            };
+            let terminal_title_override =
+                shell_params.as_ref().and_then(|e| e.title_override.clone());
 
-            Self::resolve_path(&params.program)
-                .log_err()
-                .unwrap_or(params.program.clone())
-        });
+            #[cfg(windows)]
+            let shell_program = shell_params.as_ref().map(|params| {
+                use util::ResultExt;
 
-        // Note: when remoting, this shell_kind will scrutinize `ssh` or
-        // `wsl.exe` as a shell and fall back to posix or powershell based on
-        // the compilation target. This is fine right now due to the restricted
-        // way we use the return value, but would become incorrect if we
-        // supported remoting into windows.
-        let shell_kind = shell.shell_kind(cfg!(windows));
-
-        let pty_options = {
-            let alac_shell = shell_params.as_ref().map(|params| {
-                alacritty_terminal::tty::Shell::new(
-                    params.program.clone(),
-                    params.args.clone().unwrap_or_default(),
-                )
+                Self::resolve_path(&params.program)
+                    .log_err()
+                    .unwrap_or(params.program.clone())
             });
 
-            alacritty_terminal::tty::Options {
-                shell: alac_shell,
-                working_directory: working_directory.clone(),
-                drain_on_exit: true,
-                env: env.clone().into_iter().collect(),
-                #[cfg(windows)]
-                escape_args: shell_kind.tty_escape_args(),
-            }
-        };
+            // Note: when remoting, this shell_kind will scrutinize `ssh` or
+            // `wsl.exe` as a shell and fall back to posix or powershell based on
+            // the compilation target. This is fine right now due to the restricted
+            // way we use the return value, but would become incorrect if we
+            // supported remoting into windows.
+            let shell_kind = shell.shell_kind(cfg!(windows));
+
+            let pty_options = {
+                let alac_shell = shell_params.as_ref().map(|params| {
+                    alacritty_terminal::tty::Shell::new(
+                        params.program.clone(),
+                        params.args.clone().unwrap_or_default(),
+                    )
+                });
 
-        let default_cursor_style = AlacCursorStyle::from(cursor_shape);
-        let scrolling_history = if task.is_some() {
-            // Tasks like `cargo build --all` may produce a lot of output, ergo allow maximum scrolling.
-            // After the task finishes, we do not allow appending to that terminal, so small tasks output should not
-            // cause excessive memory usage over time.
-            MAX_SCROLL_HISTORY_LINES
-        } else {
-            max_scroll_history_lines
-                .unwrap_or(DEFAULT_SCROLL_HISTORY_LINES)
-                .min(MAX_SCROLL_HISTORY_LINES)
-        };
-        let config = Config {
-            scrolling_history,
-            default_cursor_style,
-            ..Config::default()
-        };
+                alacritty_terminal::tty::Options {
+                    shell: alac_shell,
+                    working_directory: working_directory.clone(),
+                    drain_on_exit: true,
+                    env: env.clone().into_iter().collect(),
+                    #[cfg(windows)]
+                    escape_args: shell_kind.tty_escape_args(),
+                }
+            };
 
-        //Spawn a task so the Alacritty EventLoop can communicate with us
-        //TODO: Remove with a bounded sender which can be dispatched on &self
-        let (events_tx, events_rx) = unbounded();
-        //Set up the terminal...
-        let mut term = Term::new(
-            config.clone(),
-            &TerminalBounds::default(),
-            ZedListener(events_tx.clone()),
-        );
+            let default_cursor_style = AlacCursorStyle::from(cursor_shape);
+            let scrolling_history = if task.is_some() {
+                // Tasks like `cargo build --all` may produce a lot of output, ergo allow maximum scrolling.
+                // After the task finishes, we do not allow appending to that terminal, so small tasks output should not
+                // cause excessive memory usage over time.
+                MAX_SCROLL_HISTORY_LINES
+            } else {
+                max_scroll_history_lines
+                    .unwrap_or(DEFAULT_SCROLL_HISTORY_LINES)
+                    .min(MAX_SCROLL_HISTORY_LINES)
+            };
+            let config = Config {
+                scrolling_history,
+                default_cursor_style,
+                ..Config::default()
+            };
 
-        //Alacritty defaults to alternate scrolling being on, so we just need to turn it off.
-        if let AlternateScroll::Off = alternate_scroll {
-            term.unset_private_mode(PrivateMode::Named(NamedPrivateMode::AlternateScroll));
-        }
+            //Setup the pty...
+            let pty = match tty::new(&pty_options, TerminalBounds::default().into(), window_id) {
+                Ok(pty) => pty,
+                Err(error) => {
+                    bail!(TerminalError {
+                        directory: working_directory,
+                        program: shell_params.as_ref().map(|params| params.program.clone()),
+                        args: shell_params.as_ref().and_then(|params| params.args.clone()),
+                        title_override: terminal_title_override,
+                        source: error,
+                    });
+                }
+            };
 
-        let term = Arc::new(FairMutex::new(term));
+            //Spawn a task so the Alacritty EventLoop can communicate with us
+            //TODO: Remove with a bounded sender which can be dispatched on &self
+            let (events_tx, events_rx) = unbounded();
+            //Set up the terminal...
+            let mut term = Term::new(
+                config.clone(),
+                &TerminalBounds::default(),
+                ZedListener(events_tx.clone()),
+            );
 
-        //Setup the pty...
-        let pty = match tty::new(&pty_options, TerminalBounds::default().into(), window_id) {
-            Ok(pty) => pty,
-            Err(error) => {
-                bail!(TerminalError {
-                    directory: working_directory,
-                    program: shell_params.as_ref().map(|params| params.program.clone()),
-                    args: shell_params.as_ref().and_then(|params| params.args.clone()),
-                    title_override: terminal_title_override,
-                    source: error,
-                });
+            //Alacritty defaults to alternate scrolling being on, so we just need to turn it off.
+            if let AlternateScroll::Off = alternate_scroll {
+                term.unset_private_mode(PrivateMode::Named(NamedPrivateMode::AlternateScroll));
             }
-        };
 
-        let pty_info = PtyProcessInfo::new(&pty);
+            let term = Arc::new(FairMutex::new(term));
 
-        //And connect them together
-        let event_loop = EventLoop::new(
-            term.clone(),
-            ZedListener(events_tx),
-            pty,
-            pty_options.drain_on_exit,
-            false,
-        )
-        .context("failed to create event loop")?;
+            let pty_info = PtyProcessInfo::new(&pty);
 
-        //Kick things off
-        let pty_tx = event_loop.channel();
-        let _io_thread = event_loop.spawn(); // DANGER
+            //And connect them together
+            let event_loop = EventLoop::new(
+                term.clone(),
+                ZedListener(events_tx),
+                pty,
+                pty_options.drain_on_exit,
+                false,
+            )
+            .context("failed to create event loop")?;
 
-        let no_task = task.is_none();
+            let pty_tx = event_loop.channel();
+            let _io_thread = event_loop.spawn(); // DANGER
 
-        let terminal = Terminal {
-            task,
-            terminal_type: TerminalType::Pty {
-                pty_tx: Notifier(pty_tx),
-                info: pty_info,
-            },
-            completion_tx,
-            term,
-            term_config: config,
-            title_override: terminal_title_override,
-            events: VecDeque::with_capacity(10), //Should never get this high.
-            last_content: Default::default(),
-            last_mouse: None,
-            matches: Vec::new(),
-            selection_head: None,
-            breadcrumb_text: String::new(),
-            scroll_px: px(0.),
-            next_link_id: 0,
-            selection_phase: SelectionPhase::Ended,
-            hyperlink_regex_searches: RegexSearches::new(),
-            vi_mode_enabled: false,
-            is_ssh_terminal,
-            last_mouse_move_time: Instant::now(),
-            last_hyperlink_search_position: None,
-            #[cfg(windows)]
-            shell_program,
-            activation_script: activation_script.clone(),
-            template: CopyTemplate {
-                shell,
-                env,
-                cursor_shape,
-                alternate_scroll,
-                max_scroll_history_lines,
-                window_id,
-            },
-            child_exited: None,
-        };
+            let no_task = task.is_none();
+            let terminal = Terminal {
+                task,
+                terminal_type: TerminalType::Pty {
+                    pty_tx: Notifier(pty_tx),
+                    info: pty_info,
+                },
+                completion_tx,
+                term,
+                term_config: config,
+                title_override: terminal_title_override,
+                events: VecDeque::with_capacity(10), //Should never get this high.
+                last_content: Default::default(),
+                last_mouse: None,
+                matches: Vec::new(),
+                selection_head: None,
+                breadcrumb_text: String::new(),
+                scroll_px: px(0.),
+                next_link_id: 0,
+                selection_phase: SelectionPhase::Ended,
+                hyperlink_regex_searches: RegexSearches::new(),
+                vi_mode_enabled: false,
+                is_ssh_terminal,
+                last_mouse_move_time: Instant::now(),
+                last_hyperlink_search_position: None,
+                #[cfg(windows)]
+                shell_program,
+                activation_script: activation_script.clone(),
+                template: CopyTemplate {
+                    shell,
+                    env,
+                    cursor_shape,
+                    alternate_scroll,
+                    max_scroll_history_lines,
+                    window_id,
+                },
+                child_exited: None,
+                event_loop_task: Task::ready(Ok(())),
+            };
 
-        if !activation_script.is_empty() && no_task {
-            for activation_script in activation_script {
-                terminal.write_to_pty(activation_script.into_bytes());
+            if !activation_script.is_empty() && no_task {
+                for activation_script in activation_script {
+                    terminal.write_to_pty(activation_script.into_bytes());
+                    // Simulate enter key press
+                    // NOTE(PowerShell): using `\r\n` will put PowerShell in a continuation mode (infamous >> character)
+                    // and generally mess up the rendering.
+                    terminal.write_to_pty(b"\x0d");
+                }
+                // In order to clear the screen at this point, we have two options:
+                // 1. We can send a shell-specific command such as "clear" or "cls"
+                // 2. We can "echo" a marker message that we will then catch when handling a Wakeup event
+                //    and clear the screen using `terminal.clear()` method
+                // We cannot issue a `terminal.clear()` command at this point as alacritty is evented
+                // and while we have sent the activation script to the pty, it will be executed asynchronously.
+                // Therefore, we somehow need to wait for the activation script to finish executing before we
+                // can proceed with clearing the screen.
+                terminal.write_to_pty(shell_kind.clear_screen_command().as_bytes());
                 // Simulate enter key press
-                // NOTE(PowerShell): using `\r\n` will put PowerShell in a continuation mode (infamous >> character)
-                // and generally mess up the rendering.
                 terminal.write_to_pty(b"\x0d");
             }
-            // In order to clear the screen at this point, we have two options:
-            // 1. We can send a shell-specific command such as "clear" or "cls"
-            // 2. We can "echo" a marker message that we will then catch when handling a Wakeup event
-            //    and clear the screen using `terminal.clear()` method
-            // We cannot issue a `terminal.clear()` command at this point as alacritty is evented
-            // and while we have sent the activation script to the pty, it will be executed asynchronously.
-            // Therefore, we somehow need to wait for the activation script to finish executing before we
-            // can proceed with clearing the screen.
-            terminal.write_to_pty(shell_kind.clear_screen_command().as_bytes());
-            // Simulate enter key press
-            terminal.write_to_pty(b"\x0d");
-        }
 
-        Ok(TerminalBuilder {
-            terminal,
-            events_rx,
+            Ok(TerminalBuilder {
+                terminal,
+                events_rx,
+            })
         })
     }
 
     pub fn subscribe(mut self, cx: &Context<Terminal>) -> Terminal {
         //Event loop
-        cx.spawn(async move |terminal, cx| {
+        self.terminal.event_loop_task = cx.spawn(async move |terminal, cx| {
             while let Some(event) = self.events_rx.next().await {
                 terminal.update(cx, |terminal, cx| {
                     //Process the first event immediately for lowered latency
@@ -709,11 +710,8 @@ impl TerminalBuilder {
                     smol::future::yield_now().await;
                 }
             }
-
             anyhow::Ok(())
-        })
-        .detach();
-
+        });
         self.terminal
     }
 
@@ -836,6 +834,7 @@ pub struct Terminal {
     template: CopyTemplate,
     activation_script: Vec<String>,
     child_exited: Option<ExitStatus>,
+    event_loop_task: Task<Result<(), anyhow::Error>>,
 }
 
 struct CopyTemplate {
@@ -1266,15 +1265,11 @@ impl Terminal {
     }
 
     pub fn total_lines(&self) -> usize {
-        let term = self.term.clone();
-        let terminal = term.lock_unfair();
-        terminal.total_lines()
+        self.term.lock_unfair().total_lines()
     }
 
     pub fn viewport_lines(&self) -> usize {
-        let term = self.term.clone();
-        let terminal = term.lock_unfair();
-        terminal.screen_lines()
+        self.term.lock_unfair().screen_lines()
     }
 
     //To test:
@@ -2151,7 +2146,7 @@ impl Terminal {
         self.vi_mode_enabled
     }
 
-    pub fn clone_builder(&self, cx: &App, cwd: Option<PathBuf>) -> Result<TerminalBuilder> {
+    pub fn clone_builder(&self, cx: &App, cwd: Option<PathBuf>) -> Task<Result<TerminalBuilder>> {
         let working_directory = self.working_directory().or_else(|| cwd);
         TerminalBuilder::new(
             working_directory,
@@ -2388,28 +2383,30 @@ mod tests {
         let (completion_tx, completion_rx) = smol::channel::unbounded();
         let (program, args) = ShellBuilder::new(&Shell::System, false)
             .build(Some("echo".to_owned()), &["hello".to_owned()]);
-        let terminal = cx.new(|cx| {
-            TerminalBuilder::new(
-                None,
-                None,
-                task::Shell::WithArguments {
-                    program,
-                    args,
-                    title_override: None,
-                },
-                HashMap::default(),
-                CursorShape::default(),
-                AlternateScroll::On,
-                None,
-                false,
-                0,
-                Some(completion_tx),
-                cx,
-                vec![],
-            )
-            .unwrap()
-            .subscribe(cx)
-        });
+        let builder = cx
+            .update(|cx| {
+                TerminalBuilder::new(
+                    None,
+                    None,
+                    task::Shell::WithArguments {
+                        program,
+                        args,
+                        title_override: None,
+                    },
+                    HashMap::default(),
+                    CursorShape::default(),
+                    AlternateScroll::On,
+                    None,
+                    false,
+                    0,
+                    Some(completion_tx),
+                    cx,
+                    vec![],
+                )
+            })
+            .await
+            .unwrap();
+        let terminal = cx.new(|cx| builder.subscribe(cx));
         assert_eq!(
             completion_rx.recv().await.unwrap(),
             Some(ExitStatus::default())
@@ -2438,25 +2435,27 @@ mod tests {
         cx.executor().allow_parking();
 
         let (completion_tx, completion_rx) = smol::channel::unbounded();
+        let builder = cx
+            .update(|cx| {
+                TerminalBuilder::new(
+                    None,
+                    None,
+                    task::Shell::System,
+                    HashMap::default(),
+                    CursorShape::default(),
+                    AlternateScroll::On,
+                    None,
+                    false,
+                    0,
+                    Some(completion_tx),
+                    cx,
+                    Vec::new(),
+                )
+            })
+            .await
+            .unwrap();
         // Build an empty command, which will result in a tty shell spawned.
-        let terminal = cx.new(|cx| {
-            TerminalBuilder::new(
-                None,
-                None,
-                task::Shell::System,
-                HashMap::default(),
-                CursorShape::default(),
-                AlternateScroll::On,
-                None,
-                false,
-                0,
-                Some(completion_tx),
-                cx,
-                Vec::new(),
-            )
-            .unwrap()
-            .subscribe(cx)
-        });
+        let terminal = cx.new(|cx| builder.subscribe(cx));
 
         let (event_tx, event_rx) = smol::channel::unbounded::<Event>();
         cx.update(|cx| {
@@ -2507,28 +2506,30 @@ mod tests {
         let (completion_tx, completion_rx) = smol::channel::unbounded();
         let (program, args) = ShellBuilder::new(&Shell::System, false)
             .build(Some("asdasdasdasd".to_owned()), &["@@@@@".to_owned()]);
-        let terminal = cx.new(|cx| {
-            TerminalBuilder::new(
-                None,
-                None,
-                task::Shell::WithArguments {
-                    program,
-                    args,
-                    title_override: None,
-                },
-                HashMap::default(),
-                CursorShape::default(),
-                AlternateScroll::On,
-                None,
-                false,
-                0,
-                Some(completion_tx),
-                cx,
-                Vec::new(),
-            )
-            .unwrap()
-            .subscribe(cx)
-        });
+        let builder = cx
+            .update(|cx| {
+                TerminalBuilder::new(
+                    None,
+                    None,
+                    task::Shell::WithArguments {
+                        program,
+                        args,
+                        title_override: None,
+                    },
+                    HashMap::default(),
+                    CursorShape::default(),
+                    AlternateScroll::On,
+                    None,
+                    false,
+                    0,
+                    Some(completion_tx),
+                    cx,
+                    Vec::new(),
+                )
+            })
+            .await
+            .unwrap();
+        let terminal = cx.new(|cx| builder.subscribe(cx));
 
         let (event_tx, event_rx) = smol::channel::unbounded::<Event>();
         cx.update(|cx| {

crates/terminal_view/src/persistence.rs 🔗

@@ -214,14 +214,6 @@ async fn deserialize_pane_group(
         }
         SerializedPaneGroup::Pane(serialized_pane) => {
             let active = serialized_pane.active;
-            let new_items = deserialize_terminal_views(
-                workspace_id,
-                project.clone(),
-                workspace.clone(),
-                serialized_pane.children.as_slice(),
-                cx,
-            )
-            .await;
 
             let pane = panel
                 .update_in(cx, |terminal_panel, window, cx| {
@@ -236,56 +228,71 @@ async fn deserialize_pane_group(
                 .log_err()?;
             let active_item = serialized_pane.active_item;
             let pinned_count = serialized_pane.pinned_count;
-            let terminal = pane
-                .update_in(cx, |pane, window, cx| {
-                    populate_pane_items(pane, new_items, active_item, window, cx);
-                    pane.set_pinned_count(pinned_count);
+            let new_items = deserialize_terminal_views(
+                workspace_id,
+                project.clone(),
+                workspace.clone(),
+                serialized_pane.children.as_slice(),
+                cx,
+            );
+            cx.spawn({
+                let pane = pane.downgrade();
+                async move |cx| {
+                    let new_items = new_items.await;
+
+                    let items = pane.update_in(cx, |pane, window, cx| {
+                        populate_pane_items(pane, new_items, active_item, window, cx);
+                        pane.set_pinned_count(pinned_count);
+                        pane.items_len()
+                    });
                     // Avoid blank panes in splits
-                    if pane.items_len() == 0 {
+                    if items.is_ok_and(|items| items == 0) {
                         let working_directory = workspace
                             .update(cx, |workspace, cx| default_working_directory(workspace, cx))
                             .ok()
                             .flatten();
-                        let terminal = project.update(cx, |project, cx| {
-                            project.create_terminal_shell(working_directory, cx)
-                        });
-                        Some(Some(terminal))
-                    } else {
-                        Some(None)
+                        let Some(terminal) = project
+                            .update(cx, |project, cx| {
+                                project.create_terminal_shell(working_directory, cx)
+                            })
+                            .log_err()
+                        else {
+                            return;
+                        };
+
+                        let terminal = terminal.await.log_err();
+                        pane.update_in(cx, |pane, window, cx| {
+                            if let Some(terminal) = terminal {
+                                let terminal_view = Box::new(cx.new(|cx| {
+                                    TerminalView::new(
+                                        terminal,
+                                        workspace.clone(),
+                                        Some(workspace_id),
+                                        project.downgrade(),
+                                        window,
+                                        cx,
+                                    )
+                                }));
+                                pane.add_item(terminal_view, true, false, None, window, cx);
+                            }
+                        })
+                        .ok();
                     }
-                })
-                .ok()
-                .flatten()?;
-            if let Some(terminal) = terminal {
-                let terminal = terminal.await.ok()?;
-                pane.update_in(cx, |pane, window, cx| {
-                    let terminal_view = Box::new(cx.new(|cx| {
-                        TerminalView::new(
-                            terminal,
-                            workspace.clone(),
-                            Some(workspace_id),
-                            project.downgrade(),
-                            window,
-                            cx,
-                        )
-                    }));
-                    pane.add_item(terminal_view, true, false, None, window, cx);
-                })
-                .ok()?;
-            }
+                }
+            })
+            .await;
             Some((Member::Pane(pane.clone()), active.then_some(pane)))
         }
     }
 }
 
-async fn deserialize_terminal_views(
+fn deserialize_terminal_views(
     workspace_id: WorkspaceId,
     project: Entity<Project>,
     workspace: WeakEntity<Workspace>,
     item_ids: &[u64],
     cx: &mut AsyncWindowContext,
-) -> Vec<Entity<TerminalView>> {
-    let mut items = Vec::with_capacity(item_ids.len());
+) -> impl Future<Output = Vec<Entity<TerminalView>>> + use<> {
     let mut deserialized_items = item_ids
         .iter()
         .map(|item_id| {
@@ -302,12 +309,15 @@ async fn deserialize_terminal_views(
             .unwrap_or_else(|e| Task::ready(Err(e.context("no window present"))))
         })
         .collect::<FuturesUnordered<_>>();
-    while let Some(item) = deserialized_items.next().await {
-        if let Some(item) = item.log_err() {
-            items.push(item);
+    async move {
+        let mut items = Vec::with_capacity(deserialized_items.len());
+        while let Some(item) = deserialized_items.next().await {
+            if let Some(item) = item.log_err() {
+                items.push(item);
+            }
         }
+        items
     }
-    items
 }
 
 #[derive(Debug, Serialize, Deserialize)]

crates/terminal_view/src/terminal_view.rs 🔗

@@ -1223,26 +1223,26 @@ impl Item for TerminalView {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Task<Option<Entity<Self>>> {
-        let Some(terminal_task) = self
-            .project
-            .update(cx, |project, cx| {
-                let cwd = project
-                    .active_project_directory(cx)
-                    .map(|it| it.to_path_buf());
-                project.clone_terminal(self.terminal(), cx, cwd)
-            })
-            .ok()
-        else {
+        let Ok(terminal) = self.project.update(cx, |project, cx| {
+            let cwd = project
+                .active_project_directory(cx)
+                .map(|it| it.to_path_buf());
+            project.clone_terminal(self.terminal(), cx, cwd)
+        }) else {
             return Task::ready(None);
         };
-
-        let workspace = self.workspace.clone();
-        let project = self.project.clone();
-        cx.spawn_in(window, async move |_, cx| {
-            let terminal = terminal_task.await.log_err()?;
-            cx.update(|window, cx| {
+        cx.spawn_in(window, async move |this, cx| {
+            let terminal = terminal.await.log_err()?;
+            this.update_in(cx, |this, window, cx| {
                 cx.new(|cx| {
-                    TerminalView::new(terminal, workspace, workspace_id, project, window, cx)
+                    TerminalView::new(
+                        terminal,
+                        this.workspace.clone(),
+                        workspace_id,
+                        this.project.clone(),
+                        window,
+                        cx,
+                    )
                 })
             })
             .ok()