Title: [233601] trunk/Source/WebKit
- Revision
- 233601
- Author
- [email protected]
- Date
- 2018-07-06 16:26:09 -0700 (Fri, 06 Jul 2018)
Log Message
Add release assertions to try and catch a possible HashMap corruption in NetworkConnectionToWebProcess
https://bugs.webkit.org/show_bug.cgi?id=187417
Reviewed by Ryosuke Niwa.
Add assertions to make sure we:
- Always use NetworkConnectionToWebProcess::m_networkResourceLoaders from the main thread
- Never use a 0 identifier as key for NetworkConnectionToWebProcess::m_networkResourceLoaders
We see crashes (rdar://problem/39265927) that only seem to make sense if this HashMap was
somehow getting corrupted. Let's try and catch the most common reasons for HashMap corruption.
* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::didCleanupResourceLoader):
(WebKit::NetworkConnectionToWebProcess::didReceiveMessage):
(WebKit::NetworkConnectionToWebProcess::scheduleResourceLoad):
(WebKit::NetworkConnectionToWebProcess::performSynchronousLoad):
(WebKit::NetworkConnectionToWebProcess::removeLoadIdentifier):
(WebKit::NetworkConnectionToWebProcess::setDefersLoading):
(WebKit::NetworkConnectionToWebProcess::convertMainResourceLoadToDownload):
* WebProcess/Network/WebResourceLoader.cpp:
(WebKit::WebResourceLoader::messageSenderDestinationID):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (233600 => 233601)
--- trunk/Source/WebKit/ChangeLog 2018-07-06 23:11:02 UTC (rev 233600)
+++ trunk/Source/WebKit/ChangeLog 2018-07-06 23:26:09 UTC (rev 233601)
@@ -1,3 +1,28 @@
+2018-07-06 Chris Dumez <[email protected]>
+
+ Add release assertions to try and catch a possible HashMap corruption in NetworkConnectionToWebProcess
+ https://bugs.webkit.org/show_bug.cgi?id=187417
+
+ Reviewed by Ryosuke Niwa.
+
+ Add assertions to make sure we:
+ - Always use NetworkConnectionToWebProcess::m_networkResourceLoaders from the main thread
+ - Never use a 0 identifier as key for NetworkConnectionToWebProcess::m_networkResourceLoaders
+
+ We see crashes (rdar://problem/39265927) that only seem to make sense if this HashMap was
+ somehow getting corrupted. Let's try and catch the most common reasons for HashMap corruption.
+
+ * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+ (WebKit::NetworkConnectionToWebProcess::didCleanupResourceLoader):
+ (WebKit::NetworkConnectionToWebProcess::didReceiveMessage):
+ (WebKit::NetworkConnectionToWebProcess::scheduleResourceLoad):
+ (WebKit::NetworkConnectionToWebProcess::performSynchronousLoad):
+ (WebKit::NetworkConnectionToWebProcess::removeLoadIdentifier):
+ (WebKit::NetworkConnectionToWebProcess::setDefersLoading):
+ (WebKit::NetworkConnectionToWebProcess::convertMainResourceLoadToDownload):
+ * WebProcess/Network/WebResourceLoader.cpp:
+ (WebKit::WebResourceLoader::messageSenderDestinationID):
+
2018-07-06 Brian Burg <[email protected]>
WebDriver: hang in Selenium test alerts_tests.py::testShouldHandleAlertOnPageBeforeUnload
Modified: trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (233600 => 233601)
--- trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2018-07-06 23:11:02 UTC (rev 233600)
+++ trunk/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp 2018-07-06 23:26:09 UTC (rev 233601)
@@ -88,6 +88,8 @@
void NetworkConnectionToWebProcess::didCleanupResourceLoader(NetworkResourceLoader& loader)
{
+ RELEASE_ASSERT(loader.identifier());
+ RELEASE_ASSERT(RunLoop::isMain());
ASSERT(m_networkResourceLoaders.get(loader.identifier()) == &loader);
m_networkResourceLoaders.remove(loader.identifier());
@@ -101,9 +103,13 @@
}
if (decoder.messageReceiverName() == Messages::NetworkResourceLoader::messageReceiverName()) {
+ RELEASE_ASSERT(RunLoop::isMain());
+ RELEASE_ASSERT(decoder.destinationID());
auto loaderIterator = m_networkResourceLoaders.find(decoder.destinationID());
- if (loaderIterator != m_networkResourceLoaders.end())
+ if (loaderIterator != m_networkResourceLoaders.end()) {
+ RELEASE_ASSERT(loaderIterator->value);
loaderIterator->value->didReceiveNetworkResourceLoaderMessage(connection, decoder);
+ }
return;
}
@@ -238,6 +244,8 @@
void NetworkConnectionToWebProcess::scheduleResourceLoad(NetworkResourceLoadParameters&& loadParameters)
{
auto identifier = loadParameters.identifier;
+ RELEASE_ASSERT(identifier);
+ RELEASE_ASSERT(RunLoop::isMain());
ASSERT(!m_networkResourceLoaders.contains(identifier));
auto loader = NetworkResourceLoader::create(WTFMove(loadParameters), *this);
@@ -248,6 +256,8 @@
void NetworkConnectionToWebProcess::performSynchronousLoad(NetworkResourceLoadParameters&& loadParameters, Messages::NetworkConnectionToWebProcess::PerformSynchronousLoad::DelayedReply&& reply)
{
auto identifier = loadParameters.identifier;
+ RELEASE_ASSERT(identifier);
+ RELEASE_ASSERT(RunLoop::isMain());
ASSERT(!m_networkResourceLoaders.contains(identifier));
auto loader = NetworkResourceLoader::create(WTFMove(loadParameters), *this, WTFMove(reply));
@@ -280,6 +290,9 @@
void NetworkConnectionToWebProcess::removeLoadIdentifier(ResourceLoadIdentifier identifier)
{
+ RELEASE_ASSERT(identifier);
+ RELEASE_ASSERT(RunLoop::isMain());
+
RefPtr<NetworkResourceLoader> loader = m_networkResourceLoaders.get(identifier);
// It's possible we have no loader for this identifier if the NetworkProcess crashed and this was a respawned NetworkProcess.
@@ -299,6 +312,9 @@
void NetworkConnectionToWebProcess::setDefersLoading(ResourceLoadIdentifier identifier, bool defers)
{
+ RELEASE_ASSERT(identifier);
+ RELEASE_ASSERT(RunLoop::isMain());
+
RefPtr<NetworkResourceLoader> loader = m_networkResourceLoaders.get(identifier);
if (!loader)
return;
@@ -352,6 +368,8 @@
void NetworkConnectionToWebProcess::convertMainResourceLoadToDownload(PAL::SessionID sessionID, uint64_t mainResourceLoadIdentifier, DownloadID downloadID, const ResourceRequest& request, const ResourceResponse& response)
{
+ RELEASE_ASSERT(RunLoop::isMain());
+
auto& networkProcess = NetworkProcess::singleton();
// In case a response is served from service worker, we do not have yet the ability to convert the load.
if (!mainResourceLoadIdentifier || response.source() == ResourceResponse::Source::ServiceWorker) {
Modified: trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp (233600 => 233601)
--- trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp 2018-07-06 23:11:02 UTC (rev 233600)
+++ trunk/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp 2018-07-06 23:26:09 UTC (rev 233601)
@@ -73,11 +73,14 @@
uint64_t WebResourceLoader::messageSenderDestinationID()
{
+ RELEASE_ASSERT(RunLoop::isMain());
+ RELEASE_ASSERT(m_coreLoader->identifier());
return m_coreLoader->identifier();
}
void WebResourceLoader::detachFromCoreLoader()
{
+ RELEASE_ASSERT(RunLoop::isMain());
m_coreLoader = nullptr;
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
