refactor(mt): move final job-table teardown order to Rust
Normal MT context teardown still let the C wrapper directly sequence per-job synchronization destruction before Rust released the opaque job-table storage, while failed create and expansion transactions already used a Rust helper for that same order. Route the normal teardown through a repr(C) projection so Rust owns the destroy-before-storage-free policy consistently. The synchronization callback, descriptor storage, and custom allocator remain opaque C-owned operations. Add matching C/Rust layout assertions and a focused callback-order test. Test Plan: - `git diff --cached --check` -- passed - Cargo, make, native tests, and heavy verification were not run per request
This commit is contained in:
+130
-3
@@ -4654,6 +4654,45 @@ unsafe fn free_job_table_storage(job_table: *mut c_void, custom_mem: ZstdCustomM
|
||||
type JobTableInitSync = unsafe extern "C" fn(*mut c_void, c_uint, usize) -> c_int;
|
||||
type JobTableDestroySync = unsafe extern "C" fn(*mut c_void, c_uint, usize);
|
||||
|
||||
/// Projection for the final MT job-table release. Rust owns the
|
||||
/// destroy-before-storage-free order; C retains the descriptor synchronization
|
||||
/// callback and custom allocator behind this opaque projection.
|
||||
#[repr(C)]
|
||||
#[derive(Clone, Copy)]
|
||||
pub struct ZSTDMT_RustFreeJobsTableProjection {
|
||||
jobTable: *mut c_void,
|
||||
nbJobs: c_uint,
|
||||
jobSize: usize,
|
||||
customMem: ZstdCustomMem,
|
||||
destroySync: Option<JobTableDestroySync>,
|
||||
}
|
||||
|
||||
const _: () = {
|
||||
assert!(size_of::<JobTableDestroySync>() == size_of::<usize>());
|
||||
assert!(offset_of!(ZSTDMT_RustFreeJobsTableProjection, jobTable) == 0);
|
||||
assert!(
|
||||
offset_of!(ZSTDMT_RustFreeJobsTableProjection, nbJobs) == size_of::<usize>()
|
||||
);
|
||||
assert!(
|
||||
offset_of!(ZSTDMT_RustFreeJobsTableProjection, jobSize)
|
||||
== if size_of::<usize>() == 8 { 16 } else { 8 }
|
||||
);
|
||||
assert!(
|
||||
offset_of!(ZSTDMT_RustFreeJobsTableProjection, customMem)
|
||||
== offset_of!(ZSTDMT_RustFreeJobsTableProjection, jobSize) + size_of::<usize>()
|
||||
);
|
||||
assert!(
|
||||
offset_of!(ZSTDMT_RustFreeJobsTableProjection, destroySync)
|
||||
== offset_of!(ZSTDMT_RustFreeJobsTableProjection, customMem)
|
||||
+ size_of::<ZstdCustomMem>()
|
||||
);
|
||||
assert!(
|
||||
size_of::<ZSTDMT_RustFreeJobsTableProjection>()
|
||||
== offset_of!(ZSTDMT_RustFreeJobsTableProjection, destroySync)
|
||||
+ size_of::<JobTableDestroySync>()
|
||||
);
|
||||
};
|
||||
|
||||
unsafe fn free_job_table_with_sync(
|
||||
job_table: *mut c_void,
|
||||
nb_jobs: c_uint,
|
||||
@@ -4661,11 +4700,29 @@ unsafe fn free_job_table_with_sync(
|
||||
custom_mem: ZstdCustomMem,
|
||||
destroy_sync: JobTableDestroySync,
|
||||
) {
|
||||
if job_table.is_null() {
|
||||
let projection = ZSTDMT_RustFreeJobsTableProjection {
|
||||
jobTable: job_table,
|
||||
nbJobs: nb_jobs,
|
||||
jobSize: job_size,
|
||||
customMem: custom_mem,
|
||||
destroySync: Some(destroy_sync),
|
||||
};
|
||||
unsafe { free_job_table_with_sync_projection(projection) };
|
||||
}
|
||||
|
||||
unsafe fn free_job_table_with_sync_projection(
|
||||
projection: ZSTDMT_RustFreeJobsTableProjection,
|
||||
) {
|
||||
if projection.jobTable.is_null() {
|
||||
return;
|
||||
}
|
||||
unsafe { destroy_sync(job_table, nb_jobs, job_size) };
|
||||
unsafe { ZSTDMT_rust_job_table_free(job_table, nb_jobs, job_size, custom_mem) };
|
||||
let Some(destroy_sync) = projection.destroySync else {
|
||||
return;
|
||||
};
|
||||
unsafe {
|
||||
destroy_sync(projection.jobTable, projection.nbJobs, projection.jobSize);
|
||||
free_job_table_storage(projection.jobTable, projection.customMem);
|
||||
}
|
||||
}
|
||||
|
||||
unsafe fn create_job_table_with_sync(
|
||||
@@ -4777,6 +4834,19 @@ pub unsafe extern "C" fn ZSTDMT_rust_job_table_free(
|
||||
unsafe { free_job_table_storage(job_table, custom_mem) }
|
||||
}
|
||||
|
||||
/// Release the C-owned synchronization objects before Rust releases the
|
||||
/// opaque job-table storage. This is the normal teardown counterpart to the
|
||||
/// same order used by failed create/expand transactions.
|
||||
#[no_mangle]
|
||||
pub unsafe extern "C" fn ZSTDMT_rust_freeJobsTable(
|
||||
projection: *const ZSTDMT_RustFreeJobsTableProjection,
|
||||
) {
|
||||
let Some(projection) = (unsafe { projection.as_ref() }).copied() else {
|
||||
return;
|
||||
};
|
||||
unsafe { free_job_table_with_sync_projection(projection) };
|
||||
}
|
||||
|
||||
/// Expand the C-owned MT job table when the requested worker count outgrows
|
||||
/// its masked capacity. The table storage and custom allocator calls stay in
|
||||
/// Rust; C supplies callbacks for the private descriptor synchronization
|
||||
@@ -8862,6 +8932,63 @@ mod tests {
|
||||
JOB_TABLE_DESTROY_CALLS.fetch_add(1, Ordering::Relaxed);
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct FreeJobsTableTestContext {
|
||||
events: Vec<&'static str>,
|
||||
}
|
||||
|
||||
unsafe extern "C" fn free_jobs_table_test_alloc(
|
||||
_opaque: *mut c_void,
|
||||
_size: usize,
|
||||
) -> *mut c_void {
|
||||
ptr::null_mut()
|
||||
}
|
||||
|
||||
unsafe extern "C" fn free_jobs_table_test_destroy(
|
||||
job_table: *mut c_void,
|
||||
_nb_jobs: c_uint,
|
||||
_job_size: usize,
|
||||
) {
|
||||
unsafe {
|
||||
(*job_table.cast::<FreeJobsTableTestContext>())
|
||||
.events
|
||||
.push("destroy-sync");
|
||||
}
|
||||
}
|
||||
|
||||
unsafe extern "C" fn free_jobs_table_test_free(
|
||||
opaque: *mut c_void,
|
||||
allocation: *mut c_void,
|
||||
) {
|
||||
assert_eq!(allocation, opaque);
|
||||
unsafe {
|
||||
(*opaque.cast::<FreeJobsTableTestContext>())
|
||||
.events
|
||||
.push("free-storage");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn free_jobs_table_orders_sync_destruction_before_storage_release() {
|
||||
let mut context = FreeJobsTableTestContext::default();
|
||||
let context_ptr = (&mut context as *mut FreeJobsTableTestContext).cast::<c_void>();
|
||||
let projection = ZSTDMT_RustFreeJobsTableProjection {
|
||||
jobTable: context_ptr,
|
||||
nbJobs: 8,
|
||||
jobSize: 128,
|
||||
customMem: ZstdCustomMem {
|
||||
customAlloc: Some(free_jobs_table_test_alloc),
|
||||
customFree: Some(free_jobs_table_test_free),
|
||||
opaque: context_ptr,
|
||||
},
|
||||
destroySync: Some(free_jobs_table_test_destroy),
|
||||
};
|
||||
|
||||
unsafe { ZSTDMT_rust_freeJobsTable(&projection) };
|
||||
|
||||
assert_eq!(context.events, ["destroy-sync", "free-storage"]);
|
||||
}
|
||||
|
||||
fn rsync_prime_power() -> u64 {
|
||||
(0..RSYNC_LENGTH - 1).fold(1, |power, _| power.wrapping_mul(PRIME8_BYTES))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user