New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 730321 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

SEGV Crash at ash::ShelfID::operator< when closing an ARC app window.

Project Member Reported by kinaba@chromium.org, Jun 7 2017

Issue description

Chrome Version: 61.0.3119.0
OS: Chrome OS 9618.0.0 (Samus Canary; ARC-N)

What steps will reproduce the problem?
(1) Launch the Play Store app.
(2) Close by the X button.

What is the expected result?
The window closes

What happens instead?
http://crash/019bcb2e40000000

Thread 0 (id: 5314) CRASHED [SIGSEGV @ 0xfffffffd3f0f9000 ]

	0x0000581e127c9894	(chrome + 0x052d6894 )	ash::ShelfID::operator<(ash::ShelfID const&) const
0x0000581e127c832b	(chrome + 0x052d532b )	ash::ShelfModel::SetShelfItemDelegate(ash::ShelfID const&, std::unique_ptr<ash::ShelfItemDelegate, std::default_delete<ash::ShelfItemDelegate> >)
0x0000581e12936064	(chrome + 0x05443064 )	ChromeLauncherController::CloseLauncherItem(ash::ShelfID const&)
0x0000581e12a1ab0a	(chrome + 0x05527b0a )	ArcAppWindowLauncherController::OnTaskDestroyed(int)
0x0000581e12a0b773	(chrome + 0x05518773 )	ArcAppListPrefs::OnTaskDestroyed(int)
0x0000581e0f6952f3	(chrome + 0x021a22f3 )	arc::mojom::AppHostStubDispatch::Accept(arc::mojom::AppHost*, mojo::Message*)


+ARC Constables, +Yury, could you mind taking a look?

I'm seeing this crash both on my recent-ish local builds and canary dogfooding device.
 
I could have reproduced this issue. To reproduce this issue, you need to pin Play store icon in the shelf.

Comment 2 by khmel@chromium.org, Jun 7 2017

Owner: khmel@chromium.org
Status: Started (was: Untriaged)
Thanks for reporting,  I will take a look..

Comment 3 by khmel@chromium.org, Jun 7 2017

Cc: yawano@chromium.org

Comment 4 by khmel@chromium.org, Jun 7 2017

Cc: msw@chromium.org jamescook@chromium.org
Owner: msw@chromium.org
Bisection showed: 19b30c2c383d
https://codereview.chromium.org/2833173002

Mike, could you please take a look (this potentially P0).

What I saw, it looks like mem corruption.
On my side 

void ShelfController::ShelfItemDelegateChanged(const ShelfID& id,
                                               ShelfItemDelegate* delegate) {

supposed to be called but never enter this func.

Comment 5 by khmel@chromium.org, Jun 7 2017

Labels: M-61

Comment 6 by msw@chromium.org, Jun 7 2017

I will look ASAP today, is there a way to repro on a linux dev box with chromeos=1?
I'll be out tomorrow through June 27, so hopefully fixing this will be easy!

Comment 7 by msw@chromium.org, Jun 7 2017

Actually, khmel@ if you can repro, can you try my WIP fix for other issues in that same CL? I suspect it may help here too: https://codereview.chromium.org/2927693002/

Comment 8 by khmel@chromium.org, Jun 7 2017

Let me try out #7

Comment 9 by khmel@chromium.org, Jun 7 2017

#7 did not help :(

>> I will look ASAP today, is there a way to repro on a linux dev box with chromeos=1?
I can try to create some emulation layer but this could take a time.

Comment 10 by msw@chromium.org, Jun 7 2017

I think #7 might help if I revise the arc controllers similar to the extension controller.
I'll ping you when I have a new patch set; it would be great if you can help with testing.
I can try to repro on device, but that will also take time (I'm still not good at this...)

Comment 11 by msw@chromium.org, Jun 7 2017

Owner: khmel@chromium.org
Yury has a fix at https://chromium-review.googlesource.com/c/527415/
Thanks for your help investigating/fixing/testing!!!
Sorry about the inconvenience :-/
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 7 2017

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

commit 6b3ba8995cb10ed30c7db3333db1be88307cc6c5
Author: khmel <khmel@google.com>
Date: Wed Jun 07 19:53:42 2017

Fix crash in shelf on ARC App window created.

This prevents a memory corruption when ash::ShelfId can be used after
its owner is destroyed.

TBR=jamescook@chromium.org

Bug:  730321 
Test: Manually on device
Change-Id: I8fb00a38d75075aa1c69d8fe95f365f891bf1eeb
Reviewed-on: https://chromium-review.googlesource.com/527415
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Yury Khmel <khmel@google.com>
Cr-Commit-Position: refs/heads/master@{#477740}
[modify] https://crrev.com/6b3ba8995cb10ed30c7db3333db1be88307cc6c5/ash/public/cpp/shelf_model.cc

Status: Fixed (was: Started)

Comment 14 by khmel@chromium.org, Jun 19 2017

Issue 734775 has been merged into this issue.
Status: Verified (was: Fixed)
Verified on build 9765.53.0

Sign in to add a comment