-
Notifications
You must be signed in to change notification settings - Fork 68
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
Windows fixes for leaking file handles #37 #38
Conversation
Currently renaming files is not supported while the the OS doesn't support renaming open files. When closing the file, as done in the code by using http://smmap.readthedocs.io/en/latest/api.html#smmap.mman.StaticWindowMapManager.force_map_handle_removal_win force_map_handle_removal_win, we can rename, but the cache does still have a handle to this file and crashes.
Sometimes the OS or some other process has the handle to file a bit longer, and the file could not be deleted immediatly. Retry 10 Times with 100ms distance.
@stuertz Thanks for your contribution! Just as a word of warning you should know that I am way behind my GitPython related maintenance work. A lot piled up, and this PR will eventually be handled. Thanks for your understanding. |
My +1 gratitude to you @stuertz for this, thanks. |
On me, for not forgetting to check if this PR also fixes some more "hidden" leak-errors in Gitpython (see gitpython-developers/GitPython#525). |
@ankostis Thanks for looking into this. I would offer some help, but have long lost track of what it means to get it to work on windows - sorry for that. Please feel free to merge whenever you think the PR is ready. |
@Byron: I understand and accept that you are behind. But this at least makes the code usable on windows again and IMHO does not brake anything :-). Maybe the solution is currently not ideal, but helps the windows users a lot. So please consider to just merge and release the version :-) |
@stuertz Thanks for all the fixes, and sorry again for not following up on this one. I am currently working to catch up on GitPython/GitDB, and a new release should be a result of this. |
Fix for #37.
All tests run also on windows now.