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 for cleaning up os calls through Popen #501

Merged
1 commit merged into from
Mar 17, 2017
Merged

Fix for cleaning up os calls through Popen #501

1 commit merged into from
Mar 17, 2017

Conversation

gyglim
Copy link
Contributor

@gyglim gyglim commented Mar 17, 2017

This PR adds the same _del__ code for AudioFileClip as there is for VideoFileClip:
https://github.com/Zulko/moviepy/blob/master/moviepy/video/io/VideoFileClip.py#L104

Despite using moviepy master I had problems with readers not being closed (leading to OSError: [Errno 24] Too many open files), similar to:
#57
#255

I have a complicated setup with a server that runs a long time and is multi-threaded (and the issue happens after days). To find and fix the issue I wrote a wrapper around Popen to log the calls opened and closed:
https://gist.github.com/gyglim/340c9d276d7f53578eb9673156e556df

This Pull Request together with explicitly calling video.__del__() resolved the issue.
Explicitly calling __del__() relates to this:
http://stackoverflow.com/questions/1481488/what-is-the-del-method-how-to-call-it/1481512#1481512

When calling __del__() without the fix in this PR, the readers of AudioFileClip would stay open.

@coveralls
Copy link

coveralls commented Mar 17, 2017

Coverage Status

Coverage increased (+0.02%) to 48.681% when pulling f612a45 on gyglim:fix_open_pipe into d633510 on Zulko:master.

@ghost ghost merged commit 3b9bf7a into Zulko:master Mar 17, 2017
@tburrows13 tburrows13 added the enhancement Positive change that does not change the API, i.e. improved performance, using less memory etc. label Apr 17, 2017
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Positive change that does not change the API, i.e. improved performance, using less memory etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants