Fix a rare crash caused by an unchecked return value. #255
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
March 31, 2014 18:37
Log the streamError whenever we fail to write during compression, and log any failures when removing the original file or cleaning up the temporary file after compression failed.
Fix a rare crash in CompressingLogFileManager caused by an unchecked result from read. If the read fails, it returns -1. This was then added to inputDataSize, which in turn is added to inputRemaining further down the function, and used as an input to memmove (and is unsigned to boot, so it'll be very large rather than negative). This means that the memmove stomps all over memory, and we crash not long afterwards (often when an autorelease pool is freed). This crash is rare because it depends on the read call failing. Fix this by checking the return value from NSStream.read, and leaving the loop immediately if there's an error.
|
The first cset here is PR #250, so this PR is dependent on that one. (I'm sure GitHub used to be able to handle dependencies between PRs, but I can't find that option now). |
| if (readLength < 0) { | ||
| error = [inputStream streamError]; | ||
| break; | ||
| } |
There was a problem hiding this comment.
So the addition to the mentioned pull request is this right?
There was a problem hiding this comment.
Yes, exactly. Commit 44da95a.
bpoplauschi
added a commit
that referenced
this pull request
Apr 1, 2014
…ead-return Fix a rare crash caused by an unchecked return value.
|
Thanks @ewanmellor |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix a rare crash in CompressingLogFileManager caused by an
unchecked result from read. If the read fails, it returns -1.
This was then added to inputDataSize, which in turn is added to
inputRemaining further down the function, and used as an input
to memmove (and is unsigned to boot, so it'll be very large rather than
negative). This means that the memmove stomps all over memory,
and we crash not long afterwards (often when an autorelease pool is
freed).
This crash is rare because it depends on the read call failing.
Fix this by checking the return value from NSStream.read, and leaving
the loop immediately if there's an error.