New issue
Advanced search Search tips

Issue 856629 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

DCHECK(!IsBound()) hit in SessionStorageAreaImpl::Bind

Project Member Reported by vabr@chromium.org, Jun 26 2018

Issue description

Chrome Version: 69.0.3474.0, developer build with DCHECKS enabled
OS: GNU/Linux

What steps will reproduce the problem?
(1) Open Chrome, visit https://pro.upstox.com in the (only) tab.
(2) Click "Open an account" (top-right corner).
(3) Let the second tab load, then close it, then hit Ctrl+Shift

This is the stacktrace I see:
[210179:210186:0626/160424.316251:FATAL:session_storage_area_impl.cc(33)] Check failed: !IsBound(). 
#0 0x7f34402befec base::debug::StackTrace::StackTrace()
#1 0x7f34402081cb logging::LogMessage::~LogMessage()
#2 0x7f343d7c0c1c content::SessionStorageAreaImpl::Bind()
#3 0x7f343d7d5644 content::SessionStorageNamespaceImplMojo::OpenArea()
#4 0x7f343bd213af blink::mojom::SessionStorageNamespaceStubDispatch::Accept()
#5 0x7f343f46c212 mojo::InterfaceEndpointClient::HandleValidatedMessage()
#6 0x7f343f46baf6 mojo::FilterChain::Accept()
#7 0x7f343f46d722 mojo::InterfaceEndpointClient::HandleIncomingMessage()
#8 0x7f343f47445d mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#9 0x7f343f473820 mojo::internal::MultiplexRouter::Accept()
#10 0x7f343f46baf6 mojo::FilterChain::Accept()
#11 0x7f343f46648b mojo::Connector::ReadSingleMessage()
#12 0x7f343f467054 mojo::Connector::ReadAllAvailableMessages()
#13 0x7f343f466eb6 mojo::Connector::OnHandleReadyInternal()
#14 0x7f343f467864 mojo::SimpleWatcher::DiscardReadyState()
#15 0x7f343f427773 mojo::SimpleWatcher::OnHandleReady()
#16 0x7f343f427cee _ZN4base8internal7InvokerINS0_9BindStateIMN4mojo13SimpleWatcherEFvijRKNS3_18HandleSignalsStateEEJNS_7WeakPtrIS4_EEijS5_EEEFvvEE7RunImplIRKS9_RKNSt3__15tupleIJSB_ijS5_EEEJLm0ELm1ELm2ELm3EEEEvOT_OT0_NSI_16integer_sequenceImJXspT1_EEEE
#17 0x7f34401e8dfd base::debug::TaskAnnotator::RunTask()
#18 0x7f3440214776 base::internal::IncomingTaskQueue::RunTask()
#19 0x7f3440218747 base::MessageLoop::RunTask()
#20 0x7f3440218b5a base::MessageLoop::DeferOrRunPendingTask()
#21 0x7f3440218dee base::MessageLoop::DoWork()
#22 0x7f34402e1b89 base::MessagePumpLibevent::Run()
#23 0x7f3440218071 base::MessageLoop::Run()
#24 0x7f344024c346 base::RunLoop::Run()
#25 0x7f3440289a2a base::Thread::Run()
#26 0x7f343d6c7db4 content::BrowserProcessSubThread::IOThreadRun()
#27 0x7f343d6c7cff content::BrowserProcessSubThread::Run()
#28 0x7f3440289faf base::Thread::ThreadMain()
#29 0x7f34402d418f base::(anonymous namespace)::ThreadFunc()
#30 0x7f3434d05494 start_thread
#31 0x7f3432c58a8f clone


Assigning to dmurph@, who added the DCHECK in https://chromium-review.googlesource.com/954410.
 

Comment 1 by dmu...@chromium.org, Jun 26 2018

A repro case! Amazing! Thank you :)

Comment 2 by dmu...@chromium.org, Jun 26 2018

Issue 856644 has been merged into this issue.

Comment 3 by dmu...@chromium.org, Jun 26 2018

So it turns out we do try to bind so the same area (namespace+origin) twice somewhere. I wonder how this happens... It definitely shouldn't happen. I should maybe bad_message instead of crashing.

Crash link from other bug:
Crash link: https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27%5BAssert%5D+content%3A%3ASessionStorageAreaImpl%3A%3ABind%27+AND+product.Version%3D%2769.0.3473.1%27&stbtiq=&reportid=&index=0

Comment 4 by dmu...@chromium.org, Jun 26 2018

Labels: -Pri-3 Pri-1

Comment 5 by w...@chromium.org, Jun 26 2018

Components: Blink>Storage>DOMStorage
Labels: M-69

Comment 6 by mek@chromium.org, Jun 26 2018

Random thought: maybe the fact that a renderer process doesn't close storage areas as soon as all use of it is gone can sometimes result in multiple processes seemingly trying to connect to the same area at a time, even though only the last one actually is trying to use it? Or just a race condition between closing and re-opening since those are on different pipe anyway. Hopefully it's not anything more serious that invalidates our assumptions that there only is one actual process using an area at a time...

Comment 7 by dmu...@chromium.org, Jun 26 2018

Sadly, I cannot repro locally.  Did you mean Ctrl+Shift+T?

Comment 8 by mek@chromium.org, Jun 26 2018

It does reproduce with those steps (includign Ctrl+Shift+T) for me.

Comment 9 by mek@chromium.org, Jun 26 2018

And yeah, it is similar to what I was thinking. The initial window opened by clicking the link opens in the same process as the main page (I guess site isolation isn't isolating these origins for me), however re-opening the tab after closing opens it in its own renderer process.
Nevermind, reproducing locally now.
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 27 2018

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

commit 2fcc6703e42f619b22c0ceb716920d44a22a68d0
Author: Daniel Murphy <dmurph@chromium.org>
Date: Wed Jun 27 01:18:09 2018

[SessionStorageS13N] Allowing multiple binds on a storage area

Allow multiple binds on a storage area where the previous binding is
unbound. This is needed because of the following case:

1. Page A (process X) opens a session storage area.
2. Page A opens a new tab to Page B (still process X) which
   opens a session storage area.
3. The user closes Page B. The storage binding still exists in
   process X.
4. The user presses Ctrl+Shift+T, which re-opens Page B but in a
   new process (process Y).
5. Process Y now opens session storage at the same namspace & origin as
   step 2, which was already bound to process X.

R=mek@chromium.org

Bug:  856629 
Change-Id: I6fd833ce1502ec3fb58443f92ac48cfb1b282f99
Reviewed-on: https://chromium-review.googlesource.com/1115867
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570610}
[modify] https://crrev.com/2fcc6703e42f619b22c0ceb716920d44a22a68d0/content/browser/dom_storage/session_storage_area_impl.cc
[modify] https://crrev.com/2fcc6703e42f619b22c0ceb716920d44a22a68d0/content/browser/dom_storage/session_storage_area_impl_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment