- Revision
- 137783
- Author
- a...@apple.com
- Date
- 2012-12-14 15:33:45 -0800 (Fri, 14 Dec 2012)
Log Message
<rdar://problem/12874760> NetworkProcess loads may get stuck when WebProcess quits
https://bugs.webkit.org/show_bug.cgi?id=105056
Reviewed by Anders Carlsson.
Make response maps per-connection.
* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::didClose): Cancel waiting for responses
from WebProcess, they will never arrive.
* NetworkProcess/NetworkConnectionToWebProcess.h:
(WebKit::NetworkConnectionToWebProcess::willSendRequestResponseMap):
(WebKit::NetworkConnectionToWebProcess::canAuthenticateAgainstProtectionSpaceResponseMap):
Maps now live here.
* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::connectionToWebProcessDidClose): Added a FIXME.
(WebKit::NetworkResourceLoader::willSendRequest):
(WebKit::NetworkResourceLoader::willSendRequestHandled):
(WebKit::NetworkResourceLoader::canAuthenticateAgainstProtectionSpace):
(WebKit::NetworkResourceLoader::canAuthenticateAgainstProtectionSpaceHandled):
Handle the cases where we can't send a request, or can't expect a response any more.
* Shared/BlockingResponseMap.h:
(BlockingResponseMap):
(BlockingResponseMap::BlockingResponseMap):
(BlockingResponseMap::~BlockingResponseMap):
(BlockingResponseMap::waitForResponse):
(BlockingResponseMap::didReceiveResponse):
(BlockingResponseMap::cancel):
(BlockingBoolResponseMap):
(BlockingBoolResponseMap::BlockingBoolResponseMap):
(BlockingBoolResponseMap::~BlockingBoolResponseMap):
(BlockingBoolResponseMap::waitForResponse):
(BlockingBoolResponseMap::didReceiveResponse):
(BlockingBoolResponseMap::cancel):
Added an ability to cancel, and slightly beefed up overall.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (137782 => 137783)
--- trunk/Source/WebKit2/ChangeLog 2012-12-14 23:23:25 UTC (rev 137782)
+++ trunk/Source/WebKit2/ChangeLog 2012-12-14 23:33:45 UTC (rev 137783)
@@ -1,3 +1,45 @@
+2012-12-14 Alexey Proskuryakov <a...@apple.com>
+
+ <rdar://problem/12874760> NetworkProcess loads may get stuck when WebProcess quits
+ https://bugs.webkit.org/show_bug.cgi?id=105056
+
+ Reviewed by Anders Carlsson.
+
+ Make response maps per-connection.
+
+ * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+ (WebKit::NetworkConnectionToWebProcess::didClose): Cancel waiting for responses
+ from WebProcess, they will never arrive.
+
+ * NetworkProcess/NetworkConnectionToWebProcess.h:
+ (WebKit::NetworkConnectionToWebProcess::willSendRequestResponseMap):
+ (WebKit::NetworkConnectionToWebProcess::canAuthenticateAgainstProtectionSpaceResponseMap):
+ Maps now live here.
+
+ * NetworkProcess/NetworkResourceLoader.cpp:
+ (WebKit::NetworkResourceLoader::connectionToWebProcessDidClose): Added a FIXME.
+
+ (WebKit::NetworkResourceLoader::willSendRequest):
+ (WebKit::NetworkResourceLoader::willSendRequestHandled):
+ (WebKit::NetworkResourceLoader::canAuthenticateAgainstProtectionSpace):
+ (WebKit::NetworkResourceLoader::canAuthenticateAgainstProtectionSpaceHandled):
+ Handle the cases where we can't send a request, or can't expect a response any more.
+
+ * Shared/BlockingResponseMap.h:
+ (BlockingResponseMap):
+ (BlockingResponseMap::BlockingResponseMap):
+ (BlockingResponseMap::~BlockingResponseMap):
+ (BlockingResponseMap::waitForResponse):
+ (BlockingResponseMap::didReceiveResponse):
+ (BlockingResponseMap::cancel):
+ (BlockingBoolResponseMap):
+ (BlockingBoolResponseMap::BlockingBoolResponseMap):
+ (BlockingBoolResponseMap::~BlockingBoolResponseMap):
+ (BlockingBoolResponseMap::waitForResponse):
+ (BlockingBoolResponseMap::didReceiveResponse):
+ (BlockingBoolResponseMap::cancel):
+ Added an ability to cancel, and slightly beefed up overall.
+
2012-12-14 Anders Carlsson <ander...@apple.com>
DownloadProxyMap should keep track of outstanding DownloadProxy objects
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp (137782 => 137783)
--- trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp 2012-12-14 23:23:25 UTC (rev 137782)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp 2012-12-14 23:33:45 UTC (rev 137783)
@@ -102,10 +102,11 @@
RefPtr<NetworkConnectionToWebProcess> protector(this);
NetworkProcess::shared().removeNetworkConnectionToWebProcess(this);
-
- // FIXME (NetworkProcess): We might consider actively clearing out all requests for this connection.
- // But that might not be necessary as the observer mechanism used above is much more direct.
+ // Unblock waiting threads.
+ m_willSendRequestResponseMap.cancel();
+ m_canAuthenticateAgainstProtectionSpaceResponseMap.cancel();
+
Vector<NetworkConnectionToWebProcessObserver*> observers;
copyToVector(m_observers, observers);
for (size_t i = 0; i < observers.size(); ++i)
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h (137782 => 137783)
--- trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h 2012-12-14 23:23:25 UTC (rev 137782)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.h 2012-12-14 23:33:45 UTC (rev 137783)
@@ -28,6 +28,7 @@
#if ENABLE(NETWORK_PROCESS)
+#include "BlockingResponseMap.h"
#include "Connection.h"
#include "NetworkConnectionToWebProcessMessages.h"
#include <WebCore/ResourceLoadPriority.h>
@@ -61,6 +62,9 @@
bool isSerialLoadingEnabled() const { return m_serialLoadingEnabled; }
+ BlockingResponseMap<WebCore::ResourceRequest*>& willSendRequestResponseMap() { return m_willSendRequestResponseMap; }
+ BlockingBoolResponseMap& canAuthenticateAgainstProtectionSpaceResponseMap() { return m_canAuthenticateAgainstProtectionSpaceResponseMap; }
+
private:
NetworkConnectionToWebProcess(CoreIPC::Connection::Identifier);
@@ -95,7 +99,10 @@
RefPtr<CoreIPC::Connection> m_connection;
HashSet<NetworkConnectionToWebProcessObserver*> m_observers;
-
+
+ BlockingResponseMap<WebCore::ResourceRequest*> m_willSendRequestResponseMap;
+ BlockingBoolResponseMap m_canAuthenticateAgainstProtectionSpaceResponseMap;
+
bool m_serialLoadingEnabled;
};
Modified: trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp (137782 => 137783)
--- trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2012-12-14 23:23:25 UTC (rev 137782)
+++ trunk/Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp 2012-12-14 23:33:45 UTC (rev 137783)
@@ -28,7 +28,6 @@
#if ENABLE(NETWORK_PROCESS)
-#include "BlockingResponseMap.h"
#include "DataReference.h"
#include "Logging.h"
#include "NetworkConnectionToWebProcess.h"
@@ -144,6 +143,7 @@
void NetworkResourceLoader::connectionToWebProcessDidClose(NetworkConnectionToWebProcess* connection)
{
ASSERT_ARG(connection, connection == m_connection.get());
+ // FIXME (NetworkProcess): Cancel the load. The request may be long-living, so we don't want it to linger around after all clients are gone.
}
void NetworkResourceLoader::didReceiveResponse(ResourceHandle*, const ResourceResponse& response)
@@ -177,12 +177,6 @@
scheduleStopOnMainThread();
}
-static BlockingResponseMap<ResourceRequest*>& willSendRequestResponseMap()
-{
- AtomicallyInitializedStatic(BlockingResponseMap<ResourceRequest*>&, responseMap = *new BlockingResponseMap<ResourceRequest*>);
- return responseMap;
-}
-
static uint64_t generateWillSendRequestID()
{
static int64_t uniqueWillSendRequestID;
@@ -191,22 +185,25 @@
void NetworkResourceLoader::willSendRequest(ResourceHandle*, ResourceRequest& request, const ResourceResponse& redirectResponse)
{
- // We only expect to get the willSendRequest callback from ResourceHandle as the result of a redirect
+ // We only expect to get the willSendRequest callback from ResourceHandle as the result of a redirect.
ASSERT(!redirectResponse.isNull());
uint64_t requestID = generateWillSendRequestID();
- send(Messages::WebResourceLoader::WillSendRequest(requestID, request, redirectResponse));
-
- OwnPtr<ResourceRequest> newRequest = willSendRequestResponseMap().waitForResponse(requestID);
- request = *newRequest;
+ if (!send(Messages::WebResourceLoader::WillSendRequest(requestID, request, redirectResponse))) {
+ request = ResourceRequest();
+ return;
+ }
+ OwnPtr<ResourceRequest> newRequest = m_connection->willSendRequestResponseMap().waitForResponse(requestID);
+ request = newRequest ? *newRequest : ResourceRequest();
+
RunLoop::main()->dispatch(WTF::bind(&NetworkResourceLoadScheduler::receivedRedirect, &NetworkProcess::shared().networkResourceLoadScheduler(), m_identifier, request.url()));
}
void NetworkResourceLoader::willSendRequestHandled(uint64_t requestID, const WebCore::ResourceRequest& newRequest)
{
- willSendRequestResponseMap().didReceiveResponse(requestID, adoptPtr(new ResourceRequest(newRequest)));
+ m_connection->willSendRequestResponseMap().didReceiveResponse(requestID, adoptPtr(new ResourceRequest(newRequest)));
}
// FIXME (NetworkProcess): Many of the following ResourceHandleClient methods definitely need implementations. A few will not.
@@ -300,12 +297,6 @@
}
#if USE(PROTECTION_SPACE_AUTH_CALLBACK)
-static BlockingBoolResponseMap& canAuthenticateAgainstProtectionSpaceResponseMap()
-{
- AtomicallyInitializedStatic(BlockingBoolResponseMap&, responseMap = *new BlockingBoolResponseMap);
- return responseMap;
-}
-
static uint64_t generateCanAuthenticateAgainstProtectionSpaceID()
{
static int64_t uniqueCanAuthenticateAgainstProtectionSpaceID;
@@ -316,14 +307,15 @@
{
uint64_t requestID = generateCanAuthenticateAgainstProtectionSpaceID();
- send(Messages::WebResourceLoader::CanAuthenticateAgainstProtectionSpace(requestID, protectionSpace));
+ if (!send(Messages::WebResourceLoader::CanAuthenticateAgainstProtectionSpace(requestID, protectionSpace)))
+ return false;
- return canAuthenticateAgainstProtectionSpaceResponseMap().waitForResponse(requestID);
+ return m_connection->canAuthenticateAgainstProtectionSpaceResponseMap().waitForResponse(requestID);
}
void NetworkResourceLoader::canAuthenticateAgainstProtectionSpaceHandled(uint64_t requestID, bool canAuthenticate)
{
- canAuthenticateAgainstProtectionSpaceResponseMap().didReceiveResponse(requestID, canAuthenticate);
+ m_connection->canAuthenticateAgainstProtectionSpaceResponseMap().didReceiveResponse(requestID, canAuthenticate);
}
#endif
Modified: trunk/Source/WebKit2/Shared/BlockingResponseMap.h (137782 => 137783)
--- trunk/Source/WebKit2/Shared/BlockingResponseMap.h 2012-12-14 23:23:25 UTC (rev 137782)
+++ trunk/Source/WebKit2/Shared/BlockingResponseMap.h 2012-12-14 23:33:45 UTC (rev 137783)
@@ -33,12 +33,19 @@
template<typename T>
class BlockingResponseMap {
+WTF_MAKE_NONCOPYABLE(BlockingResponseMap);
public:
+ BlockingResponseMap() : m_canceled(false) { }
+ ~BlockingResponseMap() { ASSERT(m_responses.isEmpty()); }
+
PassOwnPtr<T> waitForResponse(uint64_t requestID)
{
while (true) {
MutexLocker locker(m_mutex);
+ if (m_canceled)
+ return nullptr;
+
if (OwnPtr<T> response = m_responses.take(requestID))
return response.release();
@@ -54,24 +61,41 @@
ASSERT(!m_responses.contains(requestID));
m_responses.set(requestID, response);
- // FIXME (NetworkProcess): Waking up all threads is quite inefficient.
+ // FIXME (NetworkProcess): <rdar://problem/12886430>: Waking up all threads is quite inefficient.
m_condition.broadcast();
}
+ void cancel()
+ {
+ m_canceled = true;
+
+ // FIXME (NetworkProcess): <rdar://problem/12886430>: Waking up all threads is quite inefficient.
+ m_condition.broadcast();
+ }
+
private:
Mutex m_mutex;
ThreadCondition m_condition;
HashMap<uint64_t, OwnPtr<T> > m_responses;
+ bool m_canceled;
};
class BlockingBoolResponseMap {
+WTF_MAKE_NONCOPYABLE(BlockingBoolResponseMap);
public:
+ BlockingBoolResponseMap() : m_canceled(false) { }
+ ~BlockingBoolResponseMap() { ASSERT(m_responses.isEmpty()); }
+
bool waitForResponse(uint64_t requestID)
{
while (true) {
MutexLocker locker(m_mutex);
+ // FIXME: Differentiate between canceled wait and a negative response.
+ if (m_canceled)
+ return false;
+
HashMap<uint64_t, bool>::iterator iter = m_responses.find(requestID);
if (iter != m_responses.end()) {
bool result = iter->value;
@@ -91,15 +115,24 @@
ASSERT(!m_responses.contains(requestID));
m_responses.set(requestID, response);
- // FIXME (NetworkProcess): Waking up all threads is quite inefficient.
+ // FIXME (NetworkProcess): <rdar://problem/12886430>: Waking up all threads is quite inefficient.
m_condition.broadcast();
}
+ void cancel()
+ {
+ m_canceled = true;
+
+ // FIXME (NetworkProcess): <rdar://problem/12886430>: Waking up all threads is quite inefficient.
+ m_condition.broadcast();
+ }
+
private:
Mutex m_mutex;
ThreadCondition m_condition;
HashMap<uint64_t, bool> m_responses;
+ bool m_canceled;
};
#endif // BlockingResponseMap_h
