New issue
Advanced search Search tips

Issue 921067 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 16
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Crash in LocalStorageContextMojo::PerformStorageCleanup

Project Member Reported by sky@chromium.org, Jan 11

Issue description

This crash is happening in the ChromeOS lab.

Here's the interesting part of the crash:

Operating system: Linux
                  0.0.0 Linux 4.14.91-09542-g14e8cadc0b82 #1 SMP PREEMPT Thu Jan 10 11:40:51 PST 2019 x86_64
CPU: amd64
     family 6 model 122 stepping 1
     1 CPU

GPU: UNKNOWN

Crash reason:  SIGSEGV
Crash address: 0x0
Process uptime: not available

Thread 0 (crashed)
 0  chrome!content::LocalStorageContextMojo::PerformCleanup(base::OnceCallback<void ()>) [local_storage_context_mojo.cc : 438 + 0x0]
    rax = 0x3626a2a0293a7600   rdx = 0x000021f6e773cd00
    rcx = 0x000059470c6936f0   rbx = 0x0000000000000000
    rsi = 0x00007df16aa50a30   rdi = 0x0000000000000000
    rbp = 0x00007df16aa50a20   rsp = 0x00007df16aa509e0
     r8 = 0x00007df16aa50c18    r9 = 0x0000000000000001
    r10 = 0x0039440701c93e9c   r11 = 0x0000000000000000
    r12 = 0x0000000000000000   r13 = 0x000059470ab3e194
    r14 = 0x00007df16aa50a30   r15 = 0x000021f6e5cd6480
    rip = 0x000059470c693712
    Found by: given as instruction pointer in context
 1  chrome!base::internal::Invoker<base::internal::BindState<void (arc::(anonymous namespace)::ArcTracingDataSource::*)(base::OnceCallback<void ()>), base::internal::UnretainedWrapper<arc::(anonymous namespace)::ArcTracingDataSource>, base::OnceCallback<void ()> >, void ()>::RunOnce(base::internal::BindStateBase*) [bind_internal.h : 516 + 0x5]
    rbx = 0x000021f6e6e03900   rbp = 0x00007df16aa50a40
    rsp = 0x00007df16aa50a30   r12 = 0x0000000000000000
    r13 = 0x000059470ab3e194   r14 = 0x000021f6e5f62c48
    r15 = 0x000021f6e5cd6480   rip = 0x000059470c085937
    Found by: call frame info

Ignore the names in Frame 1 as they are inlined. I *think* |this| is null, although I'm not positive there. It's also possible |this| has been deleted. I'm going with null as the crash address is 0x0 (although you would think it's offset by the member, so not positive). My guess is DOMStorageContextWrapper::Shutdown() has been called, which nulls out mojo_state_, and *then* DOMStorageContextWrapper::PerformLocalStorageCleanup() was called, which ends up binding a null value. It looks like PerformLocalStorageCleanup() is itself handled by a posted task, which implies there is no guarantee mojo_state_ is null. I'm not familiar with this code, so I could certainly be wrong.

I'll send around a speculative fix.

Full report is here: https://00e9e64bac1d135257ef991c158f6aff012b96bc0fe928a45e-apidata.googleusercontent.com/download/storage/v1/b/chromeos-autotest-results/o/275736559-chromeos-test%2Fchromeos6-row8-rack8-host12%2Fdebug%2Fchrome.20190110.145844.13171.dmp.txt?qk=AD5uMEuAFx4Q7T59IRb6C13_V4s--rPXbF-z3O4uB9qCkc9dOWUZAqTyVgdqP2yhvyqfMPFMnl6f6VFU6BVNJWDVNXRBaziNQ_6Qn68YcpM7UZ-cNIdjBxkJBCWP9DEnQxKfCZFOufVvaXQmKIzkGd3bif903OomEpUZG5rTqbVhjw86KUmn9pZQ8GMafMcqA1Wsu-u5Dlrapo89RA6pTgjZC7Gix-02ppviqirAuGiPXXn47poZ01691tNuZV_apDNz2VSg1bOUT7NXR_7um3fZQHW8bfiiLTEOBgFWYJJLjldqzjUa9L6nHlE1k5D6U__CF7v61UbHVmQDpVIHclO9RXkjFnL77bIe0QU-ARcxE5zGd479xolL5TBI9lxftIVU77XJ7qqZFGOvmd6zI5Q6rB-8xunKTH_4pIaLSAcICL2tDHpp9hyklbYof9jj5quEmqDfUThJc7QPlGnX33eMLyzKQIQOyfkk-kuA7_ztSbo-b-45ubDW4YdP14t2ac356nqfw5FoMcL3IOp6Ev6CCWoYzMkuYEkyV7vfFNUOTcMYwJUl8jT--LB5oAh4ZEjoRqMesUpCw1uyAPJGFDvd8qMa94ng3iPcJPm8m8SF7TE14pTy8PqnKqjm6XzPqWlFInrJ_qP1JRdTojY7pYUD4J1_mt_te_IK3olE-yRtUZ54-Ulne_-3fkNi6ZTq5R_XVjGlhfn2pdty-g6sKx1Y95BDDcwb25kzjU0uuNfkKAd4E8e-gYIaJpb8SDY30niPC9GYv4d8NLoJ6rpmINO_JMhtStcSO3aMhNci-sLd5H1ZOF99O619c3vwuTQNG0SCOHLQpEwtPDsxqqXdBZaN1wprN6W-kCR6Oi4cgr1AjigxZnOONfsTcMJephInRaIPfUdTEsJc 
 
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 16

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

commit c2d122c14bfe42fccd2c4c33264ceb9c66c3efba
Author: Scott Violet <sky@chromium.org>
Date: Wed Jan 16 00:57:10 2019

dom_storage: speculative crash fix

A crash seems to indicate LocalStorageContextMojo::PerformStorageCleanup()
is being called with |this| set to null. It seems the only way that could
happen is if DOMStorageContextWrapper::PerformLocalStorageCleanup() is called
after Shutdown() (Shutdown() sets mojo_state_ to null).

This is a speculative fix, and I could certainly be wrong. Please suggest
alternatives if you think I'm reading it wrong or what I've outlined isn't
possible.

BUG= 921067 
TEST=none

Change-Id: I0eb7482bf37369e4b61b05d34526c679fa71d416
Reviewed-on: https://chromium-review.googlesource.com/c/1407117
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622952}
[modify] https://crrev.com/c2d122c14bfe42fccd2c4c33264ceb9c66c3efba/content/browser/dom_storage/dom_storage_context_wrapper.cc

Comment 3 by sky@chromium.org, Jan 16 (6 days ago)

Owner: sky@chromium.org
Status: Fixed (was: Untriaged)

Sign in to add a comment