diff --git a/src/bridge/mod.rs b/src/bridge/mod.rs index 74c45ee12..9889fad0f 100644 --- a/src/bridge/mod.rs +++ b/src/bridge/mod.rs @@ -9,16 +9,20 @@ 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}; -use tokio::runtime::{Builder, Runtime}; +use std::{io::Error, ops::Add, time::Duration}; +use tokio::{ + runtime::{Builder, Runtime}, + select, + time::timeout, +}; 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::*, units::GridSize, + window::UserEvent, }; pub use handler::NeovimHandler; use session::{NeovimInstance, NeovimSession}; @@ -34,7 +38,7 @@ 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, } fn neovim_instance() -> Result { @@ -134,26 +138,40 @@ async fn launch(handler: NeovimHandler, grid_size: Option>) -> Res res.map(|()| session) } -async fn run(session: NeovimSession) { - 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); +async fn run(session: NeovimSession, proxy: EventLoopProxy) { + let mut session = session; + + if let Some(process) = session.neovim_process.as_mut() { + // 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. + 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"); + } } - } - Ok(Ok(())) => {} - }; - RUNNING_TRACKER.quit("neovim processed failed"); + }; + } else { + session.io_handle.await.ok(); + } + log::info!("Neovim has quit"); + proxy.send_event(UserEvent::NeovimExited).ok(); } impl NeovimRuntime { pub fn new() -> Result { let runtime = Builder::new_multi_thread().enable_all().build()?; - Ok(Self { - runtime: Some(runtime), - }) + Ok(Self { runtime }) } pub fn launch( @@ -161,16 +179,9 @@ impl NeovimRuntime { event_loop_proxy: EventLoopProxy, grid_size: Option>, ) -> Result<()> { - let handler = start_editor(event_loop_proxy); - let runtime = self.runtime.as_ref().unwrap(); - let session = runtime.block_on(launch(handler, grid_size))?; - runtime.spawn(run(session)); + let handler = start_editor(event_loop_proxy.clone()); + 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) { - self.runtime.take().unwrap().shutdown_background(); - } -} 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)> { diff --git a/src/bridge/ui_commands.rs b/src/bridge/ui_commands.rs index cd278508b..632ee2518 100644 --- a/src/bridge/ui_commands.rs +++ b/src/bridge/ui_commands.rs @@ -1,17 +1,16 @@ -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}; +use tokio::sync::mpsc::unbounded_channel; 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, }; @@ -271,31 +270,17 @@ impl AsRef for UiCommand { } } -struct UIChannels { - sender: LoggingSender, - receiver: Mutex>>, -} - -impl UIChannels { - fn new() -> Self { - let (sender, receiver) = unbounded_channel(); - Self { - sender: 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) { let (serial_tx, mut serial_rx) = unbounded_channel::(); let ui_command_nvim = nvim.clone(); + 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 { - 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 +294,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 +315,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"); }); } @@ -344,5 +327,8 @@ where T: Into, { let command: UiCommand = command.into(); - let _ = UI_CHANNELS.sender.send(command); + let _ = UI_COMMAND_CHANNEL + .get() + .expect("The UI command channel has not been initialized") + .send(command); } 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, 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/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 871db0b74..1cf4cc99d 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 { @@ -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,26 +307,28 @@ 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 = { 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); - if e == Event::LoopExiting { - return; - } - if !RUNNING_TRACKER.is_running() { - 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 => window_wrapper = None, + Event::UserEvent(UserEvent::NeovimExited) => { + save_window_size(window_wrapper.as_ref().unwrap()); + window_target.exit(); + } + _ => window_target + .set_control_flow(update_loop.step(window_wrapper.as_mut().unwrap(), e)), } - }) + }); + res } pub fn load_icon() -> Icon { 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) {