diff --git a/src/notcurses/builder.rs b/src/notcurses/builder.rs index ecf0a8b..5f930b8 100644 --- a/src/notcurses/builder.rs +++ b/src/notcurses/builder.rs @@ -5,10 +5,9 @@ use crate::{ error::NotcursesResult as Result, - notcurses::{LogLevel, Notcurses}, + notcurses::{LogLevel, Notcurses, NotcursesInner}, sys::{Nc, NcOptionsBuilder}, }; -use std::{cell::RefCell, rc::Rc}; /// A [`Notcurses`] builder. #[derive(Clone, Copy, Debug)] @@ -39,8 +38,9 @@ impl NotcursesBuilder { pub fn build(self) -> Result { Notcurses::lock_notcurses()?; let nc = unsafe { Nc::with_options(self.options.build())? }; + Ok(Notcurses { - nc: Rc::new(RefCell::new(nc)), + inner: NotcursesInner::new(nc), options: self.options, }) } diff --git a/src/notcurses/mod.rs b/src/notcurses/mod.rs index 0d5eca6..be7727a 100644 --- a/src/notcurses/mod.rs +++ b/src/notcurses/mod.rs @@ -10,6 +10,7 @@ mod notcurses; mod statistics; pub use self::notcurses::Notcurses; +pub(crate) use self::notcurses::NotcursesInner; pub use builder::NotcursesBuilder; pub use capabilities::Capabilities; pub use log_level::LogLevel; diff --git a/src/notcurses/notcurses.rs b/src/notcurses/notcurses.rs index c3350e1..f11160a 100644 --- a/src/notcurses/notcurses.rs +++ b/src/notcurses/notcurses.rs @@ -17,31 +17,46 @@ use crate::{ }; use std::{cell::RefCell, rc::Rc}; +/// Maintains the notcurses context alive until all dependend objects are dropped. +pub(crate) struct NotcursesInner { + pub(crate) nc: *mut Nc, +} +impl NotcursesInner { + #[inline] + #[must_use] + pub(crate) fn new(nc: *mut Nc) -> Rc> { + Rc::new(RefCell::new(NotcursesInner { nc })) + } +} + /// *Notcurses* state for a given terminal, composed of [`Plane`][crate::plane::Plane]s. /// /// There can only be a single `Notcurses` instance per thread at any given moment. pub struct Notcurses { - pub(crate) nc: Rc>, - pub(super) options: NcOptionsBuilder, + // This is cloned in dependent objects, to ensure proper drop order. + pub(crate) inner: Rc>, + pub(crate) options: NcOptionsBuilder, } mod core_impls { - use super::{Nc, Notcurses, OnceCell, NOTCURSES_LOCK}; + use super::{Notcurses, NotcursesInner, OnceCell, NOTCURSES_LOCK}; use core::fmt; - impl Drop for Notcurses { + // Notcurses will be properly stopped after all dependent objects are dropped + impl Drop for NotcursesInner { fn drop(&mut self) { - // Allows initializing a new Notcurses instance again. - NOTCURSES_LOCK.with(|refcell| { - refcell.replace(OnceCell::new()); - }); - let nc_ptr: *mut Nc = *self.nc.borrow_mut(); + let nc_ptr = self.nc; if !nc_ptr.is_null() { unsafe { (*nc_ptr).drop_planes(); (*nc_ptr).stop().expect("Notcurses.stop() failed"); } } + + // Unlock the static lock to allow new `Notcurses` instances + NOTCURSES_LOCK.with(|refcell| { + refcell.replace(OnceCell::new()); + }); } } @@ -146,7 +161,7 @@ impl Notcurses { let options = NcOptionsBuilder::new().suppress_banners(true); let nc_ptr = unsafe { Nc::with_options(options.build())? }; Ok(Notcurses { - nc: Rc::new(RefCell::new(nc_ptr)), + inner: NotcursesInner::new(nc_ptr), options, }) } @@ -157,7 +172,7 @@ impl Notcurses { let options = NcOptionsBuilder::new(); let nc_ptr = unsafe { Nc::with_options(options.build())? }; Ok(Notcurses { - nc: Rc::new(RefCell::new(nc_ptr)), + inner: NotcursesInner::new(nc_ptr), options, }) } @@ -170,7 +185,7 @@ impl Notcurses { .cli_mode(true); let nc_ptr = unsafe { Nc::with_options(options.build())? }; Ok(Notcurses { - nc: Rc::new(RefCell::new(nc_ptr)), + inner: NotcursesInner::new(nc_ptr), options, }) } @@ -181,7 +196,7 @@ impl Notcurses { let options = NcOptionsBuilder::new().cli_mode(true); let nc_ptr = unsafe { Nc::with_options(options.build())? }; Ok(Notcurses { - nc: Rc::new(RefCell::new(nc_ptr)), + inner: NotcursesInner::new(nc_ptr), options, }) } @@ -194,7 +209,7 @@ impl Notcurses { where F: FnOnce(&Nc) -> R, { - let nc_ptr = *self.nc.borrow(); + let nc_ptr = self.inner.borrow().nc; unsafe { f(&*nc_ptr) } } @@ -204,7 +219,7 @@ impl Notcurses { where F: FnOnce(&mut Nc) -> R, { - let nc_ptr = *self.nc.borrow_mut(); + let nc_ptr = self.inner.borrow_mut().nc; unsafe { f(&mut *nc_ptr) } } } diff --git a/src/plane/builder.rs b/src/plane/builder.rs index a60147b..20d98e1 100644 --- a/src/plane/builder.rs +++ b/src/plane/builder.rs @@ -9,7 +9,6 @@ use crate::{ sys::{Nc, NcPlane, NcPlaneOptionsBuilder}, Notcurses, Position, Size, }; -use std::{cell::RefMut, rc::Rc}; /// A [`Plane`] builder. #[derive(Debug, Default)] @@ -29,15 +28,15 @@ impl PlaneBuilder { /// Returns a new standalone `Plane`. pub fn build(self, nc: &Notcurses) -> Result { - let notcurses = Rc::clone(&nc.nc); + let notcurses = nc.inner.clone(); let ncplane = { - let nc_borrow: RefMut<*mut Nc> = notcurses.borrow_mut(); - let nc_ptr: *mut Nc = *nc_borrow; + let inner_nc = notcurses.borrow_mut(); + let nc_ptr: *mut Nc = inner_nc.nc; + // SAFETY: ensured via RefCell's borrowing rules let nc_ref: &mut Nc = unsafe { &mut *nc_ptr }; NcPlane::new_pile(nc_ref, &self.options.build())? }; - Ok(Plane { nc: ncplane, notcurses, diff --git a/src/plane/plane.rs b/src/plane/plane.rs index 3139327..2501434 100644 --- a/src/plane/plane.rs +++ b/src/plane/plane.rs @@ -6,9 +6,9 @@ use crate::{ color::{Channel, Channels}, error::NotcursesResult as Result, - notcurses::{Capabilities, Notcurses}, + notcurses::{Capabilities, Notcurses, NotcursesInner}, plane::{Align, Cell, PlaneBuilder, PlaneGeometry, Style}, - sys::{Nc, NcPlane}, + sys::NcPlane, visual::Blitter, Position, Size, }; @@ -17,7 +17,8 @@ use std::{cell::RefCell, rc::Rc}; /// A drawable text surface, composed of [`Cell`]s. pub struct Plane { pub(super) nc: *mut NcPlane, - pub(super) notcurses: Rc>, + // Ensures the notcurses context remains alive as long as this object exists + pub(super) notcurses: Rc>, } mod core_impls { @@ -34,8 +35,7 @@ mod core_impls { CLI_PLANE_LOCK.with(|refcell| { refcell.replace(OnceCell::new()); }); - } else if crate::Notcurses::is_initialized() && self.notcurses.try_borrow_mut().is_ok() - { + } else if crate::Notcurses::is_initialized() { let _res = self.into_ref_mut().destroy(); } } @@ -89,7 +89,7 @@ impl Plane { pub fn from_ncplane(ncplane: &mut NcPlane, notcurses: &Notcurses) -> Plane { Plane { nc: ncplane as *mut NcPlane, - notcurses: notcurses.nc.clone(), + notcurses: notcurses.inner.clone(), } } @@ -198,7 +198,7 @@ impl Plane { pub fn duplicate(&self) -> Plane { Plane { nc: self.into_ref().dup(), - notcurses: Rc::clone(&self.notcurses), + notcurses: self.notcurses.clone(), } }