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

Windows fixes for leaking file handles #37 #38

Merged
merged 7 commits into from
May 28, 2017

Conversation

stuertz
Copy link
Contributor

@stuertz stuertz commented Mar 27, 2017

Fix for #37.
All tests run also on windows now.

stuertz added 7 commits March 26, 2017 15:45
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.
@ghost
Copy link

ghost commented Mar 27, 2017

@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.

@ankostis
Copy link
Contributor

My +1 gratitude to you @stuertz for this, thanks.

@ankostis
Copy link
Contributor

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 ankostis self-assigned this Mar 27, 2017
@Byron
Copy link
Member

Byron commented Apr 9, 2017

@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.
@stuertz Sorry again for taking so much time - it's not a sign of depreciation of this PR.

@stuertz
Copy link
Contributor Author

stuertz commented Apr 11, 2017

@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 :-)

@Byron
Copy link
Member

Byron commented May 28, 2017

@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.

@Byron Byron merged commit 4ddd1a3 into gitpython-developers:master May 28, 2017
@stuertz stuertz deleted the windows_fixes branch October 30, 2017 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants