refactor: purge CHandle from module internals (M14 R5)

Module-internal object references are Rust types now (values, Arc,
Mutex); CHandle remains only at the oakengine C-ABI boundary:

- oakundo: the global stack holds UndoStack/UndoCommand values
  directly (stack token is the static's address)
- oaktimeline: marker/workarea boxes carry Arc<Mutex<T>>; commands
  share the same allocation through Arc clones (readers in oakengine
  stubs and the app's graphops updated to lock)
- oaktask/oakstorage: sessions, write-through bindings and the
  database backend pass ProjectArc; the Session drops its manual
  release bookkeeping; nodeutil keeps the CHandle<->Arc boundary
  conversion (release_project restored for the app)
- oakcodec: handle.rs deleted outright (no facade entry needed it);
  texture/block placeholders are unit structs
- oakrender: copier's project handle is an identity u64; alive-count
  machinery removed; handle.rs is make_owned/get/get_mut only
- oakplugin: the instance registry is gone (its unregister key never
  matched, leaking weak entries); handle.rs is the RefBox boundary type
- oaknode/oakcommon: only dead guard/borrow helpers removed; external
  payload handles (texture/processor) documented as the boundary

Flake hunts landed along the way: the audio recording test serializes
on the shared manager lock with a normalized state; the autocacher
cancel test uses a slow producer so cancellation is deterministic.
This commit is contained in:
2026-08-17 16:40:15 +08:00
parent ede03d0bfe
commit b36cbd6b6f
53 changed files with 1260 additions and 2369 deletions
+23 -1
View File
@@ -337,7 +337,10 @@ mod tests {
#[test]
fn single_frame_cancels_previous() {
let (mut c, mut pool) = new_cacher();
// The race this asserts ("the previous frame is cancelled") is only
// deterministic when the first job cannot finish before the
// superseding submit lands — produce frames slowly for this test.
let (mut c, mut pool) = new_cacher_slow();
c.attach(7);
let first = c.single_frame(Rational::new(0, 1));
let second = c.single_frame(Rational::new(1, 1));
@@ -349,6 +352,25 @@ mod tests {
pool.shutdown();
}
/// A cacher whose frames take ~100ms to produce (see
/// [`single_frame_cancels_previous`]).
fn new_cacher_slow() -> (PreviewAutoCacher, WorkerPool) {
let mut pool = WorkerPool::new(2);
pool.start();
let producer: crate::ticket::Producer = Arc::new(|_, _| {
std::thread::sleep(std::time::Duration::from_millis(100));
let mut f = Frame::new();
let mut p = VideoParamsPod::default();
p.width = 4;
p.height = 4;
f.set_video_params(p);
f.allocate();
Ok(crate::ticket::TicketPayload::Video(Texture::wrap_frame(f)))
});
let arena = Arc::new(TicketArena::new(pool.clone(), producer));
(PreviewAutoCacher::new(arena), pool)
}
#[test]
fn ignore_requests_suppresses_jobs() {
let (mut c, mut pool) = new_cacher();
+69 -59
View File
@@ -17,13 +17,44 @@
//! Render-side project copy client (the C++ ProjectCopier, inverted):
//! all copying happens inside oaknode. oaknode never implemented the
//! deep-copy direction (single-lib plan §4.1 — dead direction), so the
//! copy operations fail explainably and the success-path tests are
//! `#[ignore]`d.
//! copy operations fail explainably; the tests assert those failures.
//!
//! M14 R5: the module is entirely internal to oakrender (no facade entry
//! is involved), so the oaknode project handle was reduced to its numeric
//! identity — a Rust value type instead of a `CHandle`. The live project
//! object stays with oaknode; the render side stores only identity pairs
//! (see `COVERAGE.md`, "render 只存 identity 对").
use crate::error::{Error, Result};
/// A project handle (oaknode-owned; the shared canonical handle type).
pub type ProjectHandle = crate::handle::CHandle;
/// An opaque identity for an oaknode project (the C++ handle's `ctx`
/// reduced to its numeric value). oaknode owns the live project and
/// maintains the identity map; this module never holds the object, so no
/// lifetime management is required.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct ProjectHandle(u64);
impl ProjectHandle {
/// New identity; `0` is the empty handle.
pub fn new(identity: u64) -> Self {
ProjectHandle(identity)
}
/// The empty handle (no project attached).
pub fn null() -> Self {
ProjectHandle(0)
}
/// True for the empty handle.
pub fn is_null(self) -> bool {
self.0 == 0
}
/// The raw identity value.
pub fn as_u64(self) -> u64 {
self.0
}
}
/// One change record (see oaknode `ChangeRecord`).
#[repr(C)]
@@ -77,15 +108,13 @@ pub fn project_sync_copy(
))
}
/// A handle to a render-side project copy.
/// A render-side project copy (identity only — the copy's live object
/// stays with oaknode).
pub struct ProjectCopy {
/// Identity of the source project.
pub source: u64,
/// Identity of the copied project (oaknode-owned).
/// Identity of the copied project (0 = no copy attached).
pub copy: u64,
/// Owned oaknode handle to the copy (kept alive for the copier's
/// lifetime; released on drop).
copy_handle: Option<ProjectHandle>,
/// Change-generation counter of the last successful sync.
pub last_sync_generation: u64,
/// True while recorded changes await `sync`.
@@ -98,19 +127,18 @@ impl ProjectCopy {
Self {
source: 0,
copy: 0,
copy_handle: None,
last_sync_generation: 0,
has_pending_updates: false,
}
}
/// Create a deep copy of `source` through the oaknode C ABI
/// (C++ `ProjectCopier::set_project`).
/// Create a deep copy of `source` through oaknode (C++
/// `ProjectCopier::set_project`).
pub fn set_project(&mut self, source: ProjectHandle) -> Result<()> {
if source.is_null() {
return Err(Error::Invalid);
}
// Release any previous copy.
// Drop any previous copy.
self.release_copy();
let copy = crate::copier::project_deep_copy(source);
if copy.is_null() {
@@ -118,9 +146,8 @@ impl ProjectCopy {
"oaknode_project_deep_copy failed (symbol missing or copy error)".into(),
));
}
self.source = source.ctx as u64;
self.copy = copy.ctx as u64;
self.copy_handle = Some(copy);
self.source = source.as_u64();
self.copy = copy.as_u64();
self.last_sync_generation = 0;
self.has_pending_updates = false;
Ok(())
@@ -129,26 +156,26 @@ impl ProjectCopy {
/// Push a recorded change set into the copy (C++
/// ProjectCopier::process_update_queue).
pub fn sync(&mut self, changes: &[ChangeRecord]) -> Result<()> {
let source = ProjectHandle {
ctx: self.source as *mut std::ffi::c_void,
addref: None,
release: None,
abi_version: crate::handle::OAKRENDER_ABI_VERSION,
};
let copy = self.copy_handle.unwrap_or_else(ProjectHandle::null);
if copy.is_null() {
if self.copy == 0 {
return Err(Error::State);
}
crate::copier::project_sync_copy(source, copy, changes)?;
crate::copier::project_sync_copy(
ProjectHandle::new(self.source),
ProjectHandle::new(self.copy),
changes,
)?;
self.last_sync_generation += 1;
self.has_pending_updates = false;
Ok(())
}
/// The copied project handle (owned by this copier; borrowed for the
/// caller).
/// The copied project's identity (borrowed for the caller).
pub fn copied_project(&self) -> Option<ProjectHandle> {
self.copy_handle
if self.copy == 0 {
None
} else {
Some(ProjectHandle::new(self.copy))
}
}
/// The copied counterpart of an original node — requires the oaknode
@@ -158,19 +185,12 @@ impl ProjectCopy {
None
}
/// Drop the copy (releases the oaknode handle).
/// Drop the copy (forgets its identity).
pub fn destroy(&mut self) {
self.release_copy();
}
fn release_copy(&mut self) {
if let Some(handle) = self.copy_handle.take() {
if let Some(release) = handle.release {
// SAFETY: the handle came from oaknode_project_deep_copy;
// releasing the last reference destroys the copy.
unsafe { release(handle.ctx) };
}
}
self.copy = 0;
}
}
@@ -181,12 +201,6 @@ impl Default for ProjectCopy {
}
}
impl Drop for ProjectCopy {
fn drop(&mut self) {
self.release_copy();
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -209,6 +223,19 @@ mod tests {
);
}
#[test]
fn set_project_with_valid_identity_fails() {
// oaknode never implemented the deep-copy direction; even a valid
// identity cannot be copied, and the copier fails explainably.
let mut pc = ProjectCopy::new();
assert_eq!(
pc.set_project(ProjectHandle::new(1)).unwrap_err().code(),
Error::Failed(String::new()).code()
);
assert_eq!(pc.copy, 0);
assert!(pc.copied_project().is_none());
}
#[test]
fn sync_without_project_is_state_error() {
let mut pc = ProjectCopy::new();
@@ -225,21 +252,4 @@ mod tests {
pc.destroy();
assert_eq!(pc.copy, 0);
}
#[test]
#[ignore = "needs oaknode deep-copy (not implemented)"]
fn deep_copy_roundtrip_with_real_node() {
// oaknode never implemented the deep-copy direction; the copy
// always fails explainably, which is what the live path checks.
let mut pc = ProjectCopy::new();
let src = ProjectHandle {
ctx: 1 as *mut std::ffi::c_void,
addref: None,
release: None,
abi_version: crate::handle::OAKRENDER_ABI_VERSION,
};
pc.set_project(src).unwrap();
assert_ne!(pc.copy, 0);
assert!(pc.copied_project().is_some());
}
}
+17 -170
View File
@@ -14,61 +14,40 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.
//! Refcounted-handle scaffolding (same per-module pattern as the
//! oakplugin/oaknode crates; duplicated on purpose — handle function
//! pointers must run code from the creating DLL).
//! Refcounted-handle scaffolding for the oakengine facade entry points.
//!
//! M14 R5: after the single-lib unification, no object reference passes
//! as a `CHandle` inside oakrender anymore — the crate's internal calls
//! use Rust types directly. The remaining surface is only what the
//! facade's stubs.rs calls at the boundary: [`make_owned`] (box a value
//! into an owned handle), [`get`]/[`get_mut`] (typed views back out).
//!
//! Mirrors `src/render/c_api/internalhandles.h`: every public oakrender
//! handle is `{ctx, addref, release, abi_version}`; `ctx` points at a
//! [`RefBox<T>`] on this crate's heap. `owns == false` boxes (borrowed
//! wrappers) only free the box at zero.
//!
//! Live-object accounting mirrors the C++ `alive_inc`/`alive_dec`:
//! [`make_owned`] counts the handle, the owned release un-counts it, so
//! `oakrender_debug_alive_count()` stays meaningful for leak assertions.
//! [`RefBox<T>`] on this crate's heap.
use std::any::Any;
use std::panic::{catch_unwind, AssertUnwindSafe};
use std::sync::atomic::{AtomicU32, AtomicUsize, Ordering};
use std::sync::atomic::{AtomicU32, Ordering};
/// ABI version stamped into every handle.
pub const OAKRENDER_ABI_VERSION: u32 = 1;
/// Heap box behind a handle's `ctx`.
pub struct RefBox<T: ?Sized> {
struct RefBox<T: ?Sized> {
/// Atomic reference count.
pub refs: AtomicU32,
refs: AtomicU32,
/// Boxed value.
pub value: T,
value: T,
}
/// The shared ABI value-handle type (single-lib unification, see
/// `docs/zh/plans/riir/single-lib.md`): one canonical
/// `{ctx, addref, release, abi_version}` type in `oakcore-rs`, re-exported
/// here so the crate's `ffi.rs` signatures and handle scaffolding stay
/// source-compatible. `Send + Sync` come from the shared type.
/// here so the facade's handle scaffolding stays source-compatible.
/// `Send + Sync` come from the shared type.
pub use oakcore_rs::handle::CHandle;
/// Global live-object count (owned handles + cancel-atom boxes).
static ALIVE_COUNT: AtomicUsize = AtomicUsize::new(0);
/// Increment the live-object count (owned handle creation).
pub fn alive_inc() {
ALIVE_COUNT.fetch_add(1, Ordering::Relaxed);
}
/// Decrement the live-object count (owned handle destruction).
pub fn alive_dec() {
ALIVE_COUNT.fetch_sub(1, Ordering::Relaxed);
}
/// Current live-object count (`oakrender_debug_alive_count`).
pub fn alive_count() -> i32 {
ALIVE_COUNT.load(Ordering::Relaxed) as i32
}
/// addref implementation: atomic +1. Shared by owned and borrowed boxes —
/// borrowing only extends the box's lifetime, not the borrowed object's.
/// addref implementation: atomic +1 on the box's refcount.
unsafe extern "C" fn refbox_addref<T: Any + Send>(ctx: *mut std::ffi::c_void) {
unsafe {
let rb = ctx as *const RefBox<T>;
@@ -77,42 +56,25 @@ unsafe extern "C" fn refbox_addref<T: Any + Send>(ctx: *mut std::ffi::c_void) {
}
}
/// release implementation (owned): atomic -1; at zero, reclaim the box and
/// destroy the contained object.
/// release implementation: atomic -1; at zero, reclaim the box and destroy
/// the contained object.
unsafe extern "C" fn refbox_release_owned<T: Any + Send>(ctx: *mut std::ffi::c_void) {
unsafe {
let rb = ctx as *mut RefBox<T>;
// AcqRel: the thread that drops the last reference must observe all
// prior writes (including internal state the destructor needs).
if (*rb).refs.fetch_sub(1, Ordering::AcqRel) == 1 {
alive_dec();
drop(Box::from_raw(rb));
}
}
}
/// release implementation (borrowed, produced by [`make_borrowed`]): at
/// zero only reclaim the box memory, forgetting the contained object — its
/// ownership stays with the borrower.
unsafe extern "C" fn refbox_release_borrowed<T: Any + Send>(ctx: *mut std::ffi::c_void) {
unsafe {
let rb = ctx as *mut RefBox<T>;
if (*rb).refs.fetch_sub(1, Ordering::AcqRel) == 1 {
// Partial move: move the value out of the temporary Box so the
// Box drop only frees the allocation; forget the value so it is
// never dropped (double-free guard).
std::mem::forget((Box::from_raw(rb)).value);
}
}
}
/// Owned handle with count 1; empty on allocation failure.
pub fn make_owned<T: Any + Send>(value: T) -> CHandle {
let rb = Box::into_raw(Box::new(RefBox {
refs: AtomicU32::new(1),
value,
}));
alive_inc();
CHandle {
ctx: rb as *mut std::ffi::c_void,
addref: Some(refbox_addref::<T>),
@@ -121,26 +83,6 @@ pub fn make_owned<T: Any + Send>(value: T) -> CHandle {
}
}
/// Borrowed handle for an object owned elsewhere.
///
/// # Safety
/// Caller guarantees `ptr` outlives every derived handle.
pub unsafe fn make_borrowed<T: Any + Send>(ptr: *mut T) -> CHandle {
if ptr.is_null() {
return CHandle::null();
}
let rb = Box::into_raw(Box::new(RefBox {
refs: AtomicU32::new(1),
value: unsafe { std::ptr::read(ptr) },
}));
CHandle {
ctx: rb as *mut std::ffi::c_void,
addref: Some(refbox_addref::<T>),
release: Some(refbox_release_borrowed::<T>),
abi_version: OAKRENDER_ABI_VERSION,
}
}
/// Typed view into a handle; `None` for empty handles.
///
/// # Safety
@@ -165,44 +107,6 @@ pub unsafe fn get_mut<T: Any>(h: &CHandle) -> Option<&mut T> {
unsafe { Some(&mut (*(h.ctx as *mut RefBox<T>)).value) }
}
/// A boxed handle that does **not** participate in the live-object count
/// (mirrors the C++ borrowed `make_handle(…, owns=false)` boxes): the
/// release only frees the box and its value, never a foreign object.
pub fn make_borrowed_owned<T: Any + Send>(value: T) -> CHandle {
let rb = Box::into_raw(Box::new(RefBox {
refs: AtomicU32::new(1),
value,
}));
CHandle {
ctx: rb as *mut std::ffi::c_void,
addref: Some(refbox_addref::<T>),
release: Some(refbox_release_borrowed::<T>),
abi_version: OAKRENDER_ABI_VERSION,
}
}
/// Panic-catching FFI wrapper for i32-returning exports.
pub fn guard<F: FnOnce() -> crate::error::Result<()>>(f: F) -> i32 {
match catch_unwind(AssertUnwindSafe(f)) {
Ok(Ok(())) => crate::error::OAKRENDER_OK,
Ok(Err(e)) => e.code(),
Err(_) => crate::error::OAKRENDER_E_FAILED,
}
}
/// Panic-catching FFI wrapper for handle-returning exports.
pub fn guard_handle<F: FnOnce() -> crate::error::Result<CHandle>>(f: F) -> CHandle {
match catch_unwind(AssertUnwindSafe(f)) {
Ok(Ok(h)) => h,
Ok(Err(_)) | Err(_) => CHandle::null(),
}
}
/// Panic-catching FFI wrapper for void exports.
pub fn guard_void<F: FnOnce()>(f: F) {
let _ = catch_unwind(AssertUnwindSafe(f));
}
#[cfg(test)]
mod tests {
use super::*;
@@ -215,12 +119,10 @@ mod tests {
let h = make_owned(Obj(7));
assert!(!h.is_null());
assert_eq!(h.abi_version, OAKRENDER_ABI_VERSION);
let before = alive_count();
// addref/release through the stored function pointers.
unsafe { h.addref.unwrap()(h.ctx) };
unsafe { h.release.unwrap()(h.ctx) };
unsafe { h.release.unwrap()(h.ctx) };
assert_eq!(alive_count(), before - 1);
}
#[test]
@@ -232,47 +134,6 @@ mod tests {
unsafe { h.release.unwrap()(h.ctx) };
}
#[test]
fn borrowed_release_does_not_count() {
let mut obj = Obj(5);
let before = alive_count();
let h = unsafe { make_borrowed(&mut obj) };
assert!(!h.is_null());
assert_eq!(alive_count(), before, "borrowed boxes are not counted");
unsafe { h.release.unwrap()(h.ctx) };
assert_eq!(alive_count(), before);
// The borrowed value is intact (never dropped).
assert_eq!(obj, Obj(5));
}
#[test]
fn guard_maps_results() {
assert_eq!(guard(|| Ok(())), 0);
assert_eq!(guard(|| Err(crate::error::Error::Invalid)), -70001);
assert_eq!(guard(|| panic!("boom")), -70003);
}
#[test]
fn guard_handle_and_void_panic_safety() {
// Panics map to empty handles / are swallowed.
let h = guard_handle(|| panic!("boom"));
assert!(h.is_null());
let h = guard_handle(|| Err(crate::error::Error::State));
assert!(h.is_null());
let h = guard_handle(|| Ok(make_owned(Obj(1))));
assert!(!h.is_null());
unsafe { h.release.unwrap()(h.ctx) };
guard_void(|| panic!("swallowed"));
guard_void(|| {});
}
#[test]
fn make_borrowed_null_yields_empty() {
let h = unsafe { make_borrowed::<Obj>(std::ptr::null_mut()) };
assert!(h.is_null());
}
#[test]
fn get_mut_mutates_boxed_value() {
let h = make_owned(Obj(3));
@@ -283,18 +144,4 @@ mod tests {
}
unsafe { h.release.unwrap()(h.ctx) };
}
#[test]
fn make_borrowed_owned_does_not_count() {
let before = alive_count();
let h = make_borrowed_owned(Obj(4));
assert_eq!(
alive_count(),
before,
"borrowed-owned boxes are not counted"
);
assert!(!h.is_null());
unsafe { h.release.unwrap()(h.ctx) };
assert_eq!(alive_count(), before);
}
}