- Revision
- 180927
- Author
- carlo...@webkit.org
- Date
- 2015-03-03 01:17:24 -0800 (Tue, 03 Mar 2015)
Log Message
[SOUP] Synchronous XMLHttpRequests can time out when we reach the max connections limit
https://bugs.webkit.org/show_bug.cgi?id=141508
Reviewed by Sergio Villar Senin.
Source/WebCore:
Use SOUP_MESSAGE_IGNORE_CONNECTION_LIMITS flag when loading a
synchronous message instead of increasing the maximum number of
connections allowed if the soup version is recent enough.
The current solution of increasing/decreasing the limits doesn't
always work, because connections are not marked as IDLE in libsoup
until the message is unqueued, but we don't wait for the message
to be unqueued to finish our loads in WebKit, we finish them as
soon as we have finished reading the stream. This causes that
synchronous loads keep blocked in the nested main loop until the
timeout of 10 seconds is fired and the load fails.
Also marked WebCoreSynchronousLoader class as final, the virtual
methods as override and removed the unsused method isSynchronousClient.
* platform/network/soup/ResourceHandleSoup.cpp:
(WebCore::createSoupMessageForHandleAndRequest):
(WebCore::WebCoreSynchronousLoader::WebCoreSynchronousLoader):
(WebCore::WebCoreSynchronousLoader::isSynchronousClient): Deleted.
(WebCore::WebCoreSynchronousLoader::didReceiveResponse):
(WebCore::WebCoreSynchronousLoader::didReceiveData):
(WebCore::WebCoreSynchronousLoader::didReceiveBuffer):
(WebCore::WebCoreSynchronousLoader::didFinishLoading):
(WebCore::WebCoreSynchronousLoader::didFail):
(WebCore::WebCoreSynchronousLoader::didReceiveAuthenticationChallenge):
(WebCore::WebCoreSynchronousLoader::shouldUseCredentialStorage):
Tools:
Add a unit test to check that synchronous XHRs load even if the
maximum connection limits are reached.
* TestWebKitAPI/Tests/WebKit2Gtk/TestResources.cpp:
(testWebViewSyncRequestOnMaxConns):
(serverCallback):
(beforeAll):
* gtk/jhbuild.modules: Bump libsoup version to 2.49.91.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (180926 => 180927)
--- trunk/Source/WebCore/ChangeLog 2015-03-03 08:55:14 UTC (rev 180926)
+++ trunk/Source/WebCore/ChangeLog 2015-03-03 09:17:24 UTC (rev 180927)
@@ -1,3 +1,35 @@
+2015-03-03 Carlos Garcia Campos <cgar...@igalia.com>
+
+ [SOUP] Synchronous XMLHttpRequests can time out when we reach the max connections limit
+ https://bugs.webkit.org/show_bug.cgi?id=141508
+
+ Reviewed by Sergio Villar Senin.
+
+ Use SOUP_MESSAGE_IGNORE_CONNECTION_LIMITS flag when loading a
+ synchronous message instead of increasing the maximum number of
+ connections allowed if the soup version is recent enough.
+ The current solution of increasing/decreasing the limits doesn't
+ always work, because connections are not marked as IDLE in libsoup
+ until the message is unqueued, but we don't wait for the message
+ to be unqueued to finish our loads in WebKit, we finish them as
+ soon as we have finished reading the stream. This causes that
+ synchronous loads keep blocked in the nested main loop until the
+ timeout of 10 seconds is fired and the load fails.
+ Also marked WebCoreSynchronousLoader class as final, the virtual
+ methods as override and removed the unsused method isSynchronousClient.
+
+ * platform/network/soup/ResourceHandleSoup.cpp:
+ (WebCore::createSoupMessageForHandleAndRequest):
+ (WebCore::WebCoreSynchronousLoader::WebCoreSynchronousLoader):
+ (WebCore::WebCoreSynchronousLoader::isSynchronousClient): Deleted.
+ (WebCore::WebCoreSynchronousLoader::didReceiveResponse):
+ (WebCore::WebCoreSynchronousLoader::didReceiveData):
+ (WebCore::WebCoreSynchronousLoader::didReceiveBuffer):
+ (WebCore::WebCoreSynchronousLoader::didFinishLoading):
+ (WebCore::WebCoreSynchronousLoader::didFail):
+ (WebCore::WebCoreSynchronousLoader::didReceiveAuthenticationChallenge):
+ (WebCore::WebCoreSynchronousLoader::shouldUseCredentialStorage):
+
2015-03-02 David Kilzer <ddkil...@apple.com>
[iOS] Disable -Wdeprecated-declaration warnings in WebVideoFullscreenInterfaceAVKit.mm
Modified: trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp (180926 => 180927)
--- trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp 2015-03-03 08:55:14 UTC (rev 180926)
+++ trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp 2015-03-03 09:17:24 UTC (rev 180927)
@@ -75,7 +75,7 @@
static bool loadingSynchronousRequest = false;
static const size_t gDefaultReadBufferSize = 8192;
-class WebCoreSynchronousLoader : public ResourceHandleClient {
+class WebCoreSynchronousLoader final : public ResourceHandleClient {
WTF_MAKE_NONCOPYABLE(WebCoreSynchronousLoader);
public:
@@ -86,7 +86,6 @@
, m_data(data)
, m_finished(false)
, m_storedCredentials(storedCredentials)
-
{
// We don't want any timers to fire while we are doing our synchronous load
// so we replace the thread default main context. The main loop iterations
@@ -96,12 +95,16 @@
g_main_context_push_thread_default(innerMainContext.get());
m_mainLoop = adoptGRef(g_main_loop_new(innerMainContext.get(), false));
+#if !SOUP_CHECK_VERSION(2, 49, 91)
adjustMaxConnections(1);
+#endif
}
~WebCoreSynchronousLoader()
{
+#if !SOUP_CHECK_VERSION(2, 49, 91)
adjustMaxConnections(-1);
+#endif
GMainContext* context = g_main_context_get_thread_default();
while (g_main_context_pending(context))
@@ -111,6 +114,7 @@
loadingSynchronousRequest = false;
}
+#if !SOUP_CHECK_VERSION(2, 49, 91)
void adjustMaxConnections(int adjustment)
{
int maxConnections, maxConnectionsPerHost;
@@ -126,23 +130,19 @@
NULL);
}
+#endif // SOUP_CHECK_VERSION(2, 49, 91)
- virtual bool isSynchronousClient()
+ virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse& response) override
{
- return true;
- }
-
- virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse& response)
- {
m_response = response;
}
- virtual void didReceiveData(ResourceHandle*, const char* /* data */, unsigned /* length */, int)
+ virtual void didReceiveData(ResourceHandle*, const char* /* data */, unsigned /* length */, int) override
{
ASSERT_NOT_REACHED();
}
- virtual void didReceiveBuffer(ResourceHandle*, PassRefPtr<SharedBuffer> buffer, int /* encodedLength */)
+ virtual void didReceiveBuffer(ResourceHandle*, PassRefPtr<SharedBuffer> buffer, int /* encodedLength */) override
{
// This pattern is suggested by SharedBuffer.h.
const char* segment;
@@ -153,26 +153,26 @@
}
}
- virtual void didFinishLoading(ResourceHandle*, double)
+ virtual void didFinishLoading(ResourceHandle*, double) override
{
if (g_main_loop_is_running(m_mainLoop.get()))
g_main_loop_quit(m_mainLoop.get());
m_finished = true;
}
- virtual void didFail(ResourceHandle* handle, const ResourceError& error)
+ virtual void didFail(ResourceHandle* handle, const ResourceError& error) override
{
m_error = error;
didFinishLoading(handle, 0);
}
- virtual void didReceiveAuthenticationChallenge(ResourceHandle*, const AuthenticationChallenge& challenge)
+ virtual void didReceiveAuthenticationChallenge(ResourceHandle*, const AuthenticationChallenge& challenge) override
{
// We do not handle authentication for synchronous XMLHttpRequests.
challenge.authenticationClient()->receivedRequestToContinueWithoutCredential(challenge);
}
- virtual bool shouldUseCredentialStorage(ResourceHandle*)
+ virtual bool shouldUseCredentialStorage(ResourceHandle*) override
{
return m_storedCredentials == AllowStoredCredentials;
}
@@ -930,7 +930,14 @@
g_signal_connect(d->m_soupMessage.get(), "got-headers", G_CALLBACK(gotHeadersCallback), handle);
g_signal_connect(d->m_soupMessage.get(), "wrote-body-data", G_CALLBACK(wroteBodyDataCallback), handle);
- soup_message_set_flags(d->m_soupMessage.get(), static_cast<SoupMessageFlags>(soup_message_get_flags(d->m_soupMessage.get()) | SOUP_MESSAGE_NO_REDIRECT));
+ unsigned flags = SOUP_MESSAGE_NO_REDIRECT;
+#if SOUP_CHECK_VERSION(2, 49, 91)
+ // Ignore the connection limits in synchronous loads to avoid freezing the networking process.
+ // See https://bugs.webkit.org/show_bug.cgi?id=141508.
+ if (loadingSynchronousRequest)
+ flags |= SOUP_MESSAGE_IGNORE_CONNECTION_LIMITS;
+#endif
+ soup_message_set_flags(d->m_soupMessage.get(), static_cast<SoupMessageFlags>(soup_message_get_flags(d->m_soupMessage.get()) | flags));
#if ENABLE(WEB_TIMING)
g_signal_connect(d->m_soupMessage.get(), "network-event", G_CALLBACK(networkEventCallback), handle);
Modified: trunk/Tools/ChangeLog (180926 => 180927)
--- trunk/Tools/ChangeLog 2015-03-03 08:55:14 UTC (rev 180926)
+++ trunk/Tools/ChangeLog 2015-03-03 09:17:24 UTC (rev 180927)
@@ -1,3 +1,19 @@
+2015-03-03 Carlos Garcia Campos <cgar...@igalia.com>
+
+ [SOUP] Synchronous XMLHttpRequests can time out when we reach the max connections limit
+ https://bugs.webkit.org/show_bug.cgi?id=141508
+
+ Reviewed by Sergio Villar Senin.
+
+ Add a unit test to check that synchronous XHRs load even if the
+ maximum connection limits are reached.
+
+ * TestWebKitAPI/Tests/WebKit2Gtk/TestResources.cpp:
+ (testWebViewSyncRequestOnMaxConns):
+ (serverCallback):
+ (beforeAll):
+ * gtk/jhbuild.modules: Bump libsoup version to 2.49.91.
+
2015-03-02 Alexey Proskuryakov <a...@apple.com>
Update the name of ASan build step.
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestResources.cpp (180926 => 180927)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestResources.cpp 2015-03-03 08:55:14 UTC (rev 180926)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestResources.cpp 2015-03-03 09:17:24 UTC (rev 180927)
@@ -22,6 +22,8 @@
#include "WebKitTestServer.h"
#include "WebViewTest.h"
#include <wtf/Vector.h>
+#include <wtf/gobject/GMainLoopSource.h>
+#include <wtf/gobject/GMutexLocker.h>
#include <wtf/gobject/GRefPtr.h>
static WebKitTestServer* kServer;
@@ -667,6 +669,49 @@
events.clear();
}
+static GMutex s_serverMutex;
+static const unsigned s_maxConnectionsPerHost = 6;
+
+class SyncRequestOnMaxConnsTest: public ResourcesTest {
+public:
+ MAKE_GLIB_TEST_FIXTURE(SyncRequestOnMaxConnsTest);
+
+ void resourceLoadStarted(WebKitWebResource*, WebKitURIRequest*) override
+ {
+ if (!m_resourcesToStartPending)
+ return;
+
+ if (!--m_resourcesToStartPending)
+ g_main_loop_quit(m_mainLoop);
+ }
+
+ void waitUntilResourcesStarted(unsigned requestCount)
+ {
+ m_resourcesToStartPending = requestCount;
+ g_main_loop_run(m_mainLoop);
+ }
+
+ unsigned m_resourcesToStartPending;
+};
+
+static void testWebViewSyncRequestOnMaxConns(SyncRequestOnMaxConnsTest* test, gconstpointer)
+{
+ WTF::GMutexLocker<GMutex> lock(s_serverMutex);
+ test->loadURI(kServer->getURIForPath("/sync-request-on-max-conns-0").data());
+ test->waitUntilResourcesStarted(s_maxConnectionsPerHost + 1); // s_maxConnectionsPerHost resource + main resource.
+
+ for (unsigned i = 0; i < 2; ++i) {
+ GUniquePtr<char> xhr(g_strdup_printf("xhr = new XMLHttpRequest; xhr.open('GET', '/sync-request-on-max-conns-xhr%u', false); xhr.send();", i));
+ webkit_web_view_run_javascript(test->m_webView, xhr.get(), nullptr, nullptr, nullptr);
+ }
+
+ // By default sync XHRs have a 10 seconds timeout, we don't want to wait all that so use our own timeout.
+ GMainLoopSource::scheduleAfterDelayAndDeleteOnDestroy("Timeout", [] { g_assert_not_reached(); }, std::chrono::seconds(1));
+
+ GMainLoopSource::scheduleAndDeleteOnDestroy("Unlock Server Idle", [&lock] { lock.unlock(); });
+ test->waitUntilResourcesLoaded(s_maxConnectionsPerHost + 3); // s_maxConnectionsPerHost resource + main resource + 2 XHR.
+}
+
static void addCacheHTTPHeadersToResponse(SoupMessage* message)
{
// The actual date doesn't really matter.
@@ -760,7 +805,28 @@
soup_message_headers_append(message->response_headers, "Location", "/cancel-this.js");
} else if (g_str_equal(path, "/invalid.css"))
soup_message_set_status(message, SOUP_STATUS_CANT_CONNECT);
- else
+ else if (g_str_has_prefix(path, "/sync-request-on-max-conns-")) {
+ char* contents;
+ gsize contentsLength;
+ if (g_str_equal(path, "/sync-request-on-max-conns-0")) {
+ GString* imagesHTML = g_string_new("<html><body>");
+ for (unsigned i = 1; i <= s_maxConnectionsPerHost; ++i)
+ g_string_append_printf(imagesHTML, "<img src=''>", i);
+ g_string_append(imagesHTML, "</body></html>");
+
+ contentsLength = imagesHTML->len;
+ contents = g_string_free(imagesHTML, FALSE);
+ } else {
+ {
+ // We don't actually need to keep the mutex, so we release it as soon as we get it.
+ WTF::GMutexLocker<GMutex> lock(s_serverMutex);
+ }
+
+ GUniquePtr<char> filePath(g_build_filename(Test::getResourcesDir().data(), "blank.ico", nullptr));
+ g_file_get_contents(filePath.get(), &contents, &contentsLength, 0);
+ }
+ soup_message_body_append(message->response_body, SOUP_MEMORY_TAKE, contents, contentsLength);
+ } else
soup_message_set_status(message, SOUP_STATUS_NOT_FOUND);
soup_message_body_complete(message->response_body);
}
@@ -779,6 +845,9 @@
ResourcesTest::add("WebKitWebResource", "get-data", testWebResourceGetData);
SingleResourceLoadTest::add("WebKitWebView", "history-cache", testWebViewResourcesHistoryCache);
SendRequestTest::add("WebKitWebPage", "send-request", testWebResourceSendRequest);
+#if SOUP_CHECK_VERSION(2, 49, 91)
+ SyncRequestOnMaxConnsTest::add("WebKitWebView", "sync-request-on-max-conns", testWebViewSyncRequestOnMaxConns);
+#endif
}
void afterAll()
Modified: trunk/Tools/gtk/jhbuild.modules (180926 => 180927)
--- trunk/Tools/gtk/jhbuild.modules 2015-03-03 08:55:14 UTC (rev 180926)
+++ trunk/Tools/gtk/jhbuild.modules 2015-03-03 09:17:24 UTC (rev 180927)
@@ -182,9 +182,9 @@
<dependencies>
<dep package="glib-networking"/>
</dependencies>
- <branch module="libsoup" version="2.43.90"
+ <branch module="libsoup" version="2.49.91"
repo="git.gnome.org"
- tag="0ea86f566b7d526c8328c7c602ae1be8cda8dd68"/>
+ tag="933de304ffde0257b18f04bf34a653fcc356833c"/>
</autotools>
<autotools id="fontconfig"
