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 memory leak, close SpooledTemporaryFile #106

Merged
merged 1 commit into from Jan 19, 2016

Conversation

Projects
None yet
2 participants
@kmmbvnr
Copy link
Contributor

commented Dec 21, 2015

No description provided.

@jschneier

This comment has been minimized.

Copy link
Owner

commented Jan 13, 2016

This looks good. How did you discover the leak (I want to see if I can write a test). Also, the same code (basically) exists on line 174 + 175 and I'm not able to puzzle out why it's there, any thoughts on that?

@kmmbvnr

This comment has been minimized.

Copy link
Contributor Author

commented Jan 17, 2016

We just run out of memory when getting files over OurModel.objects.filter(...) Django queryset caches all model instances, thats means even with restriction storage.max_memory_size it could takes too much memory.

I didn't remove lines 174 + 175 b/c _flush_write_buffer called also at line 153 (but i have no ideas, why someone would like to close file on a write call)

@jschneier

This comment has been minimized.

Copy link
Owner

commented Jan 19, 2016

Okay. I'll remove those lines after this, I don't think they make any sense. Thanks for the patch.

jschneier added a commit that referenced this pull request Jan 19, 2016

Merge pull request #106 from kmmbvnr/kmmbvnr-fix-mem-leak
Fix memory leak, close SpooledTemporaryFile

@jschneier jschneier merged commit 5b9363f into jschneier:master Jan 19, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

jschneier added a commit that referenced this pull request Jan 19, 2016

todofixthis referenced this pull request Jan 30, 2017

@pyup-bot pyup-bot referenced this pull request Mar 27, 2017

Open

Initial Update #94

@pyup-bot pyup-bot referenced this pull request Aug 12, 2017

Closed

Initial Update #49

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.