New issue
Advanced search Search tips

Issue 606065 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocked on:
issue 607698



Sign in to add a comment

Remove blob bad_message crashes for known issues

Project Member Reported by dmu...@chromium.org, Apr 22 2016

Issue description

We have a lot of crashes since adding bad_message errors for blob messages. It looks like there is a pre-existing condition where blob references are race casing across processes.

Currently the situation is now worse, since we crash, instead of just having a 'broken' blob. I'd like to remove the crashes for this case, but leave UMA stats so I can monitor it while I fix it.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f8e7a2ad99e72ed370e17d7258f64dc7acc49fe

commit 3f8e7a2ad99e72ed370e17d7258f64dc7acc49fe
Author: dmurph <dmurph@chromium.org>
Date: Tue Apr 26 20:13:21 2016

[Blob] Removed crash on race case blob messages

We have substantial crashes since adding bad_message errors for invalid
blob messages. It looks like there is a pre-existing condition where
blob references are race casing across processes.

While we investigate the issue, we want to remove this crash so it's not
making the situation worse (crashing instead of having a broken blob).

I'm keeping the uuid.empty() condition as a bad message, because that's
definitely an error that shouldn't be happening.

BUG= 606065 

Review URL: https://codereview.chromium.org/1913343002

Cr-Commit-Position: refs/heads/master@{#389868}

[modify] https://crrev.com/3f8e7a2ad99e72ed370e17d7258f64dc7acc49fe/content/browser/blob_storage/blob_dispatcher_host.cc

Comment 2 by dmu...@chromium.org, Apr 28 2016

Labels: Merge-Request-51
We have this running successfully in Canary since 52.0.2718.0

Comment 3 by tin...@google.com, Apr 28 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)

Comment 4 by gov...@chromium.org, Apr 29 2016

Please merge your change to M51 branch 2704 before 5:00 PM PST, tomorrow (Friday), so we can take it in for next week M51 beta release. Thank you.

Comment 5 by dmu...@chromium.org, Apr 29 2016

Blockedon: 607698

Comment 6 by dmu...@chromium.org, Apr 29 2016

Cc: dmu...@chromium.org
 Issue 608050  has been merged into this issue.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 29 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff946f842cd1239d4cf7ae26bb6206c24c6e5ca1

commit ff946f842cd1239d4cf7ae26bb6206c24c6e5ca1
Author: Daniel Murphy <dmurph@chromium.org>
Date: Fri Apr 29 22:30:44 2016

[Blob] Removed crash on race case blob messages

We have substantial crashes since adding bad_message errors for invalid
blob messages. It looks like there is a pre-existing condition where
blob references are race casing across processes.

While we investigate the issue, we want to remove this crash so it's not
making the situation worse (crashing instead of having a broken blob).

I'm keeping the uuid.empty() condition as a bad message, because that's
definitely an error that shouldn't be happening.

BUG= 606065 

Review URL: https://codereview.chromium.org/1913343002

Cr-Commit-Position: refs/heads/master@{#389868}
(cherry picked from commit 3f8e7a2ad99e72ed370e17d7258f64dc7acc49fe)

Review URL: https://codereview.chromium.org/1934773002 .

Cr-Commit-Position: refs/branch-heads/2704@{#320}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/ff946f842cd1239d4cf7ae26bb6206c24c6e5ca1/content/browser/blob_storage/blob_dispatcher_host.cc

Comment 8 by dmu...@chromium.org, Apr 29 2016

Status: Fixed (was: Assigned)

Comment 9 by nick@chromium.org, Apr 29 2016

Status: Verified (was: Fixed)
Thanks for fixing and merging this. I can verify that BDH_ kills are completely gone from the Stability.BadMessageTerminated.Content uma histogram from canary versions 52.0.2718.0 onwards.
Components: Blink>Storage>FileAPI
Components: -Blink>FileAPI

Sign in to add a comment