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

Release buffer if function returns prematurely #4381

Merged
merged 1 commit into from Mar 25, 2020

Conversation

radarhere
Copy link
Member

@radarhere radarhere commented Jan 25, 2020

Resolves #4329

In PyImaging_MapBuffer, Py_Buffer view is set up, and when the image is destroyed, it is released. However, if the function returns before that, then view is not released.

The setup calls PyObject_GetBuffer, which is documented to require PyBuffer_Release - https://python.readthedocs.io/en/stable/c-api/buffer.html#c.PyObject_GetBuffer

Successful calls to PyObject_GetBuffer() must be paired with calls to PyBuffer_Release(), similar to malloc() and free(). Thus, after the consumer is done with the buffer, PyBuffer_Release() must be called exactly once.

So this PR fixes a memory leak that is reported in the issue.

@radarhere
Copy link
Member Author

Regarding the lack of coverage, it is pre-existing, and I'm not convinced that these lines can be covered, practically speaking.

To trigger "buffer has negative size", PyObject_GetBuffer in PyImaging_GetBuffer would have to return a negative length.

To cause ImagingNewPrologueSubtype to fail, either we would need to run out of memory, or a very large file would be required to trip an overflow check without first tripping "buffer is not large enough".

@python-pillow python-pillow deleted a comment from codecov bot Feb 1, 2020
@hugovk hugovk merged commit 5e4b6e9 into python-pillow:master Mar 25, 2020
@hugovk
Copy link
Member

hugovk commented Mar 25, 2020

Thanks!

@radarhere radarhere deleted the memory branch March 25, 2020 19:49
This was referenced Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory leak when parsing icon file
2 participants