From dbe926b13645f4889eb1bb0ecdce2abee6addd0e Mon Sep 17 00:00:00 2001 From: Gopalakrishna Sharma Date: Tue, 27 Oct 2026 15:14:35 +0530 Subject: [PATCH] feat: transfer byte vector allocations without copying Preserve original length and capacity for tracked byte returns, keep the even-buffer fallback for odd addresses, and test ownership, cancellation, and failure paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- AI_WORKFLOW.md | 10 ++++ CHANGELOG.md | 9 +++ PHILOSOPHY.md | 11 ++++ src/utils.rs | 157 +++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 183 insertions(+), 4 deletions(-) diff --git a/AI_WORKFLOW.md b/AI_WORKFLOW.md index 68d3b38..829807b 100644 --- a/AI_WORKFLOW.md +++ b/AI_WORKFLOW.md @@ -108,6 +108,16 @@ value. Callers must release cimpl-tracked objects, strings, and byte arrays through the allocating library's free function, not C `free()` or an untracked Rust deallocator. The `cimpl_free!` macro does not export a C ABI symbol either. +For byte returns, `to_c_bytes` transfers even-addressed `Vec` allocations +without copying or shrinking. The registry retains the original pointer, +length, and capacity for release. Its cleanup closure reconstructs the vector +only when invoked; cancellation must not free the allocation merely by dropping +the closure. Odd-addressed allocations must still be +copied into an even-addressed buffer before tracking; check parity first so a +successful fallback does not leave a tracking-refusal error. Returned bytes are +read-only to C callers. Do not reconstruct transferred vectors with the copied +buffer's alignment or add a spare-capacity copy threshold. + ### Keep Panics Out of the C ABI Do not use `panic!()`, `unwrap()`, or `expect()` for recoverable failures in diff --git a/CHANGELOG.md b/CHANGELOG.md index 17db8bd..8ad71a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,15 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +- `to_c_bytes` transfers even-addressed `Vec` allocations without copying + or shrinking, retaining the original length and capacity until the allocating + library's free function is called. Odd-addressed vectors retain the existing + even-buffer copy fallback. C signatures and release conventions are unchanged. + ## [0.4.0] _14 September 2026_ diff --git a/PHILOSOPHY.md b/PHILOSOPHY.md index c0a9821..f552be7 100644 --- a/PHILOSOPHY.md +++ b/PHILOSOPHY.md @@ -358,6 +358,17 @@ pub extern "C" fn create_object() -> *mut MyObject { All tracked allocations can be freed with library-specific `*_free()` functions that wrap `cimpl::cimpl_free()`. +`to_c_bytes` transfers an even-addressed `Vec` allocation to the registry +without copying or shrinking. The original length and capacity are retained +until release, including any spare capacity; C callers receive a read-only +pointer and must not use C `free()`. Odd addresses are reserved for opaque +handle IDs, so an odd-addressed vector still requires a copy into an +even-addressed buffer. The registry releases each allocation using its own +original owner or allocation layout, not a foreign allocator. +For transferred vectors, the cleanup closure reconstructs the original raw +parts only when invoked. Dropping a cancelled closure must not free the +allocation, preserving the registry's ownership-transfer and fork-safety rules. + ## What Makes a Good C API? ### DO: Keep it Simple diff --git a/src/utils.rs b/src/utils.rs index b17ab04..a9bfd51 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1355,23 +1355,62 @@ pub fn to_c_string(s: String) -> *mut std::os::raw::c_char { /// Converts a `Vec ` to a tracked C byte array pointer /// /// The returned pointer is tracked for allocation safety and MUST be freed -/// by calling `free_c_bytes`. +/// through the allocating library's wrapper around [`cimpl_free`]. +/// +/// Even-addressed allocations are transferred without copying or shrinking. +/// The registry retains the original length and capacity for release. +/// Odd-addressed allocations are copied into an even-addressed +/// buffer to avoid collisions with opaque handle IDs. /// /// # Arguments /// * `bytes` - The byte vector to convert /// /// # Returns -/// * `*const c_uchar` - Pointer to the byte array, or null if the vector is empty +/// * `*const c_uchar` - Read-only pointer to the byte array, or null if the +/// vector is empty, allocation fails, or tracking is refused. /// /// # Safety -/// The returned pointer must be freed exactly once by calling `free_c_bytes`. +/// The returned pointer must be freed exactly once through the allocating +/// library's free function, never through C `free`. /// Returns null for empty vectors to avoid dangling pointers from zero-sized allocations. pub fn to_c_bytes(bytes: Vec) -> *const c_uchar { + if bytes.is_empty() { + return std::ptr::null(); + } + to_c_bytes_with(bytes, get_registry(), |addr| addr.is_multiple_of(2)) +} + +fn to_c_bytes_with( + bytes: Vec, + registry: &PointerRegistry, + is_even: impl FnOnce(usize) -> bool, +) -> *const c_uchar { let len = bytes.len(); if len == 0 { return std::ptr::null(); } + let ptr = bytes.as_ptr(); + let addr = ptr.expose_provenance(); + if is_even(addr) { + let cap = bytes.capacity(); + let bytes = std::mem::ManuallyDrop::new(bytes); + // SAFETY: these are the Vec's original raw parts. Cleanup runs at most + // once; merely dropping a cancelled closure must not free the buffer. + let cleanup = Box::new(move || unsafe { + drop(Vec::from_raw_parts( + std::ptr::with_exposed_provenance_mut::(addr), + len, + cap, + )); + }); + if !registry.track_by_address(addr, TypeId::of::(), cleanup) { + drop(std::mem::ManuallyDrop::into_inner(bytes)); + return std::ptr::null(); + } + return ptr; + } + let ptr = alloc_even_buffer(&bytes, false); if ptr.is_null() { Error::other("could not allocate the byte buffer").set_last(); @@ -1379,7 +1418,7 @@ pub fn to_c_bytes(bytes: Vec) -> *const c_uchar { } drop(bytes); let ptr_val = ptr.expose_provenance(); - let tracked = get_registry().track_by_address( + let tracked = registry.track_by_address( ptr_val, TypeId::of::(), // SAFETY: ptr_val is this allocation's exposed address and len its @@ -1471,6 +1510,116 @@ mod tests { cimpl_free(ptr as *mut std::ffi::c_void); } + #[test] + fn test_to_c_bytes_preserves_allocation_with_spare_capacity() { + for capacity in [5, 64, 4096] { + let mut bytes = Vec::with_capacity(capacity); + bytes.extend_from_slice(&[1, 2, 3, 4, 5]); + let original = bytes.as_ptr(); + let ptr = to_c_bytes(bytes); + assert!(!ptr.is_null()); + if (original as usize).is_multiple_of(2) { + assert_eq!(ptr, original); + } else { + assert_ne!(ptr, original); + } + assert!((ptr as usize).is_multiple_of(2)); + assert_eq!( + unsafe { std::slice::from_raw_parts(ptr, 5) }, + [1, 2, 3, 4, 5] + ); + assert_eq!(cimpl_free(ptr as *mut std::ffi::c_void), 0); + } + } + + #[test] + fn test_to_c_bytes_even_buffer_fallback() { + let registry = PointerRegistry::new(); + let bytes = vec![1, 2, 3, 4, 5]; + let original = bytes.as_ptr(); + Error::take_last(); + let ptr = to_c_bytes_with(bytes, ®istry, |_| false); + assert!(!ptr.is_null()); + assert_ne!(ptr, original); + assert!((ptr as usize).is_multiple_of(2)); + assert_eq!( + unsafe { std::slice::from_raw_parts(ptr, 5) }, + [1, 2, 3, 4, 5] + ); + assert!(Error::take_last().is_none()); + assert!(registry.free(ptr as usize).is_ok()); + assert!(registry.free(ptr as usize).is_err()); + } + + #[test] + fn test_to_c_bytes_empty() { + for capacity in [0, 4096] { + Error::take_last(); + assert!(to_c_bytes(Vec::with_capacity(capacity)).is_null()); + assert!(Error::take_last().is_none()); + } + } + + #[test] + fn test_to_c_bytes_cancelled_cleanup_preserves_allocation() { + for capacity in [3, 64, 4096] { + let registry = PointerRegistry::new(); + let mut bytes = Vec::with_capacity(capacity); + bytes.extend_from_slice(&[1, 2, 3]); + let cap = bytes.capacity(); + let original = bytes.as_ptr(); + let ptr = to_c_bytes_with(bytes, ®istry, |addr| addr.is_multiple_of(2)); + assert!(!ptr.is_null()); + registry + .untrack(ptr as usize, TypeId::of::(), None) + .unwrap(); + assert_eq!(unsafe { std::slice::from_raw_parts(ptr, 3) }, [1, 2, 3]); + if ptr == original { + unsafe { drop(Vec::from_raw_parts(ptr.cast_mut(), 3, cap)) }; + } else { + unsafe { dealloc_even_buffer(ptr.expose_provenance(), 3) }; + } + } + } + + #[test] + fn test_to_c_bytes_tracking_refused() { + let mut registry = PointerRegistry::new(); + registry.owner_pid = current_pid().wrapping_add(1); + for handoff in [true, false] { + Error::take_last(); + let mut bytes = Vec::with_capacity(4096); + bytes.extend_from_slice(&[1, 2, 3]); + assert!(to_c_bytes_with(bytes, ®istry, |_| handoff).is_null()); + assert!(Error::take_last().is_some()); + assert!(registry.tracked.lock().unwrap().is_empty()); + } + } + + #[test] + #[cfg(not(target_os = "wasi"))] + fn test_to_c_bytes_release_on_another_thread() { + let ptr = to_c_bytes(vec![1, 2, 3, 4, 5]); + assert!(!ptr.is_null()); + let addr = ptr.expose_provenance(); + std::thread::spawn(move || { + assert_eq!(cimpl_free(std::ptr::with_exposed_provenance_mut(addr)), 0); + }) + .join() + .unwrap(); + } + + #[test] + fn test_to_c_bytes_local_registry_rejects_double_free_and_untracked_pointer() { + let registry = PointerRegistry::new(); + let ptr = to_c_bytes_with(vec![1, 2, 3], ®istry, |addr| addr.is_multiple_of(2)); + assert!(!ptr.is_null()); + assert!(registry.free(ptr as usize).is_ok()); + assert!(registry.free(ptr as usize).is_err()); + let untracked = [1u8, 2, 3]; + assert!(registry.free(untracked.as_ptr() as usize).is_err()); + } + #[test] fn test_to_c_string_with_null_byte() { // Test that strings with embedded nulls return null