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

Fix potential memory leaks by confining use of pyvips to subprocesses #117

Merged
merged 4 commits into from
Jan 23, 2025

Conversation

pkcakeout
Copy link
Member

Calls to libvips that cause errors are suspected to cause memory leaks in libvips. In order to prevent this, this PR confines calls to pyvips to subprocesses spawned using ProcessPoolExecutor.

@pkcakeout
Copy link
Member Author

Did a few spot-checks regarding wait times and error propagation behavior... seems to be alright.

@pkcakeout pkcakeout changed the title Confine used of pyvips to subprocesses Confine use of pyvips to subprocesses Jan 21, 2025
@pkcakeout pkcakeout changed the title Confine use of pyvips to subprocesses Fix potential memory leaks by confining use of pyvips to subprocesses Jan 21, 2025
@jmsmkn
Copy link
Member

jmsmkn commented Jan 23, 2025

@pkcakeout I was working on the openslide python binding removal and saw a few calls missing so put these changes in #118, could you take a look at that? Only thing missing is the globals stuff but I think that is unnecessary after a refactoring.

I couldn't remove openslide with confidence so gave up on that and moved the converter passing removal here.

@jmsmkn jmsmkn merged commit d51d72b into main Jan 23, 2025
12 checks passed
@jmsmkn jmsmkn deleted the confine-used-of-pyvips-to-subprocesses branch January 23, 2025 15:05
@pkcakeout
Copy link
Member Author

I couldn't remove openslide with confidence

Had the same issue. I also was not comfortable with modifying the tests to the extent you did, so thanks for picking that up! 👍

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.

2 participants