From 1307ff8dd9ec17f10052c08cfec20ec5d9eb02c3 Mon Sep 17 00:00:00 2001 From: Kinan <104761667+kibibytium@users.noreply.github.com> Date: Fri, 3 Jan 2025 14:35:24 +0100 Subject: [PATCH] ensure background process is terminated either immediately or after importing current chunk. closes #8198 Co-authored-by: sug --- .../facades/NativeMailImportFacade.json | 5 ++ packages/node-mimimi/src/importer_api.rs | 9 ++- packages/node-mimimi/src/logging/console.rs | 67 ++++++++++++------- packages/node-mimimi/src/logging/logger.rs | 14 ++-- src/common/desktop/DesktopMain.ts | 1 - src/common/desktop/DesktopWindowManager.ts | 8 ++- src/common/desktop/ipc/RemoteBridge.ts | 4 ++ .../mailimport/DesktopMailImportFacade.ts | 6 +- .../generatedipc/DesktopGlobalDispatcher.ts | 5 ++ .../generatedipc/NativeMailImportFacade.ts | 2 + ...NativeMailImportFacadeReceiveDispatcher.ts | 4 ++ .../NativeMailImportFacadeSendDispatcher.ts | 3 + 12 files changed, 94 insertions(+), 34 deletions(-) diff --git a/ipc-schema/facades/NativeMailImportFacade.json b/ipc-schema/facades/NativeMailImportFacade.json index df2573ba0296..af223b5b99ca 100644 --- a/ipc-schema/facades/NativeMailImportFacade.json +++ b/ipc-schema/facades/NativeMailImportFacade.json @@ -64,6 +64,11 @@ } ], "ret": "void" + }, + "deinitLogger": { + "doc": "", + "arg": [], + "ret": "void" } } } diff --git a/packages/node-mimimi/src/importer_api.rs b/packages/node-mimimi/src/importer_api.rs index f64974618cda..08b1f43a3272 100644 --- a/packages/node-mimimi/src/importer_api.rs +++ b/packages/node-mimimi/src/importer_api.rs @@ -202,9 +202,14 @@ impl ImporterApi { } #[napi] - pub fn init_log(env: Env) { + pub unsafe fn init_log(env: Env) { // this is in a separate fn because Env isn't Send, so can't be used in async fn. - crate::logging::console::Console::init(env); + crate::logging::console::Console::init(env) + } + + #[napi] + pub unsafe fn deinit_log() { + crate::logging::console::Console::deinit(); } } diff --git a/packages/node-mimimi/src/logging/console.rs b/packages/node-mimimi/src/logging/console.rs index 1d05a559f5cb..cbd56df7a550 100644 --- a/packages/node-mimimi/src/logging/console.rs +++ b/packages/node-mimimi/src/logging/console.rs @@ -1,11 +1,12 @@ -use crate::logging::logger::{LogMessage, Logger}; +use crate::logging::logger::{LogLevel, LogMessage, Logger}; use log::{Level, LevelFilter, Log, Metadata, Record}; -use napi::Env; +use napi::sys::{napi_async_work, napi_cancel_async_work}; +use napi::{AsyncWorkPromise, Env, Task}; use std::sync::OnceLock; const TAG: &str = file!(); -pub static GLOBAL_CONSOLE: OnceLock = OnceLock::new(); +pub static mut GLOBAL_CONSOLE: OnceLock = OnceLock::new(); /// A way for the rust code to log messages to the main applications log files /// without having to deal with obtaining a reference to console each time. @@ -38,31 +39,51 @@ impl Log for Console { } impl Console { - pub fn init(env: Env) { + pub unsafe fn init(env: Env) { + if GLOBAL_CONSOLE.get().is_some() { + // some other thread already initialized the cell, we don't need to set up the logger. + return; + } let (tx, rx) = std::sync::mpsc::channel::(); let console = Console { tx }; let logger = Logger::new(rx); - let Ok(()) = GLOBAL_CONSOLE.set(console) else { - // some other thread already initialized the cell, we don't need to set up the logger. - return; - }; - - // this may be the instance set by another thread, but that's okay. - let console = GLOBAL_CONSOLE.get().expect("not initialized"); let maybe_async_task = env.spawn(logger); + match maybe_async_task { - Ok(_) => console.log( - &Record::builder() - .level(Level::Info) - .file(Some(TAG)) - .args(format_args!("{}", "spawned logger")) - .build(), - ), - Err(e) => eprintln!("failed to spawn logger: {e}"), - }; - set_panic_hook(console); - log::set_logger(console).unwrap_or_else(|e| eprintln!("failed to set logger: {e}")); - log::set_max_level(LevelFilter::Info); + Ok(_logger_task) => { + console.log( + &Record::builder() + .level(Level::Info) + .file(Some(TAG)) + .args(format_args!("{}", "spawned logger")) + .build(), + ); + + GLOBAL_CONSOLE + .set(console) + .map_err(|_| "can not set") + .unwrap(); + + let console = GLOBAL_CONSOLE.get().unwrap(); + set_panic_hook(&console); + log::set_logger(console).unwrap_or_else(|e| eprintln!("failed to set logger: {e}")); + log::set_max_level(LevelFilter::Info); + }, + Err(e) => { + eprintln!("failed to spawn logger: {e}"); + }, + } + } + pub unsafe fn deinit() { + let console = GLOBAL_CONSOLE.take().expect("cannot deinit logger before initializing"); + console + .tx + .send(LogMessage { + level: LogLevel::Finish, + message: "called deinit".to_string(), + tag: "[deinit]".to_string(), + }) + .expect("Can not send finish log message. Receiver already disconnected"); } } diff --git a/packages/node-mimimi/src/logging/logger.rs b/packages/node-mimimi/src/logging/logger.rs index 3304d173e594..ad74dae17748 100644 --- a/packages/node-mimimi/src/logging/logger.rs +++ b/packages/node-mimimi/src/logging/logger.rs @@ -1,5 +1,6 @@ use log::Level; use napi::{bindgen_prelude::*, Env, JsFunction, JsObject, JsUndefined}; +use std::sync::mpsc::RecvError; /// The part of the logging setup that receives log messages from the rust log /// {@link struct Console} and forwards them to the node environment to log. @@ -44,11 +45,14 @@ impl Task for Logger { /// runs on the libuv thread pool. fn compute(&mut self) -> Result { if let Some(rx) = &self.rx { - Ok(rx.recv().unwrap_or_else(|_| LogMessage { - level: LogLevel::Finish, - tag: "Logger".to_string(), - message: "channel closed, logger finished".to_string(), - })) + match rx.recv() { + Ok(log_message) => Ok(log_message), + Err(RecvError) => Ok(LogMessage { + level: LogLevel::Finish, + tag: "Logger".to_string(), + message: "channel closed, logger finished".to_string(), + }), + } } else { // should not happen - each Logger instance listens for exactly one message and then // gets dropped and reincarnated. diff --git a/src/common/desktop/DesktopMain.ts b/src/common/desktop/DesktopMain.ts index 15a982e04a09..e1a7b58c0c9b 100644 --- a/src/common/desktop/DesktopMain.ts +++ b/src/common/desktop/DesktopMain.ts @@ -73,7 +73,6 @@ import { AlarmScheduler } from "../calendar/date/AlarmScheduler.js" import { DesktopExternalCalendarFacade } from "./ipc/DesktopExternalCalendarFacade.js" import { customFetch } from "./net/NetAgent" import { DesktopMailImportFacade } from "./mailimport/DesktopMailImportFacade.js" -import { locator } from "../../mail-app/workerUtils/worker/WorkerLocator.js" mp() diff --git a/src/common/desktop/DesktopWindowManager.ts b/src/common/desktop/DesktopWindowManager.ts index 868794b10259..3da9e424cbc4 100644 --- a/src/common/desktop/DesktopWindowManager.ts +++ b/src/common/desktop/DesktopWindowManager.ts @@ -8,7 +8,7 @@ import type { DesktopNotifier } from "./DesktopNotifier" import { DesktopContextMenu } from "./DesktopContextMenu" import { log } from "./DesktopLog" import type { LocalShortcutManager } from "./electron-localshortcut/LocalShortcut" -import { BuildConfigKey, DesktopConfigEncKey, DesktopConfigKey } from "./config/ConfigKeys" +import { DesktopConfigEncKey, DesktopConfigKey } from "./config/ConfigKeys" import { isRectContainedInRect } from "./DesktopUtils" import { DesktopThemeFacade } from "./DesktopThemeFacade" import { ElectronExports } from "./ElectronExportTypes" @@ -108,8 +108,12 @@ export class WindowManager { await this.loadStartingBounds() const w: ApplicationWindow = await this._newWindowFactory(noAutoLogin) windows.unshift(w) - w.on("close", () => { + w.on("close", async () => { this.saveBounds(w.getBounds()) + + if (windows.length === 1) { + await this.remoteBridge.denitImportFacadeLogger(w) + } }) .on("closed", () => { w.setUserId(null) diff --git a/src/common/desktop/ipc/RemoteBridge.ts b/src/common/desktop/ipc/RemoteBridge.ts index 58dab7c06502..3aa1d2a2b7f5 100644 --- a/src/common/desktop/ipc/RemoteBridge.ts +++ b/src/common/desktop/ipc/RemoteBridge.ts @@ -71,4 +71,8 @@ export class RemoteBridge { unsubscribe(ipc: { removeHandler: (channel: string) => void }) { ipc.removeHandler(primaryIpcConfig.renderToMainEvent) } + + async denitImportFacadeLogger(win: ApplicationWindow) { + return this.dispatcherFactory(win).dispatcher.denitImportFacadeLogger() + } } diff --git a/src/common/desktop/mailimport/DesktopMailImportFacade.ts b/src/common/desktop/mailimport/DesktopMailImportFacade.ts index 796c1c6e7f61..c99e487a5039 100644 --- a/src/common/desktop/mailimport/DesktopMailImportFacade.ts +++ b/src/common/desktop/mailimport/DesktopMailImportFacade.ts @@ -19,6 +19,10 @@ export class DesktopMailImportFacade implements NativeMailImportFacade { this.configDirectory = configDirectory } + async deinitLogger() { + ImporterApi.deinitLog() + } + async importFromFiles( apiUrl: string, unencTutaCredentials: UnencryptedCredentials, @@ -48,7 +52,7 @@ export class DesktopMailImportFacade implements NativeMailImportFacade { } static async importStateCallback(mailImportFacade: MailImportFacade, callbackAction: ImportProgressAction, localState: LocalImportState) { - await mailImportFacade.onNewLocalImportMailState({ + mailImportFacade.onNewLocalImportMailState({ remoteStateId: [localState.remoteStateId.listId, localState.remoteStateId.elementId], status: localState.currentStatus, start_timestamp: localState.startTimestamp, diff --git a/src/common/native/common/generatedipc/DesktopGlobalDispatcher.ts b/src/common/native/common/generatedipc/DesktopGlobalDispatcher.ts index c5ba27a84d3d..42fd7c483001 100644 --- a/src/common/native/common/generatedipc/DesktopGlobalDispatcher.ts +++ b/src/common/native/common/generatedipc/DesktopGlobalDispatcher.ts @@ -47,6 +47,7 @@ export class DesktopGlobalDispatcher { private readonly sqlCipherFacade: SqlCipherFacadeReceiveDispatcher private readonly themeFacade: ThemeFacadeReceiveDispatcher private readonly webAuthnFacade: WebAuthnFacadeReceiveDispatcher + constructor( commonSystemFacade: CommonSystemFacade, desktopSystemFacade: DesktopSystemFacade, @@ -81,6 +82,10 @@ export class DesktopGlobalDispatcher { this.webAuthnFacade = new WebAuthnFacadeReceiveDispatcher(webAuthnFacade) } + async denitImportFacadeLogger() { + return this.nativeMailImportFacade.dispatch("deinitLogger", []) + } + async dispatch(facadeName: string, methodName: string, args: Array) { switch (facadeName) { case "CommonSystemFacade": diff --git a/src/common/native/common/generatedipc/NativeMailImportFacade.ts b/src/common/native/common/generatedipc/NativeMailImportFacade.ts index bb017c236ae3..e3a617076bf1 100644 --- a/src/common/native/common/generatedipc/NativeMailImportFacade.ts +++ b/src/common/native/common/generatedipc/NativeMailImportFacade.ts @@ -41,4 +41,6 @@ export interface NativeMailImportFacade { * resumes the import for a previously paused import */ resumeImport(apiUrl: string, unencryptedTutaCredentials: UnencryptedCredentials, importStateId: IdTuple): Promise + + deinitLogger(): Promise } diff --git a/src/common/native/common/generatedipc/NativeMailImportFacadeReceiveDispatcher.ts b/src/common/native/common/generatedipc/NativeMailImportFacadeReceiveDispatcher.ts index 2d48a406430e..517d5af475ad 100644 --- a/src/common/native/common/generatedipc/NativeMailImportFacadeReceiveDispatcher.ts +++ b/src/common/native/common/generatedipc/NativeMailImportFacadeReceiveDispatcher.ts @@ -5,6 +5,7 @@ import { NativeMailImportFacade } from "./NativeMailImportFacade.js" export class NativeMailImportFacadeReceiveDispatcher { constructor(private readonly facade: NativeMailImportFacade) {} + async dispatch(method: string, arg: Array): Promise { switch (method) { case "importFromFiles": { @@ -34,6 +35,9 @@ export class NativeMailImportFacadeReceiveDispatcher { const importStateId: IdTuple = arg[2] return this.facade.resumeImport(apiUrl, unencryptedTutaCredentials, importStateId) } + case "deinitLogger": { + return this.facade.deinitLogger() + } } } } diff --git a/src/common/native/common/generatedipc/NativeMailImportFacadeSendDispatcher.ts b/src/common/native/common/generatedipc/NativeMailImportFacadeSendDispatcher.ts index 9d817ed20e7e..466ad4ede09d 100644 --- a/src/common/native/common/generatedipc/NativeMailImportFacadeSendDispatcher.ts +++ b/src/common/native/common/generatedipc/NativeMailImportFacadeSendDispatcher.ts @@ -25,4 +25,7 @@ export class NativeMailImportFacadeSendDispatcher implements NativeMailImportFac async resumeImport(...args: Parameters) { return this.transport.invokeNative("ipc", ["NativeMailImportFacade", "resumeImport", ...args]) } + async deinitLogger(...args: Parameters) { + return this.transport.invokeNative("ipc", ["NativeMailImportFacade", "deinitLogger", ...args]) + } }