fix!: Perform a clean exit again

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: Runtime,
 }
 
 fn neovim_instance() -> Result<NeovimInstance> {
@@ -134,43 +138,50 @@ async fn launch(handler: NeovimHandler, grid_size: Option<GridSize<u32>>) -> 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<UserEvent>) {
+    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<Self, Error> {
         let runtime = Builder::new_multi_thread().enable_all().build()?;
 
-        Ok(Self {
-            runtime: Some(runtime),
-        })
+        Ok(Self { runtime })
     }
 
     pub fn launch(
         &mut self,
         event_loop_proxy: EventLoopProxy<UserEvent>,
         grid_size: Option<GridSize<u32>>,
     ) -> 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();
-    }
-}

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<dyn AsyncWrite + Send + Unpin + 'static>;
 pub struct NeovimSession {
     pub neovim: Neovim<NeovimWriter>,
     pub io_handle: JoinHandle<std::result::Result<(), Box<LoopError>>>,
+    pub neovim_process: Option<Child>,
 }
 
 #[cfg(debug_assertions)]
@@ -42,12 +43,16 @@ impl NeovimSession {
         instance: NeovimInstance,
         handler: impl Handler<Writer = NeovimWriter>,
     ) -> Result<Self> {
-        let (reader, writer) = instance.connect().await?;
+        let (reader, writer, neovim_process) = instance.connect().await?;
         let (neovim, io) =
             Neovim::<NeovimWriter>::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<Child>)> {
         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<Child>)> {
         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)> {

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<str> for UiCommand {
     }
 }
 
-struct UIChannels {
-    sender: LoggingSender<UiCommand>,
-    receiver: Mutex<Option<UnboundedReceiver<UiCommand>>>,
-}
-
-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<LoggingSender<UiCommand>> = OnceLock::new();
 
 pub fn start_ui_command_handler(nvim: Neovim<NeovimWriter>, api_information: &ApiInformation) {
     let (serial_tx, mut serial_rx) = unbounded_channel::<SerialCommand>();
     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<NeovimWriter>, 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<NeovimWriter>, 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<UiCommand>,
 {
     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);
 }

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<T>
 where
     T: Debug + AsRef<str>,

src/renderer/opengl.rs

@@ -53,7 +53,7 @@ pub struct OpenGLSkiaRenderer {
     context: PossiblyCurrentContext,
     window_surface: Surface<WindowSurface>,
     config: Config,
-    window: Window,
+    window: Option<Window>,
 }
 
 fn clamp_render_buffer_size(size: &PhysicalSize<u32>) -> PhysicalSize<u32> {
@@ -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<dyn Iterator<Item = Config> + '_>) -> Config {
     config_iterator.next().unwrap()
 }

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<AtomicBool>,
     exit_code: Arc<AtomicI32>,
 }
 
 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)
     }
 }