From d714aeac231b9637772439c6462514bd3e007672 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Mon, 20 Jul 2026 19:41:52 +0200 Subject: [PATCH] refactor(compress): move heap CCtx release order to Rust ZSTD_freeCCtx previously passed one C callback that bundled dictionary, multithreaded context, workspace, and heap-object teardown. That left the lifetime order inside private C control flow and made the public destruction policy harder to audit. Project the four private operations separately and let Rust enforce the original dictionary, MT-context, workspace, and object order while preserving the static-context and workspace-embedded object guards. The callbacks still own the C layouts and custom allocator operations. Test Plan: - `ulimit -v 41943040; cargo +nightly fmt --manifest-path rust/Cargo.toml --all -- --check` -- passed - `ulimit -v 41943040; CARGO_BUILD_JOBS=1 cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings` -- passed - Broader native and original-test verification remains pending for the complete batch. --- lib/compress/zstd_compress.c | 39 +++++++++++++++--------- rust/src/zstd_compress.rs | 59 ++++++++++++++++++++++++------------ 2 files changed, 64 insertions(+), 34 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 444b5529d..5a8d621ca 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -297,14 +297,15 @@ typedef char ZSTD_rust_ref_thread_pool_state_layout[ && offsetof(ZSTD_rust_refThreadPoolState, streamStage) == 2 * sizeof(void*) && sizeof(ZSTD_rust_refThreadPoolState) == 3 * sizeof(void*)) ? 1 : -1]; -typedef void (*ZSTD_rust_freeCCtxContent_f)(void* context); -typedef void (*ZSTD_rust_freeCCtxObject_f)(void* context); +typedef void (*ZSTD_rust_freeCCtxCallback_f)(void* context); typedef struct { void* callbackContext; size_t staticSize; int cctxInWorkspace; - ZSTD_rust_freeCCtxContent_f freeContent; - ZSTD_rust_freeCCtxObject_f freeObject; + ZSTD_rust_freeCCtxCallback_f freeDictionaries; + ZSTD_rust_freeCCtxCallback_f freeMTContext; + ZSTD_rust_freeCCtxCallback_f freeWorkspace; + ZSTD_rust_freeCCtxCallback_f freeObject; } ZSTD_rust_freeCCtxState; size_t ZSTD_rust_freeCCtx(const ZSTD_rust_freeCCtxState* state); typedef char ZSTD_rust_free_cctx_state_layout[ @@ -312,11 +313,15 @@ typedef char ZSTD_rust_free_cctx_state_layout[ && offsetof(ZSTD_rust_freeCCtxState, staticSize) == sizeof(void*) && offsetof(ZSTD_rust_freeCCtxState, cctxInWorkspace) == 2 * sizeof(void*) - && offsetof(ZSTD_rust_freeCCtxState, freeContent) + && offsetof(ZSTD_rust_freeCCtxState, freeDictionaries) == 3 * sizeof(void*) - && offsetof(ZSTD_rust_freeCCtxState, freeObject) + && offsetof(ZSTD_rust_freeCCtxState, freeMTContext) == 4 * sizeof(void*) - && sizeof(ZSTD_rust_freeCCtxState) == 5 * sizeof(void*)) + && offsetof(ZSTD_rust_freeCCtxState, freeWorkspace) + == 5 * sizeof(void*) + && offsetof(ZSTD_rust_freeCCtxState, freeObject) + == 6 * sizeof(void*) + && sizeof(ZSTD_rust_freeCCtxState) == 7 * sizeof(void*)) ? 1 : -1]; typedef int (*ZSTD_rust_createCCtxValidateCustomMem_f)(void* context); typedef void* (*ZSTD_rust_createCCtxAllocate_f)(void* context, size_t size); @@ -3635,20 +3640,20 @@ static size_t ZSTD_sizeof_localDict(ZSTD_localDict dict) cdictSize); } -static void ZSTD_freeCCtxContent(ZSTD_CCtx* cctx) +static void ZSTD_rust_freeCCtx_freeMTContext(void* context) { - assert(cctx != NULL); - assert(cctx->staticSize == 0); - ZSTD_clearAllDicts(cctx); #ifdef ZSTD_MULTITHREAD + ZSTD_CCtx* const cctx = (ZSTD_CCtx*)context; ZSTDMT_freeCCtx(cctx->mtctx); cctx->mtctx = NULL; +#else + (void)context; #endif - ZSTD_cwksp_free(&cctx->workspace, cctx->customMem); } -static void ZSTD_rust_freeCCtx_freeContent(void* context) +static void ZSTD_rust_freeCCtx_freeWorkspace(void* context) { - ZSTD_freeCCtxContent((ZSTD_CCtx*)context); + ZSTD_CCtx* const cctx = (ZSTD_CCtx*)context; + ZSTD_cwksp_free(&cctx->workspace, cctx->customMem); } static void ZSTD_rust_freeCCtx_freeObject(void* context) @@ -3666,7 +3671,11 @@ size_t ZSTD_freeCCtx(ZSTD_CCtx* cctx) state.cctxInWorkspace = cctx != NULL && cctx->staticSize == 0 ? ZSTD_cwksp_owns_buffer(&cctx->workspace, cctx) : 0; - state.freeContent = ZSTD_rust_freeCCtx_freeContent; + /* Rust owns the release order; these callbacks retain only private C + * dictionary, MT-context, workspace, and object operations. */ + state.freeDictionaries = ZSTD_clearAllDicts_callback; + state.freeMTContext = ZSTD_rust_freeCCtx_freeMTContext; + state.freeWorkspace = ZSTD_rust_freeCCtx_freeWorkspace; state.freeObject = ZSTD_rust_freeCCtx_freeObject; return ZSTD_rust_freeCCtx(&state); } diff --git a/rust/src/zstd_compress.rs b/rust/src/zstd_compress.rs index 42f7c56af..015ea748b 100644 --- a/rust/src/zstd_compress.rs +++ b/rust/src/zstd_compress.rs @@ -2550,34 +2550,38 @@ pub unsafe extern "C" fn ZSTD_rust_initStaticCCtx( } } -type FreeCCtxContentFn = unsafe extern "C" fn(*mut c_void); -type FreeCCtxObjectFn = unsafe extern "C" fn(*mut c_void); +type FreeCCtxCallbackFn = unsafe extern "C" fn(*mut c_void); /// Explicit projection for the public `ZSTD_freeCCtx` wrapper. /// -/// Rust owns the null/static/embedded-object policy and callback ordering. -/// C retains the private content teardown and custom allocator callbacks. +/// Rust owns the null/static/embedded-object policy and the release order. +/// C retains only the private dictionary, MT-context, workspace, and custom +/// allocator operations behind narrow callbacks. #[repr(C)] pub struct ZSTD_rust_freeCCtxState { callback_context: *mut c_void, static_size: usize, cctx_in_workspace: c_int, - free_content: FreeCCtxContentFn, - free_object: FreeCCtxObjectFn, + free_dictionaries: FreeCCtxCallbackFn, + free_mt_context: FreeCCtxCallbackFn, + free_workspace: FreeCCtxCallbackFn, + free_object: FreeCCtxCallbackFn, } const _: () = { - assert!(size_of::() == size_of::()); - assert!(size_of::() == size_of::()); + assert!(size_of::() == size_of::()); assert!(offset_of!(ZSTD_rust_freeCCtxState, callback_context) == 0); assert!(offset_of!(ZSTD_rust_freeCCtxState, static_size) == size_of::()); assert!(offset_of!(ZSTD_rust_freeCCtxState, cctx_in_workspace) == 2 * size_of::()); - assert!(offset_of!(ZSTD_rust_freeCCtxState, free_content) == 3 * size_of::()); - assert!(offset_of!(ZSTD_rust_freeCCtxState, free_object) == 4 * size_of::()); - assert!(size_of::() == 5 * size_of::()); + assert!(offset_of!(ZSTD_rust_freeCCtxState, free_dictionaries) == 3 * size_of::()); + assert!(offset_of!(ZSTD_rust_freeCCtxState, free_mt_context) == 4 * size_of::()); + assert!(offset_of!(ZSTD_rust_freeCCtxState, free_workspace) == 5 * size_of::()); + assert!(offset_of!(ZSTD_rust_freeCCtxState, free_object) == 6 * size_of::()); + assert!(size_of::() == 7 * size_of::()); }; -/// Free the C-owned context content and, when applicable, its outer object. +/// Release a heap CCtx in the original dictionary, MT-context, workspace, and +/// outer-object order. #[no_mangle] pub unsafe extern "C" fn ZSTD_rust_freeCCtx(state: *const ZSTD_rust_freeCCtxState) -> usize { if state.is_null() { @@ -2591,7 +2595,9 @@ pub unsafe extern "C" fn ZSTD_rust_freeCCtx(state: *const ZSTD_rust_freeCCtxStat return ERROR(ZstdErrorCode::MemoryAllocation); } unsafe { - (state.free_content)(state.callback_context); + (state.free_dictionaries)(state.callback_context); + (state.free_mt_context)(state.callback_context); + (state.free_workspace)(state.callback_context); if state.cctx_in_workspace == 0 { (state.free_object)(state.callback_context); } @@ -19025,9 +19031,19 @@ mod tests { events: Vec<&'static str>, } - unsafe extern "C" fn free_cctx_test_content(context: *mut c_void) { + unsafe extern "C" fn free_cctx_test_dictionaries(context: *mut c_void) { let context = unsafe { &mut *context.cast::() }; - context.events.push("content"); + context.events.push("dictionaries"); + } + + unsafe extern "C" fn free_cctx_test_mt_context(context: *mut c_void) { + let context = unsafe { &mut *context.cast::() }; + context.events.push("mt-context"); + } + + unsafe extern "C" fn free_cctx_test_workspace(context: *mut c_void) { + let context = unsafe { &mut *context.cast::() }; + context.events.push("workspace"); } unsafe extern "C" fn free_cctx_test_object(context: *mut c_void) { @@ -19044,20 +19060,25 @@ mod tests { callback_context: context, static_size, cctx_in_workspace, - free_content: free_cctx_test_content, + free_dictionaries: free_cctx_test_dictionaries, + free_mt_context: free_cctx_test_mt_context, + free_workspace: free_cctx_test_workspace, free_object: free_cctx_test_object, } } #[test] - fn free_cctx_frees_content_before_an_external_object() { + fn free_cctx_releases_dictionaries_mt_context_and_workspace_before_object() { let mut context = FreeCCtxTestContext::default(); let state = free_cctx_test_state((&mut context as *mut FreeCCtxTestContext).cast(), 0, 0); let result = unsafe { ZSTD_rust_freeCCtx(&state) }; assert_eq!(result, 0); - assert_eq!(context.events, ["content", "object"]); + assert_eq!( + context.events, + ["dictionaries", "mt-context", "workspace", "object"] + ); } #[test] @@ -19068,7 +19089,7 @@ mod tests { let result = unsafe { ZSTD_rust_freeCCtx(&state) }; assert_eq!(result, 0); - assert_eq!(context.events, ["content"]); + assert_eq!(context.events, ["dictionaries", "mt-context", "workspace"]); } #[test]