Skip to content

Fix crash in BraveStatsUpdater when computer wakes from suspend #552

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

Merged
merged 1 commit into from
Oct 2, 2018
Merged

Fix crash in BraveStatsUpdater when computer wakes from suspend #552

merged 1 commit into from
Oct 2, 2018

Conversation

pilgrim-brave
Copy link
Contributor

@pilgrim-brave pilgrim-brave commented Oct 2, 2018

Fix brave/brave-browser#1116

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

@bbondy bbondy merged commit 41f4901 into brave:master Oct 2, 2018
bbondy added a commit that referenced this pull request Oct 2, 2018
Fix crash in BraveStatsUpdater when computer wakes from suspend
bbondy added a commit that referenced this pull request Oct 2, 2018
Fix crash in BraveStatsUpdater when computer wakes from suspend
@bbondy
Copy link
Member

bbondy commented Oct 2, 2018

master: 41f4901
0.56.x: 4dc92c3
0.55.x: 59cf3ba

if (simple_url_loader_->NetError() != net::OK || response_code < 200 ||
response_code > 299) {
LOG(ERROR) << "Failed to send usage stats to update server"
<< ", error: " << simple_url_loader_->NetError()
<< ", response code: " << response_code
<< ", payload: " << *response_body
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pilgrim-brave I guess this line was the root cause of the crash?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the crash was earlier. If SimpleURLLoader received a network error with a large response body, it attempted to copy the entire body into a fixed length buffer to call the OnSimpleLoaderComplete callback function. With a large enough body, this crashed. Fix uses DownloadHeadersOnly to ignore the response body altogether.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I see, thanks for catching that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when waking up from sleep
3 participants