Chrome_Mac: Crash Report - views::Widget::OnNativeWidgetDestroying |
||||||||||||||||||||||
Issue descriptionreporter:tapted@google.com Magic Signature: views::Widget::OnNativeWidgetDestroying Updated Link: https://crash.corp.google.com/browse?q=product_name%3D%27Chrome_Mac%27+AND+EXISTS+%28SELECT+1+FROM+UNNEST%28CrashedStackTrace.StackFrame%29+WHERE+FunctionName%3D%27views%3A%3AWidget%3A%3AOnNativeWidgetDestroying%28%29%27%29+AND+expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27views%3A%3AWidget%3A%3AOnNativeWidgetDestroying%27&stbtiq=&reportid=&index=0 Crash link: https://crash.corp.google.com//browse?q=product.name%3D'Chrome_Mac'%20AND%20EXISTS%20(SELECT%201%20FROM%20UNNEST(CrashedStackTrace.StackFrame)%20WHERE%20FunctionName%3D'views%3A%3AWidget%3A%3AOnNativeWidgetDestroying()')%20AND%20ReportID%3D'fb0f46a358d74a8e'%20AND%20expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D'views%3A%3AWidget%3A%3AOnNativeWidgetDestroying'&sql_dialect=googlesql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#3 ------------------------------------------------------------------------------- Sample Report ------------------------------------------------------------------------------- Product name: Chrome_Mac Magic Signature : views::Widget::OnNativeWidgetDestroying Product Version: 66.0.3336.5 Process type: browser Report ID: fb0f46a358d74a8e Report Url: https://crash.corp.google.com/fb0f46a358d74a8e Report Time: 2018-02-01T17:48:38-08:00 Upload Time: 2018-02-01T17:48:41.593-08:00 Uptime: 165000 ms CumulativeProductUptime: 0 ms OS Name: Mac OS X OS Version: 10.12.6 16G29 CPU Architecture: amd64 CPU Info: family 6 model 61 stepping 4 ------------------------------------------------------------------------------- Crashing thread: Thread index: 0. Stack Quality: 84%. Thread id: 506217. ------------------------------------------------------------------------------- 0x000000010d4f059e (Google Chrome Framework - widget.cc: 1086) views::Widget::OnNativeWidgetDestroying() 0x000000010d48692d (Google Chrome Framework - bridged_native_widget.mm: 786) views::BridgedNativeWidget::OnWindowWillClose() 0x000000010d48b12a (Google Chrome Framework - views_nswindow_delegate.mm: 126) -[ViewsNSWindowDelegate windowWillClose:] 0x00007fff8881d45b (CoreFoundation + 0x0009d45b) __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ 0x00007fff8881d35a (CoreFoundation + 0x0009d35a) _CFXRegistrationPost 0x00007fff8881d0c1 (CoreFoundation + 0x0009d0c1) ___CFXNotificationPost_block_invoke 0x00007fff887da522 (CoreFoundation + 0x0005a522) -[_CFXNotificationRegistrar find:object:observer:enumerator:] 0x00007fff887d955b (CoreFoundation + 0x0005955b) _CFXNotificationPost 0x00007fff8a1fe676 (Foundation + 0x00006676) -[NSNotificationCenter postNotificationName:object:userInfo:] 0x00007fff86597da3 (AppKit + 0x002ddda3) __18-[NSWindow _close]_block_invoke 0x00007fff86597c87 (AppKit + 0x002ddc87) -[NSWindow _close] 0x000000010d4ea514 (Google Chrome Framework - native_widget_mac.mm: 645) views::Widget::CloseAllSecondaryWidgets() 0x000000010e217860 (Google Chrome Framework - browser_list.cc: 111) BrowserList::RemoveBrowser(Browser*) 0x000000010e2094f4 (Google Chrome Framework - browser.cc: 497) Browser::~Browser() 0x000000010e209c2d (Google Chrome Framework - browser.cc: 481) <name omitted> 0x00007fff9d69cbb3 (libobjc.A.dylib + 0x00010bb3) object_cxxDestructFromClass(objc_object*, objc_class*) 0x00007fff9d6955f5 (libobjc.A.dylib + 0x000095f5) objc_destructInstance 0x000000010d2070e8 (Google Chrome Framework - objc_zombie.mm: 110) (anonymous namespace)::ZombieDealloc(objc_object*, objc_selector*) 0x00007fff862f465a (AppKit + 0x0003a65a) -[NSResponder dealloc] 0x000000010e42758c (Google Chrome Framework - tab_window_controller.mm: 262) -[TabWindowController dealloc] 0x000000010e35eb8f (Google Chrome Framework - browser_window_controller.mm: 483) -[BrowserWindowController dealloc] 0x00007fff8a261db3 (Foundation + 0x00069db3) __delayedPerformCleanup 0x00007fff88810fe6 (CoreFoundation + 0x00090fe6) CFRunLoopTimerInvalidate 0x00007fff88810915 (CoreFoundation + 0x00090915) __CFRunLoopDoTimer 0x00007fff88810439 (CoreFoundation + 0x00090439) __CFRunLoopDoTimers 0x00007fff88807b80 (CoreFoundation + 0x00087b80) __CFRunLoopRun 0x00007fff88807113 (CoreFoundation + 0x00087113) CFRunLoopRunSpecific 0x00007fff87d67ebb (HIToolbox + 0x00030ebb) RunCurrentEventLoopInMode 0x00007fff87d67bf8 (HIToolbox + 0x00030bf8) ReceiveNextEventCommon 0x00007fff87d67b25 (HIToolbox + 0x00030b25) _BlockUntilNextEventMatchingListInModeWithFilter 0x00007fff86300a53 (AppKit + 0x00046a53) _DPSNextEvent 0x00007fff86a7c7ed (AppKit + 0x007c27ed) -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] 0x000000010b79532f (Google Chrome Framework - chrome_browser_application_mac.mm: 174) __71-[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]_block_invoke 0x000000010bb518d9 (Google Chrome Framework + 0x01e4e8d9) base::mac::CallWithEHFrame(void () block_pointer) 0x000000010b795273 (Google Chrome Framework - chrome_browser_application_mac.mm: 173) -[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:] 0x00007fff862f53da (AppKit + 0x0003b3da) -[NSApplication run] 0x000000010bb606cb (Google Chrome Framework - message_pump_mac.mm: 815) base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) 0x000000010bb5f24d (Google Chrome Framework - message_pump_mac.mm: 189) base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) 0x000000010bb83534 (Google Chrome Framework - run_loop.cc: 133) <name omitted> 0x000000010b79b1d7 (Google Chrome Framework - chrome_browser_main.cc: 1997) ChromeBrowserMainParts::MainMessageLoopRun(int*) 0x000000010a510ca3 (Google Chrome Framework - browser_main_loop.cc: 1257) content::BrowserMainLoop::RunMainMessageLoopParts() 0x000000010a513671 (Google Chrome Framework - browser_main_runner.cc: 145) content::BrowserMainRunnerImpl::Run() 0x000000010a50d12b (Google Chrome Framework - browser_main.cc: 46) content::BrowserMain(content::MainFunctionParams const&) 0x000000010b74d3df (Google Chrome Framework - content_main_runner.cc: 713) content::ContentMainRunnerImpl::Run() 0x000000010d0d523a (Google Chrome Framework - main.cc: 456) service_manager::Main(service_manager::MainParams const&) 0x000000010b74c923 (Google Chrome Framework - content_main.cc: 19) content::ContentMain(content::ContentMainParams const&) 0x0000000109d07369 (Google Chrome Framework - chrome_main.cc: 144) ChromeMain 0x0000000107494dd3 (Google Chrome Canary + 0x00000dd3) 0x00007fff9df87234 (libdyld.dylib + 0x00005234) start 0x00007fff9df87234 (libdyld.dylib + 0x00005234) start
,
Feb 2 2018
,
Feb 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/64a3626633f586dfd6241fe62502aa182457ffeb commit 64a3626633f586dfd6241fe62502aa182457ffeb Author: Trent Apted <tapted@chromium.org> Date: Sat Feb 03 02:56:28 2018 Add crash keys inside Mac's Widget::CloseAllSecondaryWidgets() crbug/788271 is a mystery, crashing in objc_msgSend without triggering any zombie warnings. crbug/808318 points to a leaked WidgetObserver affecting all platforms, but has no indication of what the observer is, or what window it observes. Record class names and the window title in crash keys. The window title is usually set by the framework, even if it is not displayed anywhere. Bug: 788271, 808318 Change-Id: I09f9697bfca4e91c8f3b0e8dba125c1eb39cde4b Reviewed-on: https://chromium-review.googlesource.com/896774 Commit-Queue: Trent Apted <tapted@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Cr-Commit-Position: refs/heads/master@{#534240} [modify] https://crrev.com/64a3626633f586dfd6241fe62502aa182457ffeb/ui/views/BUILD.gn [modify] https://crrev.com/64a3626633f586dfd6241fe62502aa182457ffeb/ui/views/DEPS [modify] https://crrev.com/64a3626633f586dfd6241fe62502aa182457ffeb/ui/views/widget/native_widget_mac.mm [modify] https://crrev.com/64a3626633f586dfd6241fe62502aa182457ffeb/ui/views/widget/native_widget_mac_unittest.mm
,
Feb 22 2018
Query to track: https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_Mac%27%20AND%20EXISTS%20(SELECT%201%20FROM%20UNNEST(productdata)%20WHERE%20key%3D%27windowInfo%27)&stbtiq=&reportid=&index=0 This is a low crash rate that affects all platforms.
,
Feb 22 2018
Note that link has 2 reports, both of which are Issue 788271, now fixed. New ones could be this issue.
,
Feb 26 2018
Users experienced this crash on the following builds: Mac Canary 66.0.3355.0 - 0.15 CPM, 1 reports, 1 clients (signature views::Widget::OnNativeWidgetDestroying) If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates. - Go/Fracas
,
Mar 4 2018
http://go/crash/2c6f04047f83d14a The offending dialog title is "Your Internet connection is being controlled" IDS_EXTENSIONS_PROXY_CONTROLLED_TITLE_HOME_PAGE_BUBBLE I think that creates an instance of ToolbarActionsBarBubbleViews. That's just a BubbleDialogDelegateView, but it also has BrowserActionsContainer observing it. AddObserver added in BrowserActionsContainer::ShowToolbarActionBubble() here: https://cs.chromium.org/chromium/src/chrome/browser/ui/views/toolbar/browser_actions_container.cc?sq=package:chromium&dr=CSs&l=273 That observer is only removed in a call to BrowserActionsContainer::ClearActiveBubble(views::Widget* widget) { DCHECK(active_bubble_); DCHECK_EQ(active_bubble_->GetWidget(), widget); widget->RemoveObserver(this); active_bubble_ = nullptr; toolbar_actions_bar_->OnBubbleClosed(); } and the BrowserActionsContainer destructor only starts an *asynchronous* close. So it's quite possible for this Widget to outlive a BrowserActionsContainer, causing this use-after-free. BrowserActionsContainer::~BrowserActionsContainer() { if (active_bubble_) active_bubble_->GetWidget()->Close(); I think either that Close() should be a CloseNow(), or ~BrowserActionsContainer() should do the RemoveObserver call early.
,
Mar 4 2018
hm - there's more to this. This should clear it in the normal case:
void BrowserActionsContainer::OnWidgetClosing(views::Widget* widget) {
ClearActiveBubble(widget);
}
but codepaths under Widget::CloseAllSecondaryWidgets() do not invoke OnWidgetClosing (on any platform).
they *do* invoke
void BrowserActionsContainer::OnWidgetDestroying(views::Widget* widget) {
ClearActiveBubble(widget);
}
but that's where this crash is happening -- for some reason (I suspect) BrowserActionsContainer is still observing at this point, and yet it has been destroyed, causing a crash when this gets invoked.
(there is another observer -- the BubbleDialogDelegateView itself -- so it's possible that's involved somehow..)
,
Mar 6 2018
If the crash happens from BrowserWindow tear down, it could be possible that the BrowserActionsContainer is destroyed before the bubble begins the closing/destroying process, right? In which case, the later call (when the bubble *is* closing/destroying as a later part of the BrowserWindow tear down) would notify the now-destroyed BrowserActionsContainer. I gladly defer to your expertise here, since you're much more familiar with the views framework, but it seems like your suggestions from #7 should work. Of the two, calling CloseNow() seems a bit safer, just to ensure the bubble doesn't inadvertently call back into the container for anything else (and there's no point in keeping the bubble around if everything's shutting down).
,
Mar 22 2018
The NextAction date has arrived: 2018-03-22
,
Mar 23 2018
http://go/crash/2c6f04047f83d14a is still the only crash. m66 is in beta and these `ToolbarActionsBarBubbleViews` dialogs are 50/50 on Mac, about to go to 100%, and then through 100% to stable.. let's check in then.
,
Mar 26 2018
,
Mar 26 2018
+cc sky@ for the questions below, but I'm also interested in your thoughts, tapted@. My reading of #7 and #8: 1) ~BrowserActionsContainer() can only happen when the window is already being destructed, in which case the attached secondary widget should already be closing or closed (and therefore it's fine to CloseNow() it) 2) The root issue is that Widget::CloseAllSecondaryWidgets() just unceremoniously closes the involved widgets, without going via Widget::Close() (and therefore without going via WidgetObserver::OnWidgetClosing(). To fix this at the root, could we either: a) Make Widget::CloseAllSecondaryWidgets() use Widget::Close() via GetWidgetForNativeWindow(), or b) Have Widget::CloseAllSecondaryWidgets() directly prod the observers of the widgets they are closing?
,
Mar 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/22fa388609e18066449d4e124ba660594da87e6f commit 22fa388609e18066449d4e124ba660594da87e6f Author: Trent Apted <tapted@chromium.org> Date: Mon Mar 26 21:14:08 2018 CHECK() if a WidgetDelegate would be deleted before the Widget it initialized is deleted. WidgetDelegates must outlive their Widget. Crashes suggest situations where they may not, but can't pinpoint the actual client code. The crash is instead attributed to a UAF on the widget delegate either in a layout call (during an animation), or when Widget::OnNativeWidgetDestroyed() tries to delete the delegate (again). This CL adds a CHECK() in ~WidgetDelegate which should track down the misbehaving code. Bug: 825091, 814821, 808318 Change-Id: I9490ff1682ee236436edf075f7c237ccf910c44c Reviewed-on: https://chromium-review.googlesource.com/977244 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#545873} [modify] https://crrev.com/22fa388609e18066449d4e124ba660594da87e6f/ui/views/widget/widget.cc [modify] https://crrev.com/22fa388609e18066449d4e124ba660594da87e6f/ui/views/widget/widget_delegate.cc [modify] https://crrev.com/22fa388609e18066449d4e124ba660594da87e6f/ui/views/widget/widget_delegate.h
,
Mar 26 2018
I think Widget::CloseAllSecondaryWidgets() is doing the right thing. Widget client code _always_ needs to handle the case where it is unceremoniously closed (i.e. with CloseNow() rather than Close()+PostTask). The problem is that the OS can (effectively) invoke CloseNow on any window, any time it wants. Windows does this during the logoff procedure. Linux does it with `xkill`. Mac does it with -[NSWindow close] (although I think on Mac it is actually possible to refuse still). So, Widget doesn't (and can't) guarantee that WidgetObserver::OnWidgetClosing() gets invoked. But it exists as an early opportunity for a dialog to do things with the NativeWidget that would be too late to attempt in OnWidgetDestroying(). Dialogs do not always cater for this, and it's a big source of bugs. With TestBrowserDialog, I'm hoping to actually exercise the different close codepaths for dialogs that have tests. The current default is to use CloseNow in TestBrowserDialog. One dialog did need TestBrowserDialog::AlwaysCloseAsynchronously() to get running, but the code seemed innocent.
,
Mar 26 2018
tapted@, is there anything pending besided cl listed at #14 for this bug?
,
Mar 26 2018
There are no CLs pending. But the CL on #14 will hopefully get us some clearer stacks to diagnose this issue (and Issue 825091, Issue 814821) further. Possibly others. This issue specifically has a very low crash rate on Mac -- https://crash.corp.google.com/browse?q=product.name%3D%27Chrome_Mac%27%20AND%20EXISTS%20(SELECT%201%20FROM%20UNNEST(productdata)%20WHERE%20key%3D%27windowInfo%27)&stbtiq=&reportid=&index=0 and other platforms -- https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27views%3A%3AWidget%3A%3AOnNativeWidgetDestroying%27#-property-selector,productversion:1000,-magicsignature:50,-magicsignature2:50,-stablesignature:50,-magicsignaturesorted:50 E.g. there are ~6 on m65 stable, and 3 on m66 Beta, the latter all ChromeOS -- https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27views%3A%3AWidget%3A%3AOnNativeWidgetDestroying%27%20AND%20product.Version%20like%20%2766.0.3359.%25%27
,
Mar 27 2018
I agree with what Trent says in comment 15.
,
Mar 29 2018
The root cause for this is most likely issue 827169. I'm going to duplicate this issue into that one and mark that one for MacViews instead.
,
Mar 29 2018
,
Mar 29 2018
ellyjones@, per comment #19, this is dupe. Can status be change to Duplicate instead of Assigned?
,
Mar 29 2018
No - I just unduped it because it's not actually a dupe of 827169, per 827169 c#7.
,
Mar 29 2018
OK, got it. Sorry missed the previous update.
,
Mar 29 2018
** Bulk Edit ** There are only two M67 dev releases left on 04/03 & 04/10 before M67 branch on 04/12. Please try to land the fix ASAP to trunk so we can move forward with 50%-50% experiment on M67 Canary/Dev (if possible at all). Thank you.
,
Apr 3 2018
** Bulk Edit ** There is only one dev release left 04/10 before M67 branch on 04/12. Please try to land the fix ASAP to trunk so we can move forward with 50%-50% experiment on M67 Canary/Dev (if possible at all). Thank you. FYI: Change/Fix has to be landed in trunk latest by 4:00 PM PT, Friday (04/06) so we can pick it up for next week dev release.
,
Apr 6 2018
We have four crashes for this, three from M66 and one from M67. Two are "Closing Your Internet connection is being controlled (NativeWidgetMacNSWindow)". One is "Closing Open (NSOpenPanel)". One is "Closing overlay (TabWindowOverlayWindow)". The former two are via: 0x0000000105938b0e (Google Chrome Framework -widget.cc:1086 ) views::Widget::OnNativeWidgetDestroying() 0x00000001058ce3bd (Google Chrome Framework -bridged_native_widget.mm:786 ) views::BridgedNativeWidget::OnWindowWillClose() 0x00000001058d2c0a (Google Chrome Framework -views_nswindow_delegate.mm:126 ) -[ViewsNSWindowDelegate windowWillClose:] The latter two are via: 0x00007fff89d5a4dd (libobjc.A.dylib + 0x000014dd ) objc_msgSend 0x0000000111eab254 (Google Chrome Framework -native_widget_mac.mm:657 ) views::Widget::CloseAllSecondaryWidgets() 0x0000000112bcc0a0 (Google Chrome Framework -browser_list.cc:111 ) BrowserList::RemoveBrowser(Browser*) 0x0000000112bbdcf4 (Google Chrome Framework -browser.cc:497 ) Browser::~Browser() And more specifically the third is a null+0x18 dereference and the fourth is a dereference of 0x0000656d6f726878, which is "chrome"+0x15 packed little-endian. So I think there's probably two separate issues: 1) The "Your internet connection is being controlled" dialog has lifetime issues 2) Widget::CloseAllSecondaryWidgets() on Mac is closing invalid windows sometimes I'll look into (1) today, but given the low crash rate here (and the cross-platform nature of it) this isn't super important.
,
Apr 6 2018
I tried to reproduce (1) a bunch of different ways. The code doesn't "smell" right to me; in ToolbarActionsBarBubbleViews::ButtonPressed() there's a block comment like this: // Note that the Widget may or may not already be closed at this point, // depending on delegate_->ShouldCloseOnDeactivate(). Widget::Close() protects // against multiple calls (so long as they are not nested), and Widget // destruction is asynchronous, so it is safe to call Close() again. This seems dubious to me - if the widget destruction actually *did* happen synchronously in the earlier close, the Widget::Close() would be a UAF. As it happens, the "your internet connection is being controlled" bubble (ProxyOverriddenBubbleDelegate) is one of very few bubbles that don't close when deactivated, so it's one of the few bubbles for which the GetWidget()->Close() in this function actually does cause the close, which otherwise happens during the earlier OnBubbleClosed() (I think, and the comment also suggests). On the other hand, I can't concretely find anything *wrong* with this code, and I've still had no success reproducing the crash either. We might be better off waiting for more crashes and seeing where that leads us. I'm going to kick this out of Target-67, though - this bug's been around for a bit now and the crash rates are low.
,
Apr 25 2018
The NextAction date has arrived: 2018-04-25
,
Apr 25 2018
Something similar is seen on other platforms, e.g. on Windows: 0x6b7568c1 (chrome.dll -widget.cc:1086 ) views::Widget::OnNativeWidgetDestroying() 0x6b760b6b (chrome.dll -desktop_window_tree_host_win.cc:790 ) views::DesktopWindowTreeHostWin::HandleDestroying() 0x6b76ca5a (chrome.dll -hwnd_message_handler.cc:1454 ) views::HWNDMessageHandler::OnDestroy() 0x6ab01d53 (chrome.dll -hwnd_message_handler.h:426 ) views::HWNDMessageHandler::_ProcessWindowMessage(HWND__ *,unsigned int,unsigned int,long,long &,unsigned long) 0x6ab0138d (chrome.dll -hwnd_message_handler.cc:939 ) views::HWNDMessageHandler::OnWndProc(unsigned int,unsigned int,long) 0x6a954c5a (chrome.dll -window_impl.cc:303 ) gfx::WindowImpl::WndProc(HWND__ *,unsigned int,unsigned int,long) 0x6a954be8 (chrome.dll -wrapped_window_proc.h:76 ) base::win::WrappedWindowProc<&gfx::WindowImpl::WndProc(HWND__ *,unsigned int,unsigned int,long)>(HWND__ *,unsigned int,unsigned int,long) 0x75ab86ee (USER32.dll + 0x000186ee ) InternalCallWinProc 0x75ab8875 (USER32.dll + 0x00018875 ) UserCallWinProcCheckWow 0x75ab70f3 (USER32.dll + 0x000170f3 ) DispatchClientMessage 0x75ab738e (USER32.dll + 0x0001738e ) __fnDWORD 0x7769642d (ntdll.dll + 0x0004642d ) KiUserCallbackDispatcher 0x776963df (ntdll.dll + 0x000463df ) KiUserApcDispatcher 0x6a96555e (chrome.dll -task_annotator.cc:61 ) base::debug::TaskAnnotator::RunTask(char const *,base::PendingTask *) 0x6a968792 (chrome.dll -incoming_task_queue.cc:124 ) base::internal::IncomingTaskQueue::RunTask(base::PendingTask *) 0x6a967375 (chrome.dll -message_loop.cc:395 ) base::MessageLoop::RunTask(base::PendingTask *) 0x6a967192 (chrome.dll -message_loop.cc:407 ) base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) 0x6a92bdc2 (chrome.dll -message_loop.cc:451 ) base::MessageLoop::DoWork() 0x6a9d6677 (chrome.dll -message_pump_win.cc:173 ) base::MessagePumpForUI::DoRunLoop() 0x6a92bb8d (chrome.dll -message_pump_win.cc:56 ) base::MessagePumpWin::Run(base::MessagePump::Delegate *) 0x6a92bafe (chrome.dll -message_loop.cc:346 ) base::MessageLoop::Run(bool) 0x6a92b82d (chrome.dll -run_loop.cc:133 ) base::RunLoop::Run() 0x6ac19a7f (chrome.dll -chrome_browser_main.cc:2190 ) ChromeBrowserMainParts::MainMessageLoopRun(int *) 0x6ac1990c (chrome.dll -browser_main_loop.cc:1104 ) content::BrowserMainLoop::RunMainMessageLoopParts() 0x6ac198c7 (chrome.dll -browser_main_runner.cc:160 ) content::BrowserMainRunnerImpl::Run() 0x6a8e5813 (chrome.dll -browser_main.cc:46 ) content::BrowserMain(content::MainFunctionParams const &) 0x6a8e5707 (chrome.dll -content_main_runner.cc:423 ) content::RunNamedProcessTypeMain(std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,content::MainFunctionParams const &,content::ContentMainDelegate *) 0x6a8e55e1 (chrome.dll -content_main_runner.cc:703 ) content::ContentMainRunnerImpl::Run() 0x6a8d5a56 (chrome.dll -main.cc:453 ) service_manager::Main(service_manager::MainParams const &) 0x6a8d5737 (chrome.dll -content_main.cc:19 ) content::ContentMain(content::ContentMainParams const &) 0x6a8d2349 (chrome.dll -chrome_main.cc:101 ) ChromeMain 0x012c3001 (chrome.exe -main_dll_loader_win.cc:199 ) MainDllLoader::Launch(HINSTANCE__ *,base::TimeTicks) 0x012c145c (chrome.exe -chrome_exe_main_win.cc:230 ) wWinMain 0x0137df67 (chrome.exe -exe_common.inl:283 ) __scrt_common_main_seh 0x75c91173 (kernel32.dll + 0x00051173 ) BaseThreadInitThunk 0x776ab3f4 (ntdll.dll + 0x0005b3f4 ) __RtlUserThreadStart 0x776ab3c7 (ntdll.dll + 0x0005b3c7 ) _RtlUserThreadStart When I restrict the OS to Mac, we continue to see a very low level of this crash - about once a day. Perhaps we can add some DCHECKs for the WidgetDelegate, observers, and NonClientView not being destroyed early. In particular WidgetObserver::~WidgetObserver() does **NOT** remove the observer from widgets it's observing, so that seems like fertile ground for lifetime bugs.
,
Apr 25 2018
Pls mark the bug as fixed if CL is landed in trunk and nothing else is pending. Thank you.
,
Jun 20 2018
,
Jul 12
,
Jul 12
,
Jul 24
,
Jul 26
,
Jul 26
The change that fixes this is likely unsafe to merge to M69
,
Aug 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/30f97fddc348e14c23499ffc0f638deff901641c commit 30f97fddc348e14c23499ffc0f638deff901641c Author: Trent Apted <tapted@chromium.org> Date: Tue Aug 21 09:03:47 2018 base::CheckedObserver - a common base class for observers. Turns a possible UAF when trying to notify a deleted observer into a CHECK(). This is achieved with minimal changes to ObserverList and an adapter that gives WeakPtr-like semantics to an observer interface. base::ObserverList<T>::Unchecked continues to use raw pointers that are unchecked. Bug: 842987, 808318 Change-Id: I8ab20845f4f6e1d2559490287731cea2dbf40d39 Reviewed-on: https://chromium-review.googlesource.com/1053338 Commit-Queue: Trent Apted <tapted@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#584694} [modify] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/base/BUILD.gn [modify] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/base/memory/weak_ptr.h [modify] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/base/memory/weak_ptr_unittest.cc [modify] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/base/observer_list.h [add] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/base/observer_list_internal.cc [add] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/base/observer_list_internal.h [add] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/base/observer_list_types.cc [add] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/base/observer_list_types.h [modify] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/base/observer_list_unittest.cc [add] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/base/observer_list_unittest.nc [modify] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/base/test/gtest_util.h [modify] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/ui/views/widget/widget.h [modify] https://crrev.com/30f97fddc348e14c23499ffc0f638deff901641c/ui/views/widget/widget_observer.h
,
Nov 19
We have been consistently seeing this crash on M71 Beta when compared to previous Chrome Beta milestones. Please find the crash history here : https://goto.google.com/mjihf Stack : Thread 0 (id: 0x4317) CRASHED [EXC_BREAKPOINT / EXC_I386_BPT @ 0x000000010bdbed06 ] MAGIC SIGNATURE THREAD Stack Quality84%Show frame trust levels 0x000000010bdbed06 (Google Chrome Framework -observer_list_internal.h:73) views::Widget::OnNativeWidgetDestroying() 0x000000010bdd28be (Google Chrome Framework -bridged_native_widget_host_impl.mm:728) views::BridgedNativeWidgetHostImpl::OnWindowWillClose() 0x000000010bd384be (Google Chrome Framework -bridged_native_widget_impl.mm:702) views::BridgedNativeWidgetImpl::OnWindowWillClose() 0x000000010bdce0d8 (Google Chrome Framework -views_nswindow_delegate.mm:143) -[ViewsNSWindowDelegate windowWillClose:] 0x00007fff2e1e83f1 (CoreFoundation+ 0x0009f3f1) __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ 0x00007fff2e1e836b (CoreFoundation+ 0x0009f36b) ___CFXRegistrationPost_block_invoke 0x00007fff2e1e828c (CoreFoundation+ 0x0009f28c) _CFXRegistrationPost 0x00007fff2e1f06d8 (CoreFoundation+ 0x000a76d8) ___CFXNotificationPost_block_invoke 0x00007fff2e157e89 (CoreFoundation+ 0x0000ee89) -[_CFXNotificationRegistrar find:object:observer:enumerator:] 0x00007fff2e15724c (CoreFoundation+ 0x0000e24c) _CFXNotificationPost 0x00007fff304df98a (Foundation+ 0x0001198a) -[NSNotificationCenter postNotificationName:object:userInfo:] 0x00007fff2bfeff14 (AppKit+ 0x00931f14) -[NSWindow _finishClosingWindow] 0x00007fff2ba5f22b (AppKit+ 0x003a122b) -[NSWindow _close] 0x000000010bd39661 (Google Chrome Framework -bind_internal.h:498) base::internal::Invoker<base::internal::BindState<base::mac::ScopedBlock<void () block_pointer> >, void ()>::RunOnce(base::internal::BindStateBase*) 0x000000010a144769 (Google Chrome Framework -callback.h:99) base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) 0x000000010a15ff1e (Google Chrome Framework -message_loop.cc:434) base::MessageLoop::RunTask(base::PendingTask*) 0x000000010a160272 (Google Chrome Framework -message_loop.cc:445) base::MessageLoop::DoWork() 0x000000010a1624e9 (Google Chrome Framework -message_pump_mac.mm:455) base::MessagePumpCFRunLoopBase::RunWork() 0x000000010a1543c9 (Google Chrome Framework+ 0x0274d3c9) base::mac::CallWithEHFrame(void () block_pointer) 0x000000010a161e4e (Google Chrome Framework -message_pump_mac.mm:431) base::MessagePumpCFRunLoopBase::RunWorkSource(void*) 0x00007fff2e1a1154 (CoreFoundation+ 0x00058154) __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ 0x00007fff2e1a10fa (CoreFoundation+ 0x000580fa) __CFRunLoopDoSource0 0x00007fff2e184b94 (CoreFoundation+ 0x0003bb94) __CFRunLoopDoSources0 0x00007fff2e18413d (CoreFoundation+ 0x0003b13d) __CFRunLoopRun 0x00007fff2e183a27 (CoreFoundation+ 0x0003aa27) CFRunLoopRunSpecific 0x00007fff2d41cb34 (HIToolbox+ 0x0000ab34) RunCurrentEventLoopInMode 0x00007fff2d41c86a (HIToolbox+ 0x0000a86a) ReceiveNextEventCommon 0x00007fff2d41c5e7 (HIToolbox+ 0x0000a5e7) _BlockUntilNextEventMatchingListInModeWithFilter 0x00007fff2b6d8eb6 (AppKit+ 0x0001aeb6) _DPSNextEvent 0x00007fff2b6d7c55 (AppKit+ 0x00019c55) -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] 0x0000000109d4250f (Google Chrome Framework -chrome_browser_application_mac.mm:255) __71-[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]_block_invoke 0x000000010a1543c9 (Google Chrome Framework+ 0x0274d3c9) base::mac::CallWithEHFrame(void () block_pointer) 0x0000000109d42443 (Google Chrome Framework -chrome_browser_application_mac.mm:254) -[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:] 0x00007fff2b6d1cb8 (AppKit+ 0x00013cb8) -[NSApplication run] 0x000000010a162dab (Google Chrome Framework -message_pump_mac.mm:808) base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) 0x000000010a16192d (Google Chrome Framework -message_pump_mac.mm:184) base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) 0x000000010a1848e4 (Google Chrome Framework -run_loop.cc:102) <name omitted> 0x0000000109d48c0c (Google Chrome Framework -chrome_browser_main.cc:2028) ChromeBrowserMainParts::MainMessageLoopRun(int*) 0x00000001086e4b03 (Google Chrome Framework -browser_main_loop.cc:998) content::BrowserMainLoop::RunMainMessageLoopParts() 0x00000001086e7131 (Google Chrome Framework -browser_main_runner_impl.cc:165) content::BrowserMainRunnerImpl::Run() 0x00000001086e187a (Google Chrome Framework -browser_main.cc:47) content::BrowserMain(content::MainFunctionParams const&) 0x0000000109cfe08a (Google Chrome Framework -content_main_runner_impl.cc:541) content::ContentMainRunnerImpl::Run(bool) 0x000000010ba11bdc (Google Chrome Framework -main.cc:472) service_manager::Main(service_manager::MainParams const&) 0x0000000109cfd273 (Google Chrome Framework -content_main.cc:19) content::ContentMain(content::ContentMainParams const&) 0x0000000107a0ae5e (Google Chrome Framework -chrome_main.cc:102) ChromeMain 0x0000000104ee7dcd (Google Chrome -chrome_exe_main_mac.cc:101) main 0x00007fff5b36108c (libdyld.dylib+ 0x0001708c) start 0x00007fff5b36108c (libdyld.dylib+ 0x0001708c) start
,
Jan 9
We have an ongoing very low rate of this on Mac, and a higher rate on ChromeOS. I think it's pretty clear this is a cross-platform issue so I'm going to assign it to robliao@ for Views triage. I'm also stripping R-V-EI since there's nothing sensitive in this bug.
,
Jan 15
Note WidgetObserver inheriting from CheckedObserver flushed out one leaked observer in the Views Cast dialog -- that was Issue 883976 . With a repro case, we were able to flush out the stack with a diagnostic CL --> https://chromium-review.googlesource.com/c/chromium/src/+/1226472 Crashes attributable to the known repro case for the cast dialog should have dried up after r592183 (71.0.3556.0). go/mjihf shows crashes in 71.0.3578.98, so another dialog (or another repro case) that we don't know of is likely still leaking a WidgetObserver. Checking experiments, https://goto.google.com/ccivg shows 76.95% of crashes having ViewsCastDialog: Enabled (and none with it Disabled). It looks like it's currently at 50/50, so it's out of proportion, but maybe not a definite signal.
,
Jan 16
Actually, consider just 72.0.3593.0 for now (it ranks highly in the link in #c1) - 43 crashes [1] - 42 are ChromeOS [1] (1 mac) - 100% are ViewsCastDialog Enabled [1] - Only 39% of *all* ChromeOS 72.0.3593.0 crashes have ViewsCastDialog Enabled. [2] branch 3593 is a bit old though. Let's focus on the branch going to beta (3626) - 12 crashes [3] - 1+6+5 (chromeOS,linux,mac) [3] - 100% are ViewsCastDialog Enabled [3] - (vs 20/20 split across all products/platforms) [4] The numbers are low for now, but I think that's a clear signal of "something" involving the cast dialog. ChromeOS releases lag behind which might be why the numbers are low at the moment for 3626 (3626 only went to a ChromeOS beta 5 days ago, and many devices lag behind that still). I'll make a new bug. [1] dev - https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27views%3A%3AWidget%3A%3AOnNativeWidgetDestroying%27+AND+product.Version%3D%2772.0.3593.0%27#-propertyselector,productname:1000,-magicsignature:50,-magicsignature2:50,-stablesignature:50,+experiments [2] dev - https://crash.corp.google.com/browse?q=product_name%3D%27Chrome_ChromeOS%27+AND+product.Version%3D%2772.0.3593.0%27#-propertyselector,-productname:1000,-magicsignature:50,-magicsignature2:50,-stablesignature:50,+experiments:100,-magicsignaturesorted:50 [3] beta - https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27views%3A%3AWidget%3A%3AOnNativeWidgetDestroying%27+AND+product.Version+like+%2772.0.3626.%25%27#-propertyselector,productname:1000,productversion:100,-magicsignature:50,-magicsignature2:50,-stablesignature:50,+experiments [4] https://crash.corp.google.com/browse?q=product.Version+like+%2772.0.3626.%25%27#-propertyselector,productname:1000,productversion:100,-magicsignature:50,-magicsignature2:50,-stablesignature:50,+experiments:100
,
Jan 16
> I'll make a new bug Filed Issue 922328 for m72/cast. Also filed Issue 922327 which has OnNativeWidgetDestroying in the stack, but a different magic signature. And is definitely ChromeOS only and not related to cast. |
||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||
Comment 1 by tapted@chromium.org
, Feb 2 2018