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

Issue 691842 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Mar 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Shutdown crash at arc::ArcImeService::~ArcImeService()

Project Member Reported by kinaba@chromium.org, Feb 14 2017

Issue description

Chrome Version:58.0.3007.0
OS: ChromeOS

https://crash.corp.google.com/browse?q=reportid=%27a9b27b9280000000%27

0x0000565ac9335838	(chrome -window_observer.cc:16 )	aura::WindowObserver::~WindowObserver()
0x0000565aca15f9e4	(chrome -arc_ime_service.cc:72 )	arc::ArcImeService::~ArcImeService()
0x0000565aca15fa90	(chrome -arc_ime_service.cc:88 )	arc::ArcImeService::~ArcImeService()
0x0000565ac81d3a51	(chrome -unique_ptr.h:76 )	std::_Hashtable<std::string, std::pair<std::string const, std::unique_ptr<arc::ArcService, std::default_delete<arc::ArcService> > >, std::allocator<std::pair<std::string const, std::unique_ptr<arc::ArcService, std::default_delete<arc::ArcService> > > >, std::__detail::_Select1st, std::equal_to<std::string>, std::hash<std::string>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, false> >::clear()
0x0000565ac7d85cf3	(chrome -arc_service_launcher.cc:166 )	arc::ArcServiceLauncher::Shutdown()


yoshiki@, the WindowObserver inheritance introduced in https://codereview.chromium.org/2661863002/ looks causing another trouble.
 
It should be same issue as b/35198345

Comment 2 by xiy...@chromium.org, Feb 21 2017

Issue 694329 has been merged into this issue.

Comment 3 by xiy...@chromium.org, Feb 21 2017

Cc: abodenha@chromium.org
We should check |focused_arc_window_| and call focused_arc_window_->RemoveObserver() in dtor of ArcImeService. The crash happens on shutdown. IIUC, ArcImeService shuts down as part of ArcServiceLauncher and happens before we close ash. Hence the CHECK crash.
Status: Started (was: Assigned)
Thanks xiyuan, I'll add removeObserver in the dtor.
Style guides[1] say we should use DCHECK instead of CHECK for most cases, so once all the crashes are dealt with, the CHECK should be changed to DCHECK.

1: https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/c++.md#CHECK_DCHECK_and-NOTREACHED
Project Member

Comment 6 by bugdroid1@chromium.org, Feb 23 2017

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

commit a6e727ae6c9a9a9994a87ebb2e1025e9982e15c7
Author: yoshiki <yoshiki@chromium.org>
Date: Thu Feb 23 06:16:34 2017

Remove ArcImeService from window observers on shutdown

Remaining observer fails CHECK and causes crash. This patch prevents it.

BUG= 691842 
TEST=none

Review-Url: https://codereview.chromium.org/2709113002
Cr-Commit-Position: refs/heads/master@{#452417}

[modify] https://crrev.com/a6e727ae6c9a9a9994a87ebb2e1025e9982e15c7/components/arc/ime/arc_ime_service.cc

Status: Fixed (was: Started)
Status: Verified (was: Fixed)

Sign in to add a comment