From b9ca216501f21b097dc38a0beccd78ed846b5a76 Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Sat, 30 Mar 2024 19:20:02 +0200 Subject: [PATCH 1/9] Don't request exit multiple times --- src/window/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/window/mod.rs b/src/window/mod.rs index 871db0b74..ad975d6d8 100644 --- a/src/window/mod.rs +++ b/src/window/mod.rs @@ -320,7 +320,7 @@ pub fn main_loop( return; } - if !RUNNING_TRACKER.is_running() { + if !RUNNING_TRACKER.is_running() && !window_target.exiting() { save_window_size(&window_wrapper); window_target.exit(); } else { From 66a85d527b3138cc4f1cc5620350138e3d0e9545 Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Sun, 31 Mar 2024 12:46:54 +0300 Subject: [PATCH 2/9] Wait until the Neovim exits before exiting the event loop --- src/bridge/mod.rs | 13 +++++++------ src/bridge/ui_commands.rs | 15 ++++++--------- src/running_tracker.rs | 14 +++----------- src/window/mod.rs | 17 ++++++++--------- 4 files changed, 24 insertions(+), 35 deletions(-) diff --git a/src/bridge/mod.rs b/src/bridge/mod.rs index 74c45ee12..75b05b1d6 100644 --- a/src/bridge/mod.rs +++ b/src/bridge/mod.rs @@ -17,8 +17,8 @@ use tokio::runtime::{Builder, Runtime}; use winit::event_loop::EventLoopProxy; use crate::{ - cmd_line::CmdLineSettings, editor::start_editor, running_tracker::*, settings::*, - units::GridSize, window::UserEvent, + cmd_line::CmdLineSettings, editor::start_editor, settings::*, + window::UserEvent, units::GridSize, }; pub use handler::NeovimHandler; use session::{NeovimInstance, NeovimSession}; @@ -134,7 +134,7 @@ async fn launch(handler: NeovimHandler, grid_size: Option>) -> Res res.map(|()| session) } -async fn run(session: NeovimSession) { +async fn run(session: NeovimSession, proxy: EventLoopProxy) { match session.io_handle.await { Err(join_error) => error!("Error joining IO loop: '{}'", join_error), Ok(Err(error)) => { @@ -144,7 +144,8 @@ async fn run(session: NeovimSession) { } Ok(Ok(())) => {} }; - RUNNING_TRACKER.quit("neovim processed failed"); + log::info!("Neovim has quit"); + proxy.send_event(UserEvent::NeovimExited).ok(); } impl NeovimRuntime { @@ -161,10 +162,10 @@ impl NeovimRuntime { event_loop_proxy: EventLoopProxy, grid_size: Option>, ) -> Result<()> { - let handler = start_editor(event_loop_proxy); + let handler = start_editor(event_loop_proxy.clone()); let runtime = self.runtime.as_ref().unwrap(); let session = runtime.block_on(launch(handler, grid_size))?; - runtime.spawn(run(session)); + runtime.spawn(run(session, event_loop_proxy)); Ok(()) } } diff --git a/src/bridge/ui_commands.rs b/src/bridge/ui_commands.rs index cd278508b..a87ba7e0d 100644 --- a/src/bridge/ui_commands.rs +++ b/src/bridge/ui_commands.rs @@ -11,7 +11,6 @@ use super::show_error_message; use crate::{ bridge::{ApiInformation, NeovimWriter}, profiling::{tracy_dynamic_zone, tracy_fiber_enter, tracy_fiber_leave}, - running_tracker::RUNNING_TRACKER, LoggingSender, }; @@ -295,7 +294,7 @@ pub fn start_ui_command_handler(nvim: Neovim, api_information: &Ap let ui_command_nvim = nvim.clone(); tokio::spawn(async move { let mut ui_command_receiver = UI_CHANNELS.receiver.lock().unwrap().take().unwrap(); - while RUNNING_TRACKER.is_running() { + loop { match ui_command_receiver.recv().await { Some(UiCommand::Serial(serial_command)) => { tracy_dynamic_zone!(serial_command.as_ref()); @@ -309,18 +308,17 @@ pub fn start_ui_command_handler(nvim: Neovim, api_information: &Ap parallel_command.execute(&ui_command_nvim).await; }); } - None => { - RUNNING_TRACKER.quit("ui command channel failed"); - } + None => break, } } + log::info!("ui command receiver finished"); }); let has_x_buttons = api_information.version.has_version(0, 10, 0); tokio::spawn(async move { tracy_fiber_enter!("Serial command"); - while RUNNING_TRACKER.is_running() { + loop { tracy_fiber_leave(); let res = serial_rx.recv().await; tracy_fiber_enter!("Serial command"); @@ -331,11 +329,10 @@ pub fn start_ui_command_handler(nvim: Neovim, api_information: &Ap serial_command.execute(&nvim, has_x_buttons).await; tracy_fiber_enter!("Serial command"); } - None => { - RUNNING_TRACKER.quit("serial ui command channel failed"); - } + None => break, } } + log::info!("serial command receiver finished"); }); } diff --git a/src/running_tracker.rs b/src/running_tracker.rs index 5c7383960..f330dc3d8 100644 --- a/src/running_tracker.rs +++ b/src/running_tracker.rs @@ -1,5 +1,5 @@ use std::sync::{ - atomic::{AtomicBool, AtomicI32, Ordering}, + atomic::{AtomicI32, Ordering}, Arc, }; @@ -10,34 +10,26 @@ lazy_static! { } pub struct RunningTracker { - running: Arc, exit_code: Arc, } impl RunningTracker { fn new() -> Self { Self { - running: Arc::new(AtomicBool::new(true)), exit_code: Arc::new(AtomicI32::new(0)), } } pub fn quit(&self, reason: &str) { - self.running.store(false, Ordering::Relaxed); info!("Quit {}", reason); } pub fn quit_with_code(&self, code: i32, reason: &str) { - self.exit_code.store(code, Ordering::Relaxed); - self.running.store(false, Ordering::Relaxed); + self.exit_code.store(code, Ordering::Release); info!("Quit with code {}: {}", code, reason); } - pub fn is_running(&self) -> bool { - self.running.load(Ordering::Relaxed) - } - pub fn exit_code(&self) -> i32 { - self.exit_code.load(Ordering::Relaxed) + self.exit_code.load(Ordering::Acquire) } } diff --git a/src/window/mod.rs b/src/window/mod.rs index ad975d6d8..2de613afb 100644 --- a/src/window/mod.rs +++ b/src/window/mod.rs @@ -40,7 +40,6 @@ use crate::{ cmd_line::{CmdLineSettings, GeometryArgs}, frame::Frame, renderer::{build_window_config, DrawCommand, WindowConfig}, - running_tracker::*, settings::{ clamped_grid_size, load_last_window_settings, save_window_size, FontSettings, HotReloadConfigs, PersistentWindowSettings, SettingsChanged, SETTINGS, @@ -90,6 +89,7 @@ pub enum UserEvent { ConfigsChanged(Box), #[allow(dead_code)] RedrawRequested, + NeovimExited, } impl From> for UserEvent { @@ -316,15 +316,14 @@ pub fn main_loop( event_loop.run(move |e, window_target| { #[cfg(target_os = "macos")] menu.ensure_menu_added(&e); - if e == Event::LoopExiting { - return; - } - if !RUNNING_TRACKER.is_running() && !window_target.exiting() { - save_window_size(&window_wrapper); - window_target.exit(); - } else { - window_target.set_control_flow(update_loop.step(&mut window_wrapper, e)); + match e { + Event::LoopExiting => (), + Event::UserEvent(UserEvent::NeovimExited) => { + save_window_size(&window_wrapper); + window_target.exit(); + } + _ => window_target.set_control_flow(update_loop.step(&mut window_wrapper, e)), } }) } From 2bee0b4a95e424396026483e7aa405516321f04b Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Sun, 31 Mar 2024 13:28:03 +0300 Subject: [PATCH 3/9] Fix exit crash on Wayland --- src/bridge/mod.rs | 4 ++-- src/renderer/opengl.rs | 30 ++++++++++++++++++++++++++---- src/window/mod.rs | 10 ++++++---- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/bridge/mod.rs b/src/bridge/mod.rs index 75b05b1d6..d3e03c0aa 100644 --- a/src/bridge/mod.rs +++ b/src/bridge/mod.rs @@ -17,8 +17,8 @@ use tokio::runtime::{Builder, Runtime}; use winit::event_loop::EventLoopProxy; use crate::{ - cmd_line::CmdLineSettings, editor::start_editor, settings::*, - window::UserEvent, units::GridSize, + cmd_line::CmdLineSettings, editor::start_editor, settings::*, units::GridSize, + window::UserEvent, }; pub use handler::NeovimHandler; use session::{NeovimInstance, NeovimSession}; diff --git a/src/renderer/opengl.rs b/src/renderer/opengl.rs index 126854020..2d1156518 100644 --- a/src/renderer/opengl.rs +++ b/src/renderer/opengl.rs @@ -53,7 +53,7 @@ pub struct OpenGLSkiaRenderer { context: PossiblyCurrentContext, window_surface: Surface, config: Config, - window: Window, + window: Option, } fn clamp_render_buffer_size(size: &PhysicalSize) -> PhysicalSize { @@ -145,7 +145,7 @@ impl OpenGLSkiaRenderer { Self { window_surface, context, - window, + window: Some(window), config, gr_context, fb_info, @@ -156,7 +156,7 @@ impl OpenGLSkiaRenderer { impl SkiaRenderer for OpenGLSkiaRenderer { fn window(&self) -> &Window { - &self.window + self.window.as_ref().unwrap() } fn flush(&mut self) { @@ -181,7 +181,7 @@ impl SkiaRenderer for OpenGLSkiaRenderer { fn resize(&mut self) { self.skia_surface = create_surface( &self.config, - &self.window.inner_size(), + &self.window().inner_size(), &self.context, &self.window_surface, &mut self.gr_context, @@ -215,6 +215,28 @@ impl SkiaRenderer for OpenGLSkiaRenderer { } } +impl Drop for OpenGLSkiaRenderer { + fn drop(&mut self) { + match self.window_surface.display() { + #[cfg(not(target_os = "macos"))] + glutin::display::Display::Egl(display) => { + // Ensure that all the windows are dropped, so the destructors for + // Renderer and contexts ran. + self.window = None; + + self.gr_context.release_resources_and_abandon(); + + // SAFETY: the display is being destroyed after destroying all the + // windows, thus no attempt to access the EGL state will be made. + unsafe { + display.terminate(); + } + } + _ => (), + } + } +} + fn gen_config(mut config_iterator: Box + '_>) -> Config { config_iterator.next().unwrap() } diff --git a/src/window/mod.rs b/src/window/mod.rs index 2de613afb..03462fe0f 100644 --- a/src/window/mod.rs +++ b/src/window/mod.rs @@ -299,7 +299,7 @@ pub fn main_loop( event_loop: EventLoop, ) -> Result<(), EventLoopError> { let cmd_line_settings = SETTINGS.get::(); - let mut window_wrapper = WinitWindowWrapper::new( + let window_wrapper = WinitWindowWrapper::new( window, initial_window_size, initial_font_settings, @@ -307,6 +307,7 @@ pub fn main_loop( ); let mut update_loop = UpdateLoop::new(cmd_line_settings.idle); + let mut window_wrapper = Some(window_wrapper); #[cfg(target_os = "macos")] let mut menu = { @@ -318,12 +319,13 @@ pub fn main_loop( menu.ensure_menu_added(&e); match e { - Event::LoopExiting => (), + Event::LoopExiting => window_wrapper = None, Event::UserEvent(UserEvent::NeovimExited) => { - save_window_size(&window_wrapper); + save_window_size(window_wrapper.as_ref().unwrap()); window_target.exit(); } - _ => window_target.set_control_flow(update_loop.step(&mut window_wrapper, e)), + _ => window_target + .set_control_flow(update_loop.step(window_wrapper.as_mut().unwrap(), e)), } }) } From 4d164ee73bc6ba8853bfb8f2a9975ede4ae302f5 Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Wed, 3 Apr 2024 18:24:55 +0300 Subject: [PATCH 4/9] Disable detach when the window is closed for remote connections --- src/window/window_wrapper.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/window/window_wrapper.rs b/src/window/window_wrapper.rs index 21f53c218..da9e443eb 100644 --- a/src/window/window_wrapper.rs +++ b/src/window/window_wrapper.rs @@ -14,7 +14,6 @@ use crate::{ bridge::{send_ui, ParallelCommand, SerialCommand}, profiling::{tracy_frame, tracy_gpu_collect, tracy_gpu_zone, tracy_plot, tracy_zone}, renderer::{create_skia_renderer, DrawCommand, Renderer, SkiaRenderer, VSync, WindowConfig}, - running_tracker::RUNNING_TRACKER, settings::{ clamped_grid_size, FontSettings, HotReloadConfigs, SettingsChanged, DEFAULT_GRID_SIZE, MIN_GRID_SIZE, SETTINGS, @@ -293,11 +292,7 @@ impl WinitWindowWrapper { } pub fn handle_quit(&mut self) { - if SETTINGS.get::().server.is_none() { - send_ui(ParallelCommand::Quit); - } else { - RUNNING_TRACKER.quit("window closed"); - } + send_ui(ParallelCommand::Quit); } pub fn handle_focus_lost(&mut self) { From 098b6ffda157bc620b637ca22fd30c8cf94fd433 Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Wed, 3 Apr 2024 22:11:34 +0300 Subject: [PATCH 5/9] Wait for all tasks to exits, so that logs don't go to stderr --- src/bridge/mod.rs | 44 +++++++++++++++++++++++++++++---------- src/bridge/ui_commands.rs | 32 +++++++++++++++++++++------- src/window/mod.rs | 5 +++-- 3 files changed, 61 insertions(+), 20 deletions(-) diff --git a/src/bridge/mod.rs b/src/bridge/mod.rs index d3e03c0aa..392b93064 100644 --- a/src/bridge/mod.rs +++ b/src/bridge/mod.rs @@ -12,8 +12,11 @@ use itertools::Itertools; use log::{error, info}; use nvim_rs::{error::CallError, Neovim, UiAttachOptions, Value}; use rmpv::Utf8String; -use std::{io::Error, ops::Add}; -use tokio::runtime::{Builder, Runtime}; +use std::{io::Error, ops::Add, time::Instant}; +use tokio::{ + runtime::{Builder, Runtime}, + task::JoinSet, +}; use winit::event_loop::EventLoopProxy; use crate::{ @@ -28,13 +31,16 @@ pub use api_info::*; pub use command::create_nvim_command; pub use events::*; pub use session::NeovimWriter; -pub use ui_commands::{send_ui, start_ui_command_handler, ParallelCommand, SerialCommand}; +pub use ui_commands::{ + send_ui, shutdown_ui, start_ui_command_handler, ParallelCommand, SerialCommand, +}; const INTRO_MESSAGE_LUA: &str = include_str!("../../lua/intro.lua"); const NEOVIM_REQUIRED_VERSION: &str = "0.9.2"; pub struct NeovimRuntime { - runtime: Option, + runtime: Runtime, + join_set: JoinSet<()>, } fn neovim_instance() -> Result { @@ -77,7 +83,11 @@ pub async fn show_error_message( nvim.echo(prepared_lines, true, vec![]).await } -async fn launch(handler: NeovimHandler, grid_size: Option>) -> Result { +async fn launch( + handler: NeovimHandler, + grid_size: Option>, + join_set: &mut JoinSet<()>, +) -> Result { let neovim_instance = neovim_instance()?; let session = NeovimSession::new(neovim_instance, handler) @@ -110,7 +120,7 @@ async fn launch(handler: NeovimHandler, grid_size: Option>) -> Res setup_neovide_specific_state(&session.neovim, should_handle_clipboard, &api_information) .await?; - start_ui_command_handler(session.neovim.clone(), &api_information); + start_ui_command_handler(session.neovim.clone(), &api_information, join_set); SETTINGS.read_initial_values(&session.neovim).await?; let mut options = UiAttachOptions::new(); @@ -148,12 +158,17 @@ async fn run(session: NeovimSession, proxy: EventLoopProxy) { proxy.send_event(UserEvent::NeovimExited).ok(); } +async fn wait(join_set: &mut JoinSet<()>) { + while join_set.join_next().await.is_some() {} +} + impl NeovimRuntime { pub fn new() -> Result { let runtime = Builder::new_multi_thread().enable_all().build()?; Ok(Self { - runtime: Some(runtime), + runtime, + join_set: JoinSet::new(), }) } @@ -163,15 +178,22 @@ impl NeovimRuntime { grid_size: Option>, ) -> Result<()> { let handler = start_editor(event_loop_proxy.clone()); - let runtime = self.runtime.as_ref().unwrap(); - let session = runtime.block_on(launch(handler, grid_size))?; - runtime.spawn(run(session, event_loop_proxy)); + let session = self + .runtime + .block_on(launch(handler, grid_size, &mut self.join_set))?; + self.join_set + .spawn_on(run(session, event_loop_proxy), self.runtime.handle()); Ok(()) } } impl Drop for NeovimRuntime { fn drop(&mut self) { - self.runtime.take().unwrap().shutdown_background(); + log::info!("Starting neovim runtime shutdown"); + let start = Instant::now(); + shutdown_ui(); + self.runtime.block_on(wait(&mut self.join_set)); + let elapsed = start.elapsed().as_millis(); + log::info!("Neovim runtime shutdown took {elapsed} ms"); } } diff --git a/src/bridge/ui_commands.rs b/src/bridge/ui_commands.rs index a87ba7e0d..56bf69c0f 100644 --- a/src/bridge/ui_commands.rs +++ b/src/bridge/ui_commands.rs @@ -5,7 +5,10 @@ use log::trace; use anyhow::{Context, Result}; use nvim_rs::{call_args, error::CallError, rpc::model::IntoVal, Neovim, Value}; use strum::AsRefStr; -use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver}; +use tokio::{ + sync::mpsc::{unbounded_channel, UnboundedReceiver}, + task::JoinSet, +}; use super::show_error_message; use crate::{ @@ -271,7 +274,7 @@ impl AsRef for UiCommand { } struct UIChannels { - sender: LoggingSender, + sender: Mutex>>, receiver: Mutex>>, } @@ -279,7 +282,7 @@ impl UIChannels { fn new() -> Self { let (sender, receiver) = unbounded_channel(); Self { - sender: LoggingSender::attach(sender, "UICommand"), + sender: Mutex::new(Some(LoggingSender::attach(sender, "UICommand"))), receiver: Mutex::new(Some(receiver)), } } @@ -289,10 +292,14 @@ lazy_static! { static ref UI_CHANNELS: UIChannels = UIChannels::new(); } -pub fn start_ui_command_handler(nvim: Neovim, api_information: &ApiInformation) { +pub fn start_ui_command_handler( + nvim: Neovim, + api_information: &ApiInformation, + join_set: &mut JoinSet<()>, +) { let (serial_tx, mut serial_rx) = unbounded_channel::(); let ui_command_nvim = nvim.clone(); - tokio::spawn(async move { + join_set.spawn(async move { let mut ui_command_receiver = UI_CHANNELS.receiver.lock().unwrap().take().unwrap(); loop { match ui_command_receiver.recv().await { @@ -316,7 +323,7 @@ pub fn start_ui_command_handler(nvim: Neovim, api_information: &Ap let has_x_buttons = api_information.version.has_version(0, 10, 0); - tokio::spawn(async move { + join_set.spawn(async move { tracy_fiber_enter!("Serial command"); loop { tracy_fiber_leave(); @@ -341,5 +348,16 @@ where T: Into, { let command: UiCommand = command.into(); - let _ = UI_CHANNELS.sender.send(command); + let _ = UI_CHANNELS + .sender + .lock() + .unwrap() + .as_ref() + .unwrap() + .send(command); +} + +pub fn shutdown_ui() { + *UI_CHANNELS.sender.lock().unwrap() = None; + log::info!("The UI subsystem has been shut down") } diff --git a/src/window/mod.rs b/src/window/mod.rs index 03462fe0f..1cf4cc99d 100644 --- a/src/window/mod.rs +++ b/src/window/mod.rs @@ -314,7 +314,7 @@ pub fn main_loop( let mtm = MainThreadMarker::new().expect("must be on the main thread"); macos::Menu::new(mtm) }; - event_loop.run(move |e, window_target| { + let res = event_loop.run(move |e, window_target| { #[cfg(target_os = "macos")] menu.ensure_menu_added(&e); @@ -327,7 +327,8 @@ pub fn main_loop( _ => window_target .set_control_flow(update_loop.step(window_wrapper.as_mut().unwrap(), e)), } - }) + }); + res } pub fn load_icon() -> Icon { From 686d8a1231895ce8de648436b1d0752e885101f2 Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Thu, 4 Apr 2024 03:06:13 +0300 Subject: [PATCH 6/9] Force quit after some timeout, when the neovim process quits --- src/bridge/mod.rs | 40 +++++++++++++++++++++++++++++++--------- src/bridge/session.rs | 21 ++++++++++++++------- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/bridge/mod.rs b/src/bridge/mod.rs index 392b93064..7ab0930dd 100644 --- a/src/bridge/mod.rs +++ b/src/bridge/mod.rs @@ -9,13 +9,19 @@ mod ui_commands; use anyhow::{bail, Context, Result}; use itertools::Itertools; -use log::{error, info}; +use log::info; use nvim_rs::{error::CallError, Neovim, UiAttachOptions, Value}; use rmpv::Utf8String; -use std::{io::Error, ops::Add, time::Instant}; +use std::{ + io::Error, + ops::Add, + time::{Duration, Instant}, +}; use tokio::{ runtime::{Builder, Runtime}, + select, task::JoinSet, + time::sleep, }; use winit::event_loop::EventLoopProxy; @@ -145,15 +151,31 @@ async fn launch( } async fn run(session: NeovimSession, proxy: EventLoopProxy) { - match session.io_handle.await { - Err(join_error) => error!("Error joining IO loop: '{}'", join_error), - Ok(Err(error)) => { - if !error.is_channel_closed() { - error!("Error: '{}'", error); + let mut session = session; + + if let Some(process) = session.neovim_process.as_mut() { + let neovim_exited = select! { + _ = &mut session.io_handle => false, + _ = process.wait() => true, + }; + + // We primarily wait for the stdio to finish, but due to bugs, + // for example, this one in in Neovim 0.9.5 + // https://github.com/neovim/neovim/issues/26743 + // it does not always finish. + // So wait for some additional time, both to make the bug obvious and to prevent incomplete + // data. + if neovim_exited { + let sleep = sleep(Duration::from_millis(2000)); + tokio::pin!(sleep); + select! { + _ = session.io_handle => {} + _ = &mut sleep => {} } } - Ok(Ok(())) => {} - }; + } else { + session.io_handle.await.ok(); + } log::info!("Neovim has quit"); proxy.send_event(UserEvent::NeovimExited).ok(); } diff --git a/src/bridge/session.rs b/src/bridge/session.rs index 2793ce8b6..1ffb4f50b 100644 --- a/src/bridge/session.rs +++ b/src/bridge/session.rs @@ -12,7 +12,7 @@ use nvim_rs::{error::LoopError, neovim::Neovim, Handler}; use tokio::{ io::{split, AsyncRead, AsyncWrite}, net::TcpStream, - process::Command, + process::{Child, Command}, spawn, task::JoinHandle, }; @@ -26,6 +26,7 @@ type BoxedWriter = Box; pub struct NeovimSession { pub neovim: Neovim, pub io_handle: JoinHandle>>, + pub neovim_process: Option, } #[cfg(debug_assertions)] @@ -42,12 +43,16 @@ impl NeovimSession { instance: NeovimInstance, handler: impl Handler, ) -> Result { - let (reader, writer) = instance.connect().await?; + let (reader, writer, neovim_process) = instance.connect().await?; let (neovim, io) = Neovim::::new(reader.compat(), Box::new(writer.compat_write()), handler); let io_handle = spawn(io); - Ok(Self { neovim, io_handle }) + Ok(Self { + neovim, + io_handle, + neovim_process, + }) } } @@ -66,14 +71,16 @@ pub enum NeovimInstance { } impl NeovimInstance { - async fn connect(self) -> Result<(BoxedReader, BoxedWriter)> { + async fn connect(self) -> Result<(BoxedReader, BoxedWriter, Option)> { match self { NeovimInstance::Embedded(cmd) => Self::spawn_process(cmd).await, - NeovimInstance::Server { address } => Self::connect_to_server(address).await, + NeovimInstance::Server { address } => Self::connect_to_server(address) + .await + .map(|(reader, writer)| (reader, writer, None)), } } - async fn spawn_process(mut cmd: Command) -> Result<(BoxedReader, BoxedWriter)> { + async fn spawn_process(mut cmd: Command) -> Result<(BoxedReader, BoxedWriter, Option)> { let mut child = cmd.stdin(Stdio::piped()).stdout(Stdio::piped()).spawn()?; let reader = Box::new( child @@ -88,7 +95,7 @@ impl NeovimInstance { .ok_or_else(|| Error::new(ErrorKind::Other, "Can't open stdin"))?, ); - Ok((reader, writer)) + Ok((reader, writer, Some(child))) } async fn connect_to_server(address: String) -> Result<(BoxedReader, BoxedWriter)> { From 522375b2cdd57e424fdfa2775f394c58228608ee Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Thu, 4 Apr 2024 03:31:53 +0300 Subject: [PATCH 7/9] Add some logging --- src/bridge/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/bridge/mod.rs b/src/bridge/mod.rs index 7ab0930dd..5644d240c 100644 --- a/src/bridge/mod.rs +++ b/src/bridge/mod.rs @@ -156,7 +156,10 @@ async fn run(session: NeovimSession, proxy: EventLoopProxy) { if let Some(process) = session.neovim_process.as_mut() { let neovim_exited = select! { _ = &mut session.io_handle => false, - _ = process.wait() => true, + _ = process.wait() => { + log::info!("The Neovim process quit before the IO stream, waiting two seconds"); + true + } }; // We primarily wait for the stdio to finish, but due to bugs, @@ -170,7 +173,9 @@ async fn run(session: NeovimSession, proxy: EventLoopProxy) { tokio::pin!(sleep); select! { _ = session.io_handle => {} - _ = &mut sleep => {} + _ = &mut sleep => { + log::info!("The IO stream was never closed, forcing Neovide to exit"); + } } } } else { From 07f00c64cfc461394b65a45e5e6b495a9aaf0277 Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Fri, 5 Apr 2024 20:53:44 +0300 Subject: [PATCH 8/9] Clean up the io stream wait code --- src/bridge/mod.rs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/bridge/mod.rs b/src/bridge/mod.rs index 5644d240c..843abeccb 100644 --- a/src/bridge/mod.rs +++ b/src/bridge/mod.rs @@ -21,7 +21,7 @@ use tokio::{ runtime::{Builder, Runtime}, select, task::JoinSet, - time::sleep, + time::timeout, }; use winit::event_loop::EventLoopProxy; @@ -154,30 +154,24 @@ async fn run(session: NeovimSession, proxy: EventLoopProxy) { let mut session = session; if let Some(process) = session.neovim_process.as_mut() { - let neovim_exited = select! { - _ = &mut session.io_handle => false, - _ = process.wait() => { - log::info!("The Neovim process quit before the IO stream, waiting two seconds"); - true - } - }; - // We primarily wait for the stdio to finish, but due to bugs, // for example, this one in in Neovim 0.9.5 // https://github.com/neovim/neovim/issues/26743 // it does not always finish. // So wait for some additional time, both to make the bug obvious and to prevent incomplete // data. - if neovim_exited { - let sleep = sleep(Duration::from_millis(2000)); - tokio::pin!(sleep); - select! { - _ = session.io_handle => {} - _ = &mut sleep => { + select! { + _ = &mut session.io_handle => {} + _ = process.wait() => { + log::info!("The Neovim process quit before the IO stream, waiting two seconds"); + if timeout(Duration::from_millis(2000), session.io_handle) + .await + .is_err() + { log::info!("The IO stream was never closed, forcing Neovide to exit"); } } - } + }; } else { session.io_handle.await.ok(); } From 35582e89abd7a113c64f6be0ce4330b4a273d945 Mon Sep 17 00:00:00 2001 From: Fred Sundvik Date: Fri, 5 Apr 2024 23:30:52 +0300 Subject: [PATCH 9/9] Simplify the code, let the runtime shutdown the UICommand tasks --- src/bridge/mod.rs | 47 +++++---------------------------- src/bridge/ui_commands.rs | 55 +++++++++------------------------------ src/channel_utils.rs | 2 +- 3 files changed, 21 insertions(+), 83 deletions(-) diff --git a/src/bridge/mod.rs b/src/bridge/mod.rs index 843abeccb..9889fad0f 100644 --- a/src/bridge/mod.rs +++ b/src/bridge/mod.rs @@ -12,15 +12,10 @@ use itertools::Itertools; use log::info; use nvim_rs::{error::CallError, Neovim, UiAttachOptions, Value}; use rmpv::Utf8String; -use std::{ - io::Error, - ops::Add, - time::{Duration, Instant}, -}; +use std::{io::Error, ops::Add, time::Duration}; use tokio::{ runtime::{Builder, Runtime}, select, - task::JoinSet, time::timeout, }; use winit::event_loop::EventLoopProxy; @@ -37,16 +32,13 @@ pub use api_info::*; pub use command::create_nvim_command; pub use events::*; pub use session::NeovimWriter; -pub use ui_commands::{ - send_ui, shutdown_ui, start_ui_command_handler, ParallelCommand, SerialCommand, -}; +pub use ui_commands::{send_ui, start_ui_command_handler, ParallelCommand, SerialCommand}; const INTRO_MESSAGE_LUA: &str = include_str!("../../lua/intro.lua"); const NEOVIM_REQUIRED_VERSION: &str = "0.9.2"; pub struct NeovimRuntime { runtime: Runtime, - join_set: JoinSet<()>, } fn neovim_instance() -> Result { @@ -89,11 +81,7 @@ pub async fn show_error_message( nvim.echo(prepared_lines, true, vec![]).await } -async fn launch( - handler: NeovimHandler, - grid_size: Option>, - join_set: &mut JoinSet<()>, -) -> Result { +async fn launch(handler: NeovimHandler, grid_size: Option>) -> Result { let neovim_instance = neovim_instance()?; let session = NeovimSession::new(neovim_instance, handler) @@ -126,7 +114,7 @@ async fn launch( setup_neovide_specific_state(&session.neovim, should_handle_clipboard, &api_information) .await?; - start_ui_command_handler(session.neovim.clone(), &api_information, join_set); + start_ui_command_handler(session.neovim.clone(), &api_information); SETTINGS.read_initial_values(&session.neovim).await?; let mut options = UiAttachOptions::new(); @@ -179,18 +167,11 @@ async fn run(session: NeovimSession, proxy: EventLoopProxy) { proxy.send_event(UserEvent::NeovimExited).ok(); } -async fn wait(join_set: &mut JoinSet<()>) { - while join_set.join_next().await.is_some() {} -} - impl NeovimRuntime { pub fn new() -> Result { let runtime = Builder::new_multi_thread().enable_all().build()?; - Ok(Self { - runtime, - join_set: JoinSet::new(), - }) + Ok(Self { runtime }) } pub fn launch( @@ -199,22 +180,8 @@ impl NeovimRuntime { grid_size: Option>, ) -> Result<()> { let handler = start_editor(event_loop_proxy.clone()); - let session = self - .runtime - .block_on(launch(handler, grid_size, &mut self.join_set))?; - self.join_set - .spawn_on(run(session, event_loop_proxy), self.runtime.handle()); + let session = self.runtime.block_on(launch(handler, grid_size))?; + self.runtime.spawn(run(session, event_loop_proxy)); Ok(()) } } - -impl Drop for NeovimRuntime { - fn drop(&mut self) { - log::info!("Starting neovim runtime shutdown"); - let start = Instant::now(); - shutdown_ui(); - self.runtime.block_on(wait(&mut self.join_set)); - let elapsed = start.elapsed().as_millis(); - log::info!("Neovim runtime shutdown took {elapsed} ms"); - } -} diff --git a/src/bridge/ui_commands.rs b/src/bridge/ui_commands.rs index 56bf69c0f..632ee2518 100644 --- a/src/bridge/ui_commands.rs +++ b/src/bridge/ui_commands.rs @@ -1,14 +1,11 @@ -use std::sync::Mutex; +use std::sync::OnceLock; use log::trace; use anyhow::{Context, Result}; use nvim_rs::{call_args, error::CallError, rpc::model::IntoVal, Neovim, Value}; use strum::AsRefStr; -use tokio::{ - sync::mpsc::{unbounded_channel, UnboundedReceiver}, - task::JoinSet, -}; +use tokio::sync::mpsc::unbounded_channel; use super::show_error_message; use crate::{ @@ -273,34 +270,16 @@ impl AsRef for UiCommand { } } -struct UIChannels { - sender: Mutex>>, - receiver: Mutex>>, -} - -impl UIChannels { - fn new() -> Self { - let (sender, receiver) = unbounded_channel(); - Self { - sender: Mutex::new(Some(LoggingSender::attach(sender, "UICommand"))), - receiver: Mutex::new(Some(receiver)), - } - } -} - -lazy_static! { - static ref UI_CHANNELS: UIChannels = UIChannels::new(); -} +static UI_COMMAND_CHANNEL: OnceLock> = OnceLock::new(); -pub fn start_ui_command_handler( - nvim: Neovim, - api_information: &ApiInformation, - join_set: &mut JoinSet<()>, -) { +pub fn start_ui_command_handler(nvim: Neovim, api_information: &ApiInformation) { let (serial_tx, mut serial_rx) = unbounded_channel::(); let ui_command_nvim = nvim.clone(); - join_set.spawn(async move { - let mut ui_command_receiver = UI_CHANNELS.receiver.lock().unwrap().take().unwrap(); + let (sender, mut ui_command_receiver) = unbounded_channel(); + UI_COMMAND_CHANNEL + .set(LoggingSender::attach(sender, "UIComand")) + .expect("The UI command channel is already created"); + tokio::spawn(async move { loop { match ui_command_receiver.recv().await { Some(UiCommand::Serial(serial_command)) => { @@ -323,7 +302,7 @@ pub fn start_ui_command_handler( let has_x_buttons = api_information.version.has_version(0, 10, 0); - join_set.spawn(async move { + tokio::spawn(async move { tracy_fiber_enter!("Serial command"); loop { tracy_fiber_leave(); @@ -348,16 +327,8 @@ where T: Into, { let command: UiCommand = command.into(); - let _ = UI_CHANNELS - .sender - .lock() - .unwrap() - .as_ref() - .unwrap() + let _ = UI_COMMAND_CHANNEL + .get() + .expect("The UI command channel has not been initialized") .send(command); } - -pub fn shutdown_ui() { - *UI_CHANNELS.sender.lock().unwrap() = None; - log::info!("The UI subsystem has been shut down") -} diff --git a/src/channel_utils.rs b/src/channel_utils.rs index a34bf4775..d0b9fc5c1 100644 --- a/src/channel_utils.rs +++ b/src/channel_utils.rs @@ -5,7 +5,7 @@ use tokio::sync::mpsc::{error::SendError as TokioSendError, UnboundedSender}; use crate::profiling::tracy_dynamic_zone; -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct LoggingSender where T: Debug + AsRef,