Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rm: move open DLLs to temp dir to allow dir to be deleted #53456

Merged
merged 8 commits into from
Feb 27, 2024

Conversation

IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Feb 24, 2024

Based on findings in #53394, loaded (memory mapped) DLLs cannot be deleted on Windows, even with posix delete mode, but they can be moved without breaking the loaded library. So move out to the temp dir to allow the dir to be deleted and the DLL will be deleted via the temp dir cleanup on next reboot. Turns out Windows doesn't tidy up the temp dir automatically.. So this saves them to a fixed temp dir that Pkg.gc will tidy up JuliaLang/Pkg.jl#3812

base/file.jl Outdated
if err.code==Base.UV_EACCES && endswith(path, ".dll")
# Loaded DLLs cannot be deleted on Windows, even with posix delete mode
# but they can be moved. So move out to allow the dir to be deleted
# and DLL deleted via the temp dir cleanup on next reboot
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: tempdir is never cleaned up on Windows

@IanButterworth IanButterworth marked this pull request as ready for review February 25, 2024 00:47
base/file.jl Outdated
Comment on lines 284 to 285
# TODO: Add a mechanism to delete these moved files after dlclose or process exit
dir = mkpath(joinpath(tempdir(), "julia_delayed_deletes"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding something to Pkg.gc() that checks for this folder and attempts to delete everything inside (silently failing if something is still opened) should work well. Moving it outside of the depot is good in once sense, as it allows the depot to be deleted, but it's bad in another sense in that it's more likely to be on a different disk, which will probably fail the mv() as I doubt you can move a mmap'ed file across device boundaries. That's probably quite an edge case however.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sounds good JuliaLang/Pkg.jl#3812

@IanButterworth
Copy link
Member Author

This is ready to go from my perspective. Windows CI looks clean.
I'll follow up with JuliaLang/Pkg.jl#3812

@IanButterworth IanButterworth merged commit 2e9b0bb into JuliaLang:master Feb 27, 2024
9 checks passed
@IanButterworth IanButterworth deleted the ib/win_dll_cleanup branch February 27, 2024 02:42
tecosaur pushed a commit to tecosaur/julia that referenced this pull request Mar 4, 2024
mkitti pushed a commit to mkitti/julia that referenced this pull request Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants