Summary: | Web Inspector: CRASH in backend at Inspector::HeapFrontendDispatcher::garbageCollected + 552 when closing frontend/inspected page | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||||||||
Component: | Web Inspector | Assignee: | BJ Burg <bburg> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bburg, commit-queue, joepeck, keith_miller, mark.lam, mattbaker, msaboff, nvasilyev, saam, timothy, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | 159105 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
BJ Burg
2016-06-23 16:06:02 PDT
Created attachment 281938 [details]
Proposed Fix
Comment on attachment 281938 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=281938&action=review > Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.cpp:297 > + // The frontend could have gone away after the activity was scheduled. > + if (m_frontendDispatcher) Is `this` valid? Could it be stale? Comment on attachment 281938 [details]
Proposed Fix
Yeah, you are right. I guess it needs to go the direction of CSSAgent's ChangeRegionOversetTask... blah.
Created attachment 282001 [details]
Proposed Fix
Comment on attachment 282001 [details] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=282001&action=review r=me! > Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.cpp:52 > + RunLoop::Timer<SendGarbageCollectionEventsTask> m_timer; I did not know there was a RunLoop::Timer! This is excellent. Comment on attachment 282001 [details] Proposed Fix Clearing flags on attachment: 282001 Committed r202443: <http://trac.webkit.org/changeset/202443> All reviewed patches have been landed. Closing bug. This has introduced a number of memory corruption crashes, rolling out. Will follow up with details in e-mail. Re-opened since this is blocked by bug 159105 Created attachment 282033 [details]
Proposed Fix v3
Comment on attachment 282033 [details]
Proposed Fix v3
Let's give this another try.
Comment on attachment 282033 [details] Proposed Fix v3 Clearing flags on attachment: 282033 Committed r202492: <http://trac.webkit.org/changeset/202492> All reviewed patches have been landed. Closing bug. Causing JSC stress tests to fail due to an (apparently not harmless) change introduced in the last patch. Will post a fix. Reopening to attach new patch. Created attachment 282165 [details]
Followup fix
Landed followup in <https://trac.webkit.org/r202515>. Comment on attachment 282033 [details] Proposed Fix v3 View in context: https://bugs.webkit.org/attachment.cgi?id=282033&action=review > Source/JavaScriptCore/inspector/agents/InspectorHeapAgent.cpp:71 > + std::lock_guard<Lock> lock(m_mutex); Maybe we should use LockHolder instead of std::lock_guard. |