Bug 191734

Summary: ASSERTION FAILED: m_messageReceivers.contains(...) under ViewGestureController removeMessageReceiver
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: WebKit Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, cdumez, commit-queue, jmulani, koivisto, rniwa, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=196029
Bug Depends on:    
Bug Blocks: 191083    
Attachments:
Description Flags
API test
none
Patch
none
Patch
none
Patch none

Description Joseph Pecoraro 2018-11-15 21:43:56 PST
ASSERTION FAILED: m_messageReceivers.contains(...) under ViewGestureController removeMessageReceiver

Steps to Reproduce:
1. Load a web page in Safari
2. Kill WebContent Process for that page (kill -9)
3. Kill WebContent Process for that page again (kill -9)
4. Click the "Reload" button in Safari
5. Kill WebContent Process for that page again (kill -9)
  => ASSERT

ASSERTION FAILED: m_messageReceivers.contains(std::make_pair(messageReceiverName, destinationID))
/Volumes/Data/Code/safari/OpenSource/Source/WebKit/Platform/IPC/MessageReceiverMap.cpp(72) : void IPC::MessageReceiverMap::removeMessageReceiver(IPC::StringReference, uint64_t)
1   0x110a97299 WTFCrash
2   0x119dad6ab WTFCrashWithInfo(int, char const*, char const*, int)
3   0x119e5c380 IPC::MessageReceiverMap::removeMessageReceiver(IPC::StringReference, unsigned long long)
4   0x11a4430a7 WebKit::ChildProcessProxy::removeMessageReceiver(IPC::StringReference, unsigned long long)
5   0x11a9172f0 WebKit::ViewGestureController::~ViewGestureController()
6   0x11a9174f5 WebKit::ViewGestureController::~ViewGestureController()
7   0x11a917519 WebKit::ViewGestureController::~ViewGestureController()
8   0x11a569269 WebKit::WebViewImpl::processDidExit()
9   0x11a813eeb WebKit::PageClientImpl::processDidExit()
10  0x11a67c8ba WebKit::WebPageProxy::resetStateAfterProcessExited(WebKit::ProcessTerminationReason)
11  0x11a649067 WebKit::WebPageProxy::processDidTerminate(WebKit::ProcessTerminationReason)
12  0x11a73af38 WebKit::WebProcessProxy::processDidTerminateOrFailedToLaunch()
13  0x11a73acbc WebKit::WebProcessProxy::didClose(IPC::Connection&)
14  0x119e157d6 IPC::Connection::connectionDidClose()::$_13::operator()()
...

Notes:
- Seems a receiver is being removed that wasn't registered?
- This doesn't happen on simple process restarts, but does happen after the special safari "Reload" case, what is different?
Comment 1 Tim Horton 2018-11-16 00:48:15 PST
A bit odd since we add in the constructor and remove in the destructor. Maybe something else emptied out the map?
Comment 2 Alexey Proskuryakov 2018-11-17 12:18:19 PST
*** Bug 191780 has been marked as a duplicate of this bug. ***
Comment 3 Alexey Proskuryakov 2018-11-17 12:20:46 PST
More practical steps to reproduce in bug 191780.

Possibly a recent regression, given that both reports are recent. Chris, is this related to r237615?
Comment 4 Radar WebKit Bug Importer 2018-11-17 12:21:34 PST
<rdar://problem/46151497>
Comment 5 Chris Dumez 2018-11-17 12:27:38 PST
Could be related to PSON or process prewarming if this is recent. I'll investigate.
Comment 6 Chris Dumez 2018-11-17 12:36:19 PST
After clicking the reload button in Safari I see:
CHRIS: addMessageReceiver(1, 0x7fce56c3dd90), process is 0x1324e3400

Then we killing the process I see:
CHRIS: removeMessageReceiver(1, 0x7fce56c3dd90) process is 0x135ce1400

I added itself as a message receiver from one process and is trying to unregister itself from another.
Comment 7 Chris Dumez 2018-11-17 12:40:31 PST
Yes, this is a PSON regression. This is due to us not destroying the gesture controller in WebViewImpl::processWillSwap(). We only do it in WebViewImpl::processDidExit().

We did this to fix Bug 191083. I will fix it shortly.
Comment 8 Chris Dumez 2018-11-17 13:45:52 PST
Created attachment 355200 [details]
API test
Comment 9 Chris Dumez 2018-11-17 14:03:22 PST
Created attachment 355201 [details]
Patch
Comment 10 Ryosuke Niwa 2018-11-17 14:08:10 PST
Comment on attachment 355201 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=355201&action=review

Looks sane to me. I guess we should wait for EWS.

> Source/WebKit/ChangeLog:10
> +        after we've relaunched a new Webprocess. The ViewGestureController controller takes care

Nit: Web*P*rocess.

> Source/WebKit/ChangeLog:19
> +        To address the issue, we now take sure the ViewGestureController unregisters itself from

we now *make* sure?
Comment 11 Chris Dumez 2018-11-17 14:10:26 PST
Created attachment 355204 [details]
Patch
Comment 12 Chris Dumez 2018-11-17 14:44:35 PST
Created attachment 355207 [details]
Patch
Comment 13 WebKit Commit Bot 2018-11-17 16:25:27 PST
Comment on attachment 355207 [details]
Patch

Clearing flags on attachment: 355207

Committed r238356: <https://trac.webkit.org/changeset/238356>
Comment 14 WebKit Commit Bot 2018-11-17 16:25:29 PST
All reviewed patches have been landed.  Closing bug.