diff options
| -rw-r--r-- | CHANGELOG.md | 5 | ||||
| -rw-r--r-- | mingling_core/src/program.rs | 4 | ||||
| -rw-r--r-- | mingling_core/src/program/once_exec.rs | 30 | ||||
| -rw-r--r-- | mingling_core/src/program/repl_exec.rs | 4 | ||||
| -rw-r--r-- | mingling_core/src/program/single_instance.rs | 92 |
5 files changed, 112 insertions, 23 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index bc74d21..8f43976 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ 2. **\[core:comp\]** Fixed `default_completion` jumping to the next subcommand level on partial input (e.g. typing `b` for `bind` would skip `bind` and directly suggest third-level commands `add`/`ls`/`rm`). Now if the last input word is only a partial match (`starts_with` but not equal), the current-level word is suggested instead of skipping ahead +3. **\[core\]** Replaced `OnceLock<Option<Box<dyn Any>>>` with a custom `ProgramCell` type backed by `UnsafeCell` and `AtomicBool`. The new `ProgramCell` replaces `OnceLock`'s `get_or_init` / `get` / `as_ref` calls with a direct `set` / `get_raw` / `take` API. This change: + - Eliminates the double indirection (`OnceLock<Option<Box<...>>>` → `UnsafeCell<Option<Box<...>>>`) + - Allows the program instance to be **taken** (moved out) via an `unsafe fn take()` after execution completes, enabling proper cleanup before `std::process::exit()` in `exec_and_exit` + - Is paired with corresponding simplifications in `once_exec.rs` and `repl_exec.rs` that switch from `THIS_PROGRAM.get().unwrap().as_ref()` to `THIS_PROGRAM.get_raw().unwrap()` + #### Optimizations: 1. **\[core:flag\]** Refactored the `special_argument!` and `special_arguments!` macros to replace index‑based `while` loops with iterator `position` and `drain`, improving both performance and readability. diff --git a/mingling_core/src/program.rs b/mingling_core/src/program.rs index 096f292..d14c366 100644 --- a/mingling_core/src/program.rs +++ b/mingling_core/src/program.rs @@ -118,9 +118,7 @@ where C: 'static, { THIS_PROGRAM - .get() - .unwrap() - .as_ref() + .get_raw() .unwrap() .downcast_ref::<Program<C>>() .unwrap() diff --git a/mingling_core/src/program/once_exec.rs b/mingling_core/src/program/once_exec.rs index f6eb0c0..fe90784 100644 --- a/mingling_core/src/program/once_exec.rs +++ b/mingling_core/src/program/once_exec.rs @@ -13,11 +13,9 @@ where F: FnOnce(&'static Program<C>) -> Fut + Send + Sync, Fut: Future + Send, { - THIS_PROGRAM.get_or_init(|| Some(Box::new(self))); + THIS_PROGRAM.set(Box::new(self)); let program = THIS_PROGRAM - .get() - .unwrap() - .as_ref() + .get_raw() .unwrap() .downcast_ref::<Program<C>>() .unwrap(); @@ -100,7 +98,12 @@ where where C: 'static + Send + Sync, { - std::process::exit(self.exec().await) + let exit_code = self.exec().await; + // SAFETY: exec() is synchronous — it returns only after all + // chain handlers and renderers have finished. No code still + // holds references from get_raw() at this point. + drop(unsafe { THIS_PROGRAM.take() }); + std::process::exit(exit_code) } } @@ -115,11 +118,9 @@ where C: 'static + Send + Sync, F: FnOnce(&'static Program<C>) -> R + Send + Sync, { - THIS_PROGRAM.get_or_init(|| Some(Box::new(self))); + THIS_PROGRAM.set(Box::new(self)); let program = THIS_PROGRAM - .get() - .unwrap() - .as_ref() + .get_raw() .unwrap() .downcast_ref::<Program<C>>() .unwrap(); @@ -165,9 +166,7 @@ where }; let program = THIS_PROGRAM - .get() - .unwrap() - .as_ref() + .get_raw() .unwrap() .downcast_ref::<Program<C>>() .unwrap(); @@ -232,6 +231,11 @@ where where C: 'static + Send + Sync, { - std::process::exit(self.exec()) + let exit_code = self.exec(); + // SAFETY: exec() is synchronous — it returns only after all + // chain handlers and renderers have finished. No code still + // holds references from get_raw() at this point. + drop(unsafe { THIS_PROGRAM.take() }); + std::process::exit(exit_code) } } diff --git a/mingling_core/src/program/repl_exec.rs b/mingling_core/src/program/repl_exec.rs index d6b3f00..d292be1 100644 --- a/mingling_core/src/program/repl_exec.rs +++ b/mingling_core/src/program/repl_exec.rs @@ -129,9 +129,7 @@ where payload: panic_info, }; let program = crate::program::THIS_PROGRAM - .get() - .unwrap() - .as_ref() + .get_raw() .unwrap() .downcast_ref::<Program<C>>() .unwrap(); diff --git a/mingling_core/src/program/single_instance.rs b/mingling_core/src/program/single_instance.rs index 70771d5..d16bcf5 100644 --- a/mingling_core/src/program/single_instance.rs +++ b/mingling_core/src/program/single_instance.rs @@ -1,10 +1,94 @@ -use std::sync::OnceLock; +use std::cell::UnsafeCell; +use std::sync::atomic::{AtomicBool, Ordering}; use crate::{Program, ProgramCollect}; +/// A single-slot container that can be initialized once, read many times, +/// and taken out once (for cleanup before process exit). +/// +/// # Safety +/// +/// - `set()` is called once during `exec_wrapper`, before any other access. +/// - `get_raw()` is called during execution (concurrent reads are safe because +/// the inner value is immutable once set until `take()`). +/// - `take()` is called only after execution completes, when no code still +/// holds a reference from `get_raw()`. +pub(crate) struct ProgramCell { + initialized: AtomicBool, + inner: UnsafeCell<Option<Box<dyn std::any::Any + Send + Sync>>>, +} + +// Safety: Sync is safe because: +// - `initialized` is AtomicBool (Sync) +// - `inner` is only read-after-write in sequence: set() → repeated get(), then +// optionally take() after all get() callers are done. +// - No concurrent write+read or write+write exists. +unsafe impl Sync for ProgramCell {} + +impl ProgramCell { + pub(crate) const fn new() -> Self { + Self { + initialized: AtomicBool::new(false), + inner: UnsafeCell::new(None), + } + } + + /// Initialize the cell with a value. Panics if already initialized. + pub(crate) fn set(&self, val: Box<dyn std::any::Any + Send + Sync>) { + assert!( + !self.initialized.swap(true, Ordering::AcqRel), + "ProgramCell already initialized" + ); + // SAFETY: `set()` is the sole writer — the `swap(true, AcqRel)` above + // guarantees exclusive access before the write becomes visible. + unsafe { + *self.inner.get() = Some(val); + } + } + + /// Returns a reference to the stored value, or `None` if not yet + /// initialized or already taken. + pub(crate) fn get_raw(&self) -> Option<&Box<dyn std::any::Any + Send + Sync>> { + if self.initialized.load(Ordering::Acquire) { + // SAFETY: after the Acquire load sees `true`, the matching + // Release-store in `set()` has happened, so the write to `inner` + // is visible. Only shared references (no mutation) are handed + // out, so this is safe. If `take()` has already been called + // (initialized → false), `get_raw()` returns `None` because the + // Acquire load won't see `true`. + unsafe { (*self.inner.get()).as_ref() } + } else { + None + } + } + + /// Take ownership of the stored value and reset the cell. + /// After this, `get_raw()` returns `None`. + /// + /// # Safety + /// + /// The caller must ensure that **no references returned by `get_raw()`** + /// are still alive when this method is called — otherwise a dangling + /// pointer would be exposed. + /// + /// This is intended to be called once, in `exec_and_exit()`, **after** + /// execution has finished and no code still holds references from + /// `get_raw()`. + pub(crate) unsafe fn take(&self) -> Option<Box<dyn std::any::Any + Send + Sync>> { + // Swap the flag to false so that future `get_raw()` calls return None. + if self.initialized.swap(false, Ordering::AcqRel) { + // SAFETY: `take()` is the sole mutator, called after all + // `get_raw()` callers have finished. No other thread reads + // `inner` at this point. + unsafe { (*self.inner.get()).take() } + } else { + None + } + } +} + /// Global static reference to the current program instance -pub(crate) static THIS_PROGRAM: OnceLock<Option<Box<dyn std::any::Any + Send + Sync>>> = - OnceLock::new(); +pub(crate) static THIS_PROGRAM: ProgramCell = ProgramCell::new(); /// Returns a reference to the current program instance, panics if not set. /// @@ -24,5 +108,5 @@ fn try_get_this_program<C>() -> Option<&'static Program<C>> where C: ProgramCollect<Enum = C> + 'static, { - THIS_PROGRAM.get()?.as_ref()?.downcast_ref::<Program<C>>() + THIS_PROGRAM.get_raw()?.downcast_ref::<Program<C>>() } |