Zlib memory leak and leastsize assertion #1116

Merged
merged 1 commit into from Nov 10, 2015

Conversation

Projects
None yet
2 participants
@etcimon
Contributor

etcimon commented May 21, 2015

This fixes an issue where zlib is not guaranteed to call deflateEnd or inflateEnd.

It also fixes an assertion failure that occurs when the underlying stream is closed prematurely.

leastsize should return 0 if it cannot read, rather than fail with an…
… assertion. Secondly, the memory must be guaranteed to be freed even when the stream cannot be finalized
@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Sep 13, 2015

Contributor

Bump

Contributor

etcimon commented Sep 13, 2015

Bump

@etcimon etcimon referenced this pull request Nov 10, 2015

Closed

HTTP client leaks memory. #1321

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Nov 10, 2015

Member

Sorry for having forgot about this. I meant to comment on this for a while.

Member

s-ludwig commented Nov 10, 2015

Sorry for having forgot about this. I meant to comment on this for a while.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Nov 10, 2015

Member

I still need to understand the issue with that assertion. This should really have a unit test, but I think that it would be a little safer to, instead of removing the assertion, to set the m_finished flag when a premature end of input is detected.

Member

s-ludwig commented Nov 10, 2015

I still need to understand the issue with that assertion. This should really have a unit test, but I think that it would be a little safer to, instead of removing the assertion, to set the m_finished flag when a premature end of input is detected.

@etcimon

This comment has been minimized.

Show comment
Hide comment
@etcimon

etcimon Nov 10, 2015

Contributor

The issue is when the connection closes (for an unlimited amount of non-program-related reasons), readChunk -> m_in.leastSize returns 0 and zlib stream sees a premature end of input, and fails on this assertion, so it stands out from other streams where you can end up with a closed connection and fail an enforcement at a higher level to e.g. try again, rather than having the task end unexpectedly as a whole.

Contributor

etcimon commented Nov 10, 2015

The issue is when the connection closes (for an unlimited amount of non-program-related reasons), readChunk -> m_in.leastSize returns 0 and zlib stream sees a premature end of input, and fails on this assertion, so it stands out from other streams where you can end up with a closed connection and fail an enforcement at a higher level to e.g. try again, rather than having the task end unexpectedly as a whole.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Nov 10, 2015

Member

I've fixed this locally by adding a check for empty input in readChunk. I'll pull and then commit my changes on top.

Member

s-ludwig commented Nov 10, 2015

I've fixed this locally by adding a check for empty input in readChunk. I'll pull and then commit my changes on top.

s-ludwig added a commit that referenced this pull request Nov 10, 2015

Merge pull request #1116 from etcimon/zlib-fix
Zlib memory leak and leastsize assertion

@s-ludwig s-ludwig merged commit 731bd01 into vibe-d:master Nov 10, 2015

1 check passed

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

s-ludwig added a commit that referenced this pull request Nov 10, 2015

Add back assertion and fix root cause. See #1116.
Also adds simple unit tests for GzipInputStream, including a partial input test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment