From 39758e2b9a63a722fea7187e4b0f4ae15873d3e9 Mon Sep 17 00:00:00 2001 From: Federico Terzi Date: Tue, 18 May 2021 21:04:23 +0200 Subject: [PATCH] feat(core): improve exit handling --- espanso/src/cli/daemon/ipc.rs | 6 +- espanso/src/cli/daemon/mod.rs | 32 +++++----- espanso/src/cli/worker/config.rs | 3 +- espanso/src/cli/worker/engine/mod.rs | 6 +- espanso/src/cli/worker/engine/source/exit.rs | 2 +- espanso/src/cli/worker/mod.rs | 37 ++++++++---- espanso/src/engine/event/mod.rs | 4 +- espanso/src/engine/mod.rs | 10 ++-- .../engine/process/middleware/context_menu.rs | 60 +++++++++++-------- espanso/src/engine/process/middleware/exit.rs | 6 +- espanso/src/exit_code.rs | 27 +++++++++ espanso/src/main.rs | 1 + 12 files changed, 122 insertions(+), 72 deletions(-) create mode 100644 espanso/src/exit_code.rs diff --git a/espanso/src/cli/daemon/ipc.rs b/espanso/src/cli/daemon/ipc.rs index efee57d..c8b91e5 100644 --- a/espanso/src/cli/daemon/ipc.rs +++ b/espanso/src/cli/daemon/ipc.rs @@ -23,9 +23,7 @@ use anyhow::Result; use crossbeam::channel::{Sender}; use log::{error, warn}; -use crate::ipc::IPCEvent; - -use super::ExitCode; +use crate::{exit_code::DAEMON_SUCCESS, ipc::IPCEvent}; pub fn initialize_and_spawn(runtime_dir: &Path, exit_notify: Sender) -> Result<()> { let receiver = crate::ipc::spawn_daemon_ipc_server(runtime_dir)?; @@ -37,7 +35,7 @@ pub fn initialize_and_spawn(runtime_dir: &Path, exit_notify: Sender) -> Res Ok(event) => { match event { IPCEvent::Exit => { - if let Err(err) = exit_notify.send(ExitCode::Success as i32) { + if let Err(err) = exit_notify.send(DAEMON_SUCCESS) { error!("experienced error while sending exit signal from daemon ipc handler: {}", err); } }, diff --git a/espanso/src/cli/daemon/mod.rs b/espanso/src/cli/daemon/mod.rs index e35427e..7adc2b8 100644 --- a/espanso/src/cli/daemon/mod.rs +++ b/espanso/src/cli/daemon/mod.rs @@ -27,20 +27,12 @@ use espanso_ipc::IPCClient; use espanso_path::Paths; use log::{error, info, warn}; -use crate::{ - ipc::{create_ipc_client_to_worker, IPCEvent}, - lock::{acquire_daemon_lock, acquire_worker_lock}, -}; +use crate::{exit_code::{DAEMON_ALREADY_RUNNING, DAEMON_GENERAL_ERROR, DAEMON_SUCCESS, WORKER_EXIT_ALL_PROCESSES, WORKER_SUCCESS}, ipc::{create_ipc_client_to_worker, IPCEvent}, lock::{acquire_daemon_lock, acquire_worker_lock}}; use super::{CliModule, CliModuleArgs}; mod ipc; -pub enum ExitCode { - Success = 0, - ExitCodeUnwrapError = 100, -} - pub fn new() -> CliModule { #[allow(clippy::needless_update)] CliModule { @@ -63,7 +55,7 @@ fn daemon_main(args: CliModuleArgs) -> i32 { let lock_file = acquire_daemon_lock(&paths.runtime); if lock_file.is_none() { error!("daemon is already running!"); - return 1; + return DAEMON_ALREADY_RUNNING; } // TODO: we might need to check preconditions: accessibility on macOS, presence of binaries on Linux, etc @@ -87,18 +79,26 @@ fn daemon_main(args: CliModuleArgs) -> i32 { // TODO: start file watcher thread - let mut exit_code: i32 = ExitCode::Success as i32; + let mut exit_code: i32 = DAEMON_SUCCESS; loop { select! { recv(exit_signal) -> code => { match code { Ok(code) => { - exit_code = code + match code { + WORKER_EXIT_ALL_PROCESSES => { + info!("worker requested a general exit, quitting the daemon"); + } + _ => { + error!("received unexpected exit code from worker {}, exiting", code); + exit_code = code + } + } }, Err(err) => { error!("received error when unwrapping exit_code: {}", err); - exit_code = ExitCode::ExitCodeUnwrapError as i32; + exit_code = DAEMON_GENERAL_ERROR; }, } break; @@ -177,11 +177,7 @@ fn spawn_worker(paths: &Paths, exit_notify: Sender) { let result = child.wait(); if let Ok(status) = result { if let Some(code) = status.code() { - if code != 0 { - error!( - "worker process exited with non-zero code: {}, exiting", - code - ); + if code != WORKER_SUCCESS { exit_notify .send(code) .expect("unable to forward worker exit code"); diff --git a/espanso/src/cli/worker/config.rs b/espanso/src/cli/worker/config.rs index 15d654f..d888a06 100644 --- a/espanso/src/cli/worker/config.rs +++ b/espanso/src/cli/worker/config.rs @@ -18,8 +18,7 @@ */ use std::{ - cell::RefCell, - collections::{HashMap, HashSet}, + collections::{HashSet}, }; use crate::engine::{dispatch::ModeProvider, process::MatchFilter}; diff --git a/espanso/src/cli/worker/engine/mod.rs b/espanso/src/cli/worker/engine/mod.rs index f8e96a9..d7c62bc 100644 --- a/espanso/src/cli/worker/engine/mod.rs +++ b/espanso/src/cli/worker/engine/mod.rs @@ -48,7 +48,7 @@ pub fn initialize_and_spawn( ui_remote: Box, exit_signal: Receiver<()>, ui_event_receiver: Receiver, -) -> Result> { +) -> Result> { let handle = std::thread::Builder::new() .name("engine thread".to_string()) .spawn(move || { @@ -154,10 +154,12 @@ pub fn initialize_and_spawn( ); let mut engine = crate::engine::Engine::new(&funnel, &mut processor, &dispatcher); - engine.run(); + let exit_all_processes = engine.run(); info!("engine eventloop has terminated, propagating exit event..."); ui_remote.exit(); + + exit_all_processes })?; Ok(handle) diff --git a/espanso/src/cli/worker/engine/source/exit.rs b/espanso/src/cli/worker/engine/source/exit.rs index 3b1c351..cd0dae9 100644 --- a/espanso/src/cli/worker/engine/source/exit.rs +++ b/espanso/src/cli/worker/engine/source/exit.rs @@ -48,7 +48,7 @@ impl<'a> funnel::Source<'a> for ExitSource<'a> { .expect("unable to select data from ExitSource receiver"); Event { source_id: self.sequencer.next_id(), - etype: EventType::ExitRequested, + etype: EventType::ExitRequested(false), } } } \ No newline at end of file diff --git a/espanso/src/cli/worker/mod.rs b/espanso/src/cli/worker/mod.rs index ff1f9ee..86c6702 100644 --- a/espanso/src/cli/worker/mod.rs +++ b/espanso/src/cli/worker/mod.rs @@ -20,7 +20,7 @@ use crossbeam::channel::unbounded; use log::{error, info}; -use crate::lock::acquire_worker_lock; +use crate::{exit_code::{WORKER_ALREADY_RUNNING, WORKER_EXIT_ALL_PROCESSES, WORKER_GENERAL_ERROR, WORKER_SUCCESS}, lock::acquire_worker_lock}; use self::ui::util::convert_icon_paths_to_tray_vec; @@ -51,7 +51,7 @@ fn worker_main(args: CliModuleArgs) -> i32 { let lock_file = acquire_worker_lock(&paths.runtime); if lock_file.is_none() { error!("worker is already running!"); - return 1; + return WORKER_ALREADY_RUNNING; } let config_store = args @@ -83,7 +83,7 @@ fn worker_main(args: CliModuleArgs) -> i32 { let (engine_ui_event_sender, engine_ui_event_receiver) = unbounded(); // Initialize the engine on another thread and start it - engine::initialize_and_spawn( + let engine_handle = engine::initialize_and_spawn( paths.clone(), config_store, match_store, @@ -98,13 +98,28 @@ fn worker_main(args: CliModuleArgs) -> i32 { ipc::initialize_and_spawn(&paths.runtime, engine_exit_notify) .expect("unable to initialize IPC server"); - eventloop.run(Box::new(move |event| { - if let Err(error) = engine_ui_event_sender.send(event) { - error!("unable to send UIEvent to engine: {}", error); - } - })).expect("unable to run main eventloop"); + eventloop + .run(Box::new(move |event| { + if let Err(error) = engine_ui_event_sender.send(event) { + error!("unable to send UIEvent to engine: {}", error); + } + })) + .expect("unable to run main eventloop"); - info!("exiting worker process..."); - - 0 + info!("waiting for engine exit mode..."); + match engine_handle.join() { + Ok(exit_all_processes) => { + if exit_all_processes { + info!("exiting worker process and daemon..."); + return WORKER_EXIT_ALL_PROCESSES; + } else { + info!("exiting worker process..."); + return WORKER_SUCCESS; + } + } + Err(err) => { + error!("unable to read engine exit mode: {:?}", err); + return WORKER_GENERAL_ERROR; + } + } } diff --git a/espanso/src/engine/event/mod.rs b/espanso/src/engine/event/mod.rs index aadbd15..8fe6d1a 100644 --- a/espanso/src/engine/event/mod.rs +++ b/espanso/src/engine/event/mod.rs @@ -48,8 +48,8 @@ impl Event { pub enum EventType { NOOP, ProcessingError(String), - ExitRequested, - Exit, + ExitRequested(bool), // If true, exit also the daemon process and not just the worker + Exit(bool), // Inputs Keyboard(input::KeyboardEvent), diff --git a/espanso/src/engine/mod.rs b/espanso/src/engine/mod.rs index 26deadf..01b81a0 100644 --- a/espanso/src/engine/mod.rs +++ b/espanso/src/engine/mod.rs @@ -41,15 +41,15 @@ impl <'a> Engine<'a> { } } - pub fn run(&mut self) { - 'main: loop { + pub fn run(&mut self) -> bool { + loop { match self.funnel.receive() { FunnelResult::Event(event) => { let processed_events = self.processor.process(event); for event in processed_events { - if let EventType::Exit = &event.etype { + if let EventType::Exit(exit_all_processes) = &event.etype { debug!("exit event received, exiting engine"); - break 'main; + return *exit_all_processes; } self.dispatcher.dispatch(event); @@ -57,7 +57,7 @@ impl <'a> Engine<'a> { } FunnelResult::EndOfStream => { debug!("end of stream received"); - break; + return false; } } } diff --git a/espanso/src/engine/process/middleware/context_menu.rs b/espanso/src/engine/process/middleware/context_menu.rs index b85d04d..9a91d85 100644 --- a/espanso/src/engine/process/middleware/context_menu.rs +++ b/espanso/src/engine/process/middleware/context_menu.rs @@ -18,10 +18,14 @@ */ use super::super::Middleware; -use crate::engine::{event::{Event, EventType, ui::{MenuItem, ShowContextMenuEvent, SimpleMenuItem}}}; +use crate::engine::event::{ + ui::{MenuItem, ShowContextMenuEvent, SimpleMenuItem}, + Event, EventType, +}; -pub struct ContextMenuMiddleware { -} +const CONTEXT_ITEM_EXIT: u32 = 0; + +pub struct ContextMenuMiddleware {} impl ContextMenuMiddleware { pub fn new() -> Self { @@ -35,30 +39,38 @@ impl Middleware for ContextMenuMiddleware { } fn next(&self, event: Event, _: &mut dyn FnMut(Event)) -> Event { - if let EventType::TrayIconClicked = event.etype { - // TODO: fetch top matches for the active config to be added + match &event.etype { + EventType::TrayIconClicked => { + // TODO: fetch top matches for the active config to be added - // TODO: my idea is to use a set of reserved u32 ids for built-in - // actions such as Exit, Open Editor etc - // then we need some u32 for the matches, so we need to create - // a mapping structure match_id <-> context-menu-id - return Event::caused_by( - event.source_id, - EventType::ShowContextMenu(ShowContextMenuEvent { - // TODO: add actual entries - items: vec![ - MenuItem::Simple(SimpleMenuItem { - id: 0, + // TODO: my idea is to use a set of reserved u32 ids for built-in + // actions such as Exit, Open Editor etc + // then we need some u32 for the matches, so we need to create + // a mapping structure match_id <-> context-menu-id + return Event::caused_by( + event.source_id, + EventType::ShowContextMenu(ShowContextMenuEvent { + // TODO: add actual entries + items: vec![MenuItem::Simple(SimpleMenuItem { + id: CONTEXT_ITEM_EXIT, label: "Exit espanso".to_string(), - }) - ] - }), - ) + })], + }), + ); + } + EventType::ContextMenuClicked(context_click_event) => { + match context_click_event.context_item_id { + CONTEXT_ITEM_EXIT => { + Event::caused_by(event.source_id, EventType::ExitRequested(true)) + } + custom => { + // TODO: handle dynamic items + todo!() + } + } + } + _ => event, } - - // TODO: handle context menu clicks - - event } } diff --git a/espanso/src/engine/process/middleware/exit.rs b/espanso/src/engine/process/middleware/exit.rs index 51e026b..177b7a7 100644 --- a/espanso/src/engine/process/middleware/exit.rs +++ b/espanso/src/engine/process/middleware/exit.rs @@ -36,9 +36,9 @@ impl Middleware for ExitMiddleware { } fn next(&self, event: Event, _: &mut dyn FnMut(Event)) -> Event { - if let EventType::ExitRequested = &event.etype { - debug!("received ExitRequested event, dispatching exit"); - return Event::caused_by(event.source_id, EventType::Exit); + if let EventType::ExitRequested(exit_all_processes) = &event.etype { + debug!("received ExitRequested event with 'exit_all_processes: {}', dispatching exit", exit_all_processes); + return Event::caused_by(event.source_id, EventType::Exit(*exit_all_processes)); } event diff --git a/espanso/src/exit_code.rs b/espanso/src/exit_code.rs new file mode 100644 index 0000000..6d4618e --- /dev/null +++ b/espanso/src/exit_code.rs @@ -0,0 +1,27 @@ +/* + * This file is part of espanso. + * + * Copyright (C) 2019-2021 Federico Terzi + * + * espanso is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * espanso is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with espanso. If not, see . + */ + +pub const WORKER_SUCCESS: i32 = 0; +pub const WORKER_ALREADY_RUNNING: i32 = 1; +pub const WORKER_GENERAL_ERROR: i32 = 2; +pub const WORKER_EXIT_ALL_PROCESSES: i32 = 101; + +pub const DAEMON_SUCCESS: i32 = 0; +pub const DAEMON_ALREADY_RUNNING: i32 = 1; +pub const DAEMON_GENERAL_ERROR: i32 = 2; \ No newline at end of file diff --git a/espanso/src/main.rs b/espanso/src/main.rs index 0e3ff4b..9f009da 100644 --- a/espanso/src/main.rs +++ b/espanso/src/main.rs @@ -37,6 +37,7 @@ use crate::cli::LogMode; mod cli; mod engine; +mod exit_code; mod gui; mod ipc; mod lock;