From 998c88c97b543c5ec445568c51cb1657ec8ccf79 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Mon, 20 Jul 2026 06:02:52 +0200 Subject: [PATCH] refactor(compress): move dictionary teardown ordering to Rust The compression context still owns private dictionary storage and CDict objects, but the teardown order is observable because each step can invoke an allocator or clear state used by later steps. Keep those storage operations as C callbacks while moving the sequencing policy into the Rust compression module. The C bridge now contains only callback adapters and compile-time ABI layout checks, and the Rust unit test pins the original five-step order. Test Plan: - `cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings` -- passed - `cargo test --manifest-path rust/Cargo.toml --all-targets` -- 771 passed - `cargo clippy --manifest-path rust/cli/Cargo.toml --all-targets -- -D warnings` -- passed - `make -j1` under `ulimit -v 41943040` -- passed - `make -j1 -C tests test` under `ulimit -v 41943040` -- passed --- lib/compress/zstd_compress.c | 67 ++++++++++++++++++-- rust/src/zstd_compress.rs | 118 +++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 5 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 116b064fd..b36d6d81f 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -444,6 +444,30 @@ typedef char ZSTD_rust_init_static_cdict_state_layout[ == 8 * sizeof(void*) && sizeof(ZSTD_rust_initStaticCDictState) == 9 * sizeof(void*)) ? 1 : -1]; +typedef void (*ZSTD_rust_clearAllDictsCallback_f)(void* context); +typedef struct { + void* callbackContext; + ZSTD_rust_clearAllDictsCallback_f freeLocalDictBuffer; + ZSTD_rust_clearAllDictsCallback_f freeLocalCDict; + ZSTD_rust_clearAllDictsCallback_f clearLocalDict; + ZSTD_rust_clearAllDictsCallback_f clearPrefixDict; + ZSTD_rust_clearAllDictsCallback_f clearCDict; +} ZSTD_rust_clearAllDictsState; +void ZSTD_rust_clearAllDicts(const ZSTD_rust_clearAllDictsState* state); +typedef char ZSTD_rust_clear_all_dicts_state_layout[ + (offsetof(ZSTD_rust_clearAllDictsState, callbackContext) == 0 + && offsetof(ZSTD_rust_clearAllDictsState, freeLocalDictBuffer) + == sizeof(void*) + && offsetof(ZSTD_rust_clearAllDictsState, freeLocalCDict) + == 2 * sizeof(void*) + && offsetof(ZSTD_rust_clearAllDictsState, clearLocalDict) + == 3 * sizeof(void*) + && offsetof(ZSTD_rust_clearAllDictsState, clearPrefixDict) + == 4 * sizeof(void*) + && offsetof(ZSTD_rust_clearAllDictsState, clearCDict) + == 5 * sizeof(void*) + && sizeof(ZSTD_rust_clearAllDictsState) == 6 * sizeof(void*)) + ? 1 : -1]; typedef void (*ZSTD_rust_resetCCtxClearAllDicts_f)(void* context); typedef size_t (*ZSTD_rust_resetCCtxResetParams_f)(void* context); typedef struct { @@ -3214,13 +3238,46 @@ ZSTD_CCtx* ZSTD_initStaticCCtx(void* workspace, size_t workspaceSize) /** * Clears and frees all of the dictionaries in the CCtx. */ +static void ZSTD_clearAllDicts_freeLocalDictBuffer(void* context) +{ + ZSTD_CCtx* const cctx = (ZSTD_CCtx*)context; + ZSTD_customFree(cctx->localDict.dictBuffer, cctx->customMem); +} + +static void ZSTD_clearAllDicts_freeLocalCDict(void* context) +{ + ZSTD_CCtx* const cctx = (ZSTD_CCtx*)context; + ZSTD_freeCDict(cctx->localDict.cdict); +} + +static void ZSTD_clearAllDicts_clearLocalDict(void* context) +{ + ZSTD_CCtx* const cctx = (ZSTD_CCtx*)context; + ZSTD_memset(&cctx->localDict, 0, sizeof(cctx->localDict)); +} + +static void ZSTD_clearAllDicts_clearPrefixDict(void* context) +{ + ZSTD_CCtx* const cctx = (ZSTD_CCtx*)context; + ZSTD_memset(&cctx->prefixDict, 0, sizeof(cctx->prefixDict)); +} + +static void ZSTD_clearAllDicts_clearCDict(void* context) +{ + ZSTD_CCtx* const cctx = (ZSTD_CCtx*)context; + cctx->cdict = NULL; +} + static void ZSTD_clearAllDicts(ZSTD_CCtx* cctx) { - ZSTD_customFree(cctx->localDict.dictBuffer, cctx->customMem); - ZSTD_freeCDict(cctx->localDict.cdict); - ZSTD_memset(&cctx->localDict, 0, sizeof(cctx->localDict)); - ZSTD_memset(&cctx->prefixDict, 0, sizeof(cctx->prefixDict)); - cctx->cdict = NULL; + ZSTD_rust_clearAllDictsState state; + state.callbackContext = cctx; + state.freeLocalDictBuffer = ZSTD_clearAllDicts_freeLocalDictBuffer; + state.freeLocalCDict = ZSTD_clearAllDicts_freeLocalCDict; + state.clearLocalDict = ZSTD_clearAllDicts_clearLocalDict; + state.clearPrefixDict = ZSTD_clearAllDicts_clearPrefixDict; + state.clearCDict = ZSTD_clearAllDicts_clearCDict; + ZSTD_rust_clearAllDicts(&state); } static void ZSTD_clearAllDicts_callback(void* context) diff --git a/rust/src/zstd_compress.rs b/rust/src/zstd_compress.rs index e447a3658..365174b38 100644 --- a/rust/src/zstd_compress.rs +++ b/rust/src/zstd_compress.rs @@ -2263,6 +2263,51 @@ pub unsafe extern "C" fn ZSTD_rust_freeCCtx(state: *const ZSTD_rust_freeCCtxStat 0 } +type ClearAllDictsCallbackFn = unsafe extern "C" fn(*mut c_void); + +/// Compose the dictionary teardown operations while leaving the private +/// ZSTD_CCtx fields behind C callbacks. +#[repr(C)] +pub struct ZSTD_rust_clearAllDictsState { + callback_context: *mut c_void, + free_local_dict_buffer: ClearAllDictsCallbackFn, + free_local_cdict: ClearAllDictsCallbackFn, + clear_local_dict: ClearAllDictsCallbackFn, + clear_prefix_dict: ClearAllDictsCallbackFn, + clear_cdict: ClearAllDictsCallbackFn, +} + +const _: () = { + assert!(size_of::() == size_of::()); + assert!(offset_of!(ZSTD_rust_clearAllDictsState, callback_context) == 0); + assert!(offset_of!(ZSTD_rust_clearAllDictsState, free_local_dict_buffer) == size_of::()); + assert!(offset_of!(ZSTD_rust_clearAllDictsState, free_local_cdict) == 2 * size_of::()); + assert!(offset_of!(ZSTD_rust_clearAllDictsState, clear_local_dict) == 3 * size_of::()); + assert!(offset_of!(ZSTD_rust_clearAllDictsState, clear_prefix_dict) == 4 * size_of::()); + assert!(offset_of!(ZSTD_rust_clearAllDictsState, clear_cdict) == 5 * size_of::()); + assert!(size_of::() == 6 * size_of::()); +}; + +/// Preserve the original local-dictionary, prefix-dictionary, and attached +/// CDict teardown order while keeping their private storage C-owned. +#[no_mangle] +pub unsafe extern "C" fn ZSTD_rust_clearAllDicts(state: *const ZSTD_rust_clearAllDictsState) { + if state.is_null() { + return; + } + let state = unsafe { &*state }; + if state.callback_context.is_null() { + return; + } + unsafe { + (state.free_local_dict_buffer)(state.callback_context); + (state.free_local_cdict)(state.callback_context); + (state.clear_local_dict)(state.callback_context); + (state.clear_prefix_dict)(state.callback_context); + (state.clear_cdict)(state.callback_context); + } +} + type ResetCCtxClearAllDictsFn = unsafe extern "C" fn(*mut c_void); type ResetCCtxResetParamsFn = unsafe extern "C" fn(*mut c_void) -> usize; @@ -15606,6 +15651,79 @@ mod tests { assert_eq!(non_contiguous_state.lowLimit, 14); } + #[derive(Default)] + struct ClearAllDictsTestContext { + events: Vec<&'static str>, + } + + unsafe fn clear_all_dicts_test_context( + context: *mut c_void, + ) -> &'static mut ClearAllDictsTestContext { + unsafe { &mut *context.cast::() } + } + + unsafe extern "C" fn clear_all_dicts_test_free_local_dict_buffer(context: *mut c_void) { + unsafe { clear_all_dicts_test_context(context) } + .events + .push("free-local-buffer"); + } + + unsafe extern "C" fn clear_all_dicts_test_free_local_cdict(context: *mut c_void) { + unsafe { clear_all_dicts_test_context(context) } + .events + .push("free-local-cdict"); + } + + unsafe extern "C" fn clear_all_dicts_test_clear_local_dict(context: *mut c_void) { + unsafe { clear_all_dicts_test_context(context) } + .events + .push("clear-local"); + } + + unsafe extern "C" fn clear_all_dicts_test_clear_prefix_dict(context: *mut c_void) { + unsafe { clear_all_dicts_test_context(context) } + .events + .push("clear-prefix"); + } + + unsafe extern "C" fn clear_all_dicts_test_clear_cdict(context: *mut c_void) { + unsafe { clear_all_dicts_test_context(context) } + .events + .push("clear-cdict"); + } + + fn clear_all_dicts_test_state( + context: &mut ClearAllDictsTestContext, + ) -> ZSTD_rust_clearAllDictsState { + ZSTD_rust_clearAllDictsState { + callback_context: (context as *mut ClearAllDictsTestContext).cast(), + free_local_dict_buffer: clear_all_dicts_test_free_local_dict_buffer, + free_local_cdict: clear_all_dicts_test_free_local_cdict, + clear_local_dict: clear_all_dicts_test_clear_local_dict, + clear_prefix_dict: clear_all_dicts_test_clear_prefix_dict, + clear_cdict: clear_all_dicts_test_clear_cdict, + } + } + + #[test] + fn clear_all_dicts_preserves_dictionary_teardown_order() { + let mut context = ClearAllDictsTestContext::default(); + let state = clear_all_dicts_test_state(&mut context); + + unsafe { ZSTD_rust_clearAllDicts(&state) }; + + assert_eq!( + context.events, + [ + "free-local-buffer", + "free-local-cdict", + "clear-local", + "clear-prefix", + "clear-cdict" + ] + ); + } + #[derive(Default)] struct ResetCCtxTestContext { events: Vec<&'static str>,