Modernize to Rust 2024 edition and migrate from winapi to windows-sys#22
Open
coconutbird wants to merge 13 commits intodokan-dev:masterfrom
Open
Modernize to Rust 2024 edition and migrate from winapi to windows-sys#22coconutbird wants to merge 13 commits intodokan-dev:masterfrom
coconutbird wants to merge 13 commits intodokan-dev:masterfrom
Conversation
Member
|
Hey @coconutbird , really appreciate the update and adding the workflow! Did you have a chance to see why AppVeyor MSVC fails? Maybe that's a long-standing issue |
Author
it is long standing but I will look at it |
- Replace hardcoded sleep with synchronous mount point polling using list_mount_points() to wait for the previous unmount to complete before mounting again, preventing a race condition. - Remove check_pid from metadata-only handlers (get_file_information, find_files, find_streams) so system services probing the volume get valid responses instead of ACCESS_DENIED retry loops. - Keep check_pid in signal-sending handlers (read_file, write_file, flush_file_buffers, etc.) to prevent cache manager paging I/O from polluting the test signal channel.
- Replace tabs with spaces in doc comments (tabs_in_doc_comments) - Use ptr::read_unaligned instead of transmute_copy with raw pointer dereference in FileTimeOperation::from (not_unsafe_ptr_arg_deref) - Remove useless transmute of i64 to i64 in FindStreamData (useless_transmute) - Extract LockUnlockFn type alias to reduce type complexity (type_complexity)
The previous fix only polled list_mount_points before mounting. But the race occurs when the previous test's Drop (which calls unmount) is still being processed by the driver when the next test mounts. Now with_test_drive also waits after the drive thread joins, ensuring the driver has fully released Z:\ before the mutex is released and the next test can start. Extracted wait_for_z_unmounted() helper to deduplicate the polling logic.
DokanRemoveMountPoint is asynchronous - it signals the driver to release the mount point but returns before the teardown completes. Any caller who unmounts and remounts on the same drive letter risks a race where a pending release tears down the new mount. Add unmount_and_wait() that polls list_mount_points() until the mount point is fully released, with a configurable timeout. Update unmount() docs to clearly state it is asynchronous and recommend the new function for sequential unmount/remount scenarios. Replace hand-rolled polling in test harness with unmount_and_wait().
Author
|
The existing code base has a series of race conditions, I can re-structure the rust code to avoid all of this but I'm not sure if you'd appreciate that. |
The dispatch layer in operations.rs previously called the panicking info.context() method for all file operations except cleanup and close_file. When create_file fails (e.g. for system service probes), the context is never stored, but the driver still dispatches subsequent operations for that handle. Each of those operations would panic on the null context dereference. Replace all info.context() calls in the dispatch layer with info.try_context().ok_or(STATUS_INVALID_HANDLE)?, so that operations on handles without a valid context return STATUS_INVALID_HANDLE instead of panicking. This means user handlers are never called with invalid context - the guard is enforced by the dispatch layer. Deprecate the panicking context() method on OperationInfo in favor of try_context(), which returns Option and lets callers handle the null case gracefully.
Member
|
Thanks for looking at it! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Modernizes the entire workspace to Rust 2024 edition and migrates from the legacy
winapicrate towindows-sysv0.61.2.Changes
Dependency migration
winapi→windows-sys 0.61.2acrossdokan-sys,dokan, and thememfsexample.LPCWSTR/LPWSTRtype aliases withPCWSTR/PWSTRfromwindows_sys::core.SECURITY_DESCRIPTOR_REVISION,SEF_AVOID_PRIVILEGE_CHECK,FILE_CASE_PRESERVED_NAMES, etc.) with proper imports fromwindows-sys.parking_lotdev-dependency —std::sync::Mutexis now competitive on modern Rust (futex-based on Windows since 1.62).lazy_staticusage withstd::sync::LazyLock(stable since Rust 1.80).Rust 2024 edition
Cargo.tomlfiles toedition = "2024".unsafequalifier toexternblocks as required by the new edition.OperationInfo::context()that was always UB but now triggers a non-unwinding panic (abort) in Rust 2024. Addedtry_context()method with a null check, and updatedcleanup/close_filecallbacks to handle null context gracefully.CI
.github/workflows/build.yml) that builds and tests onwindows-latest.wingetso all tests (including driver-dependent ones) can run.std::sync::Mutexpoisoning in test harness to prevent cascading failures.GetFileSecurityqueries with unexpectedsecurity_informationvalues.Housekeeping
Cargo.lockto.gitignore.Testing
All 32 tests pass locally and in CI (Windows). 1 test ignored (
can_reset_timeout). Doctests pass.