diff --git a/programs/fileio.c b/programs/fileio.c index c3754504e..81b9991d7 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -380,8 +380,11 @@ int FIO_rust_setDictBufferMMap(const char* fileName, void** buffer, size_t* mappedSize, void** dictHandle); -void FIO_rust_munmapDict(void** buffer, size_t* bufferSize, void** dictHandle); #endif +void FIO_rust_freeDict(int dictBufferType, + void** buffer, + size_t* bufferSize, + void** dictHandle); int FIO_rust_removeFile(const char* path); int FIO_rust_passThrough(ReadPoolCtx_t* readCtx, WritePoolCtx_t* writeCtx); #ifdef ZSTD_LZ4COMPRESS @@ -656,10 +659,6 @@ static size_t FIO_setDictBufferMalloc(FIO_Dict_t* dict, const char* fileName, FI } #if (PLATFORM_POSIX_VERSION > 0) -static void FIO_munmap(FIO_Dict_t* dict) -{ - FIO_rust_munmapDict(&dict->dictBuffer, &dict->dictBufferSize, NULL); -} static size_t FIO_setDictBufferMMap(FIO_Dict_t* dict, const char* fileName, FIO_prefs_t* const prefs, stat_t* dictFileStat) { U64 fileSize; @@ -701,12 +700,6 @@ static size_t FIO_setDictBufferMMap(FIO_Dict_t* dict, const char* fileName, FIO_ return 0; } #elif defined(_MSC_VER) || defined(_WIN32) -static void FIO_munmap(FIO_Dict_t* dict) -{ - FIO_rust_munmapDict(&dict->dictBuffer, - &dict->dictBufferSize, - (void**)&dict->dictHandle); -} static size_t FIO_setDictBufferMMap(FIO_Dict_t* dict, const char* fileName, FIO_prefs_t* const prefs, stat_t* dictFileStat) { U64 fileSize; @@ -755,26 +748,30 @@ static size_t FIO_setDictBufferMMap(FIO_Dict_t* dict, const char* fileName, FIO_ { return FIO_setDictBufferMalloc(dict, fileName, prefs, dictFileStat); } -static void FIO_munmap(FIO_Dict_t* dict) { - free(dict->dictBuffer); - dict->dictBuffer = NULL; - dict->dictBufferSize = 0; -} #endif static void FIO_freeDict(FIO_Dict_t* dict) { - if (dict->dictBufferType == FIO_mallocDict) { - free(dict->dictBuffer); - dict->dictBuffer = NULL; - dict->dictBufferSize = 0; - } else if (dict->dictBufferType == FIO_mmapDict) { - FIO_munmap(dict); - } else { + if (dict->dictBufferType != FIO_mallocDict + && dict->dictBufferType != FIO_mmapDict) { assert(0); /* Should not reach this case */ + return; } + + FIO_rust_freeDict((int)dict->dictBufferType, + &dict->dictBuffer, + &dict->dictBufferSize, +#if defined(_MSC_VER) || defined(_WIN32) + (void**)&dict->dictHandle +#else + NULL +#endif + ); } static void FIO_initDict(FIO_Dict_t* dict, const char* fileName, FIO_prefs_t* const prefs, stat_t* dictFileStat, FIO_dictBufferType_t dictBufferType) { +#if defined(_MSC_VER) || defined(_WIN32) + dict->dictHandle = NULL; +#endif dict->dictBufferType = dictBufferType; if (dict->dictBufferType == FIO_mallocDict) { dict->dictBufferSize = FIO_setDictBufferMalloc(dict, fileName, prefs, dictFileStat); diff --git a/rust/src/fileio_backend.rs b/rust/src/fileio_backend.rs index 26f1e62f4..1be321a6b 100644 --- a/rust/src/fileio_backend.rs +++ b/rust/src/fileio_backend.rs @@ -454,6 +454,13 @@ pub unsafe extern "C" fn FIO_rust_munmapDict( } } + #[cfg(not(any(unix, windows)))] + { + if !mapping.is_null() { + unsafe { libc::free(mapping) }; + } + } + unsafe { *buffer = ptr::null_mut(); *buffer_size = 0; @@ -463,6 +470,47 @@ pub unsafe extern "C" fn FIO_rust_munmapDict( } } +const FIO_MALLOC_DICT: c_int = 0; +const FIO_MMAP_DICT: c_int = 1; + +/// Releases either kind of dictionary buffer and clears its C-visible fields. +/// +/// C retains the enum validation and calls this leaf only for the two valid +/// `FIO_dictBufferType_t` values. Mapped buffers go through the existing +/// platform-aware release leaf so Windows views and file handles remain paired +/// correctly, while malloc-backed buffers use the matching libc allocator. +#[no_mangle] +pub unsafe extern "C" fn FIO_rust_freeDict( + dict_buffer_type: c_int, + buffer: *mut *mut c_void, + buffer_size: *mut usize, + dict_handle: *mut *mut c_void, +) { + match dict_buffer_type { + FIO_MALLOC_DICT => { + if !buffer.is_null() { + let allocation = unsafe { *buffer }; + if !allocation.is_null() { + unsafe { libc::free(allocation) }; + } + } + unsafe { + if !buffer.is_null() { + *buffer = ptr::null_mut(); + } + if !buffer_size.is_null() { + *buffer_size = 0; + } + if !dict_handle.is_null() { + *dict_handle = ptr::null_mut(); + } + } + } + FIO_MMAP_DICT => unsafe { FIO_rust_munmapDict(buffer, buffer_size, dict_handle) }, + _ => {} + } +} + #[cfg(test)] mod tests { use super::*; @@ -664,12 +712,59 @@ mod tests { let mapped = unsafe { std::slice::from_raw_parts(buffer.cast::(), mapped_size) }; assert_eq!(mapped, payload); - unsafe { FIO_rust_munmapDict(&mut buffer, &mut mapped_size, ptr::null_mut()) }; + let mut dict_handle = 1usize as *mut c_void; + unsafe { + FIO_rust_freeDict( + FIO_MMAP_DICT, + &mut buffer, + &mut mapped_size, + &mut dict_handle, + ) + }; assert!(buffer.is_null()); assert_eq!(mapped_size, 0); + assert!(dict_handle.is_null()); + + // The release leaf is idempotent after the dispatch has cleared the + // ownership fields, so cleanup cannot double-release the mapping. + unsafe { FIO_rust_munmapDict(&mut buffer, &mut mapped_size, &mut dict_handle) }; fs::remove_file(path).expect("remove temporary file"); } + #[test] + fn frees_malloc_dictionary_and_clears_ownership() { + let allocation = unsafe { libc::malloc(16) }; + assert!(!allocation.is_null()); + let mut buffer = allocation; + let mut buffer_size = 16; + let mut dict_handle = 1usize as *mut c_void; + + unsafe { + FIO_rust_freeDict( + FIO_MALLOC_DICT, + &mut buffer, + &mut buffer_size, + &mut dict_handle, + ) + }; + assert!(buffer.is_null()); + assert_eq!(buffer_size, 0); + assert!(dict_handle.is_null()); + + // A second release sees only cleared fields and must not double-free. + unsafe { + FIO_rust_freeDict( + FIO_MALLOC_DICT, + &mut buffer, + &mut buffer_size, + &mut dict_handle, + ) + }; + assert!(buffer.is_null()); + assert_eq!(buffer_size, 0); + assert!(dict_handle.is_null()); + } + #[cfg(unix)] #[test] fn maps_empty_dictionary_with_owned_zero_size_sentinel() {