[soup] Crash while loading http://www.jusco.cn
authormrobinson@webkit.org <mrobinson@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Mar 2012 18:15:32 +0000 (18:15 +0000)
committermrobinson@webkit.org <mrobinson@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Mar 2012 18:15:32 +0000 (18:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=68238

Reviewed by Philippe Normand.

.:

* configure.ac: Bumped the libsoup dependency to 2.37.90.

Source/WebCore:

Test: http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html

When running synchronous XMLHttpRequests, push a new inner thread default
context, so that other sources from timers and network activity do not run.
This will make synchronous requests truly synchronous with the rest of
WebCore.

* platform/network/soup/ResourceHandleSoup.cpp:
(WebCoreSynchronousLoader): Clean up the method definitions a bit by writing them inline.
(WebCore::WebCoreSynchronousLoader::WebCoreSynchronousLoader): Push a new thread default
context to prevent other sources from running.
(WebCore::WebCoreSynchronousLoader::~WebCoreSynchronousLoader): Pop the inner thread default context.
(WebCore::closeCallback): If the client is synchronous call didFinishLoading now.
(WebCore::readCallback): Only call didFinishLoading if the client isn't synchronous.
(WebCore::ResourceHandle::defaultSession): Activate use-thread-context so that the soup session
respects the inner thread context.
(ResourceHandleClient):
(WebCore::ResourceHandleClient::isSynchronousClient): Added this virtual method.

Tools:

* gtk/jhbuild.modules: Bumped the libsoup and glib dependencies
in the jhbuild file.

LayoutTests:

* http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt: Added.
* http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@109760 268f45cc-cd09-0410-ab3c-d52691b4dbfc

ChangeLog
LayoutTests/ChangeLog
LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/network/ResourceHandleClient.h
Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp
Tools/ChangeLog
Tools/gtk/jhbuild.modules
configure.ac

index 9c34888..290e422 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2012-03-05  Martin Robinson  <mrobinson@igalia.com>
+
+        [soup] Crash while loading http://www.jusco.cn
+        https://bugs.webkit.org/show_bug.cgi?id=68238
+
+        Reviewed by Philippe Normand.
+
+        * configure.ac: Bumped the libsoup dependency to 2.37.90.
+
 2012-03-04  Raphael Kubo da Costa  <kubo@profusion.mobi>
 
         [CMake] Libraries are installed to /usr/lib and not /usr/lib64 on x86_64
index 2ac1a82..beaf97e 100644 (file)
@@ -1,3 +1,13 @@
+2012-03-05  Martin Robinson  <mrobinson@igalia.com>
+
+        [soup] Crash while loading http://www.jusco.cn
+        https://bugs.webkit.org/show_bug.cgi?id=68238
+
+        Reviewed by Philippe Normand.
+
+        * http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt: Added.
+        * http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html: Added.
+
 2012-03-05  Zan Dobersek  <zandobersek@gmail.com>
 
         [GTK] plugins/netscape-plugin-page-cache-works.html fails
diff --git a/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt b/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt
new file mode 100644 (file)
index 0000000..bb78da0
--- /dev/null
@@ -0,0 +1,4 @@
+Test for: bug 68238: [soup] Crash while loading http://www.jusco.cn This test verifies that WebCore timers do not fire during synchronous XMLHttpRequests.
+
+PASS
+
diff --git a/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html b/LayoutTests/http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html
new file mode 100644 (file)
index 0000000..916a4a7
--- /dev/null
@@ -0,0 +1,32 @@
+<html><body>
+
+<p> Test for: <a href="https://bugs.webkit.org/show_bug.cgi?id=68238">bug 68238<a>: [soup] Crash while loading http://www.jusco.cn</a> This test verifies that WebCore timers do not fire during synchronous XMLHttpRequests.
+<pre id=log></pre>
+
+<script type="text/javascript">
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+function log(message)
+{
+    document.getElementById("log").innerHTML += message + "\n";
+}
+
+var timerEverFired = false;
+var intervalId = setInterval(function() {
+    timerEverFired = true;
+}, 10);
+
+try {
+    var req = new XMLHttpRequest();
+    req.open("GET", "resources/download-with-delay.php?iteration=5&delay=50", false);
+    req.send(null);
+} catch (ex) {
+    log(ex);
+}
+
+clearInterval(intervalId);
+log(timerEverFired ? "FAIL" : "PASS");
+</script>
+
+</body></html>
index 839680a..93381e9 100644 (file)
@@ -1,3 +1,29 @@
+2012-03-05  Martin Robinson  <mrobinson@igalia.com>
+
+        [soup] Crash while loading http://www.jusco.cn
+        https://bugs.webkit.org/show_bug.cgi?id=68238
+
+        Reviewed by Philippe Normand.
+
+        Test: http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html
+
+        When running synchronous XMLHttpRequests, push a new inner thread default
+        context, so that other sources from timers and network activity do not run.
+        This will make synchronous requests truly synchronous with the rest of
+        WebCore.
+
+        * platform/network/soup/ResourceHandleSoup.cpp:
+        (WebCoreSynchronousLoader): Clean up the method definitions a bit by writing them inline.
+        (WebCore::WebCoreSynchronousLoader::WebCoreSynchronousLoader): Push a new thread default
+        context to prevent other sources from running.
+        (WebCore::WebCoreSynchronousLoader::~WebCoreSynchronousLoader): Pop the inner thread default context.
+        (WebCore::closeCallback): If the client is synchronous call didFinishLoading now.
+        (WebCore::readCallback): Only call didFinishLoading if the client isn't synchronous.
+        (WebCore::ResourceHandle::defaultSession): Activate use-thread-context so that the soup session
+        respects the inner thread context.
+        (ResourceHandleClient):
+        (WebCore::ResourceHandleClient::isSynchronousClient): Added this virtual method.
+
 2012-03-05  Alexander Færøy  <alexander.faeroy@nokia.com>
 
         Remove pointer to timer and use the timers directly in the tiled backing store
index befbccf..d38f48c 100644 (file)
@@ -109,6 +109,9 @@ namespace WebCore {
 #if ENABLE(BLOB)
         virtual AsyncFileStream* createAsyncFileStream(FileStreamClient*) { return 0; }
 #endif
+#if USE(SOUP)
+        virtual bool isSynchronousClient() { return false; }
+#endif
     };
 
 }
index 92c66e5..74907ca 100644 (file)
@@ -72,15 +72,59 @@ namespace WebCore {
 class WebCoreSynchronousLoader : public ResourceHandleClient {
     WTF_MAKE_NONCOPYABLE(WebCoreSynchronousLoader);
 public:
-    WebCoreSynchronousLoader(ResourceError&, ResourceResponse &, Vector<char>&);
-    ~WebCoreSynchronousLoader();
 
-    virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse&);
-    virtual void didReceiveData(ResourceHandle*, const char*, int, int encodedDataLength);
-    virtual void didFinishLoading(ResourceHandle*, double /*finishTime*/);
-    virtual void didFail(ResourceHandle*, const ResourceError&);
+    WebCoreSynchronousLoader(ResourceError& error, ResourceResponse& response, Vector<char>& data)
+        : m_error(error)
+        , m_response(response)
+        , m_data(data)
+        , m_finished(false)
+    {
+        // 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
+        // will only process GSources associated with this inner context.
+        GRefPtr<GMainContext> innerMainContext = adoptGRef(g_main_context_new());
+        g_main_context_push_thread_default(innerMainContext.get());
+        m_mainLoop = g_main_loop_new(innerMainContext.get(), false);
+    }
+
+    ~WebCoreSynchronousLoader()
+    {
+        g_main_context_pop_thread_default(g_main_context_get_thread_default());
+    }
+
+    virtual bool isSynchronousClient()
+    {
+        return true;
+    }
 
-    void run();
+    virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse& response)
+    {
+        m_response = response;
+    }
+
+    virtual void didReceiveData(ResourceHandle*, const char* data, int length, int)
+    {
+        m_data.append(data, length);
+    }
+
+    virtual void didFinishLoading(ResourceHandle*, double)
+    {
+        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)
+    {
+        m_error = error;
+        didFinishLoading(handle, 0);
+    }
+
+    void run()
+    {
+        if (!m_finished)
+            g_main_loop_run(m_mainLoop.get());
+    }
 
 private:
     ResourceError& m_error;
@@ -90,47 +134,6 @@ private:
     GRefPtr<GMainLoop> m_mainLoop;
 };
 
-WebCoreSynchronousLoader::WebCoreSynchronousLoader(ResourceError& error, ResourceResponse& response, Vector<char>& data)
-    : m_error(error)
-    , m_response(response)
-    , m_data(data)
-    , m_finished(false)
-{
-    m_mainLoop = adoptGRef(g_main_loop_new(0, false));
-}
-
-WebCoreSynchronousLoader::~WebCoreSynchronousLoader()
-{
-}
-
-void WebCoreSynchronousLoader::didReceiveResponse(ResourceHandle*, const ResourceResponse& response)
-{
-    m_response = response;
-}
-
-void WebCoreSynchronousLoader::didReceiveData(ResourceHandle*, const char* data, int length, int)
-{
-    m_data.append(data, length);
-}
-
-void WebCoreSynchronousLoader::didFinishLoading(ResourceHandle*, double)
-{
-    g_main_loop_quit(m_mainLoop.get());
-    m_finished = true;
-}
-
-void WebCoreSynchronousLoader::didFail(ResourceHandle* handle, const ResourceError& error)
-{
-    m_error = error;
-    didFinishLoading(handle, 0);
-}
-
-void WebCoreSynchronousLoader::run()
-{
-    if (!m_finished)
-        g_main_loop_run(m_mainLoop.get());
-}
-
 static void cleanupSoupRequestOperation(ResourceHandle*, bool isDestroying);
 static void sendRequestCallback(GObject*, GAsyncResult*, gpointer);
 static void readCallback(GObject*, GAsyncResult*, gpointer);
@@ -637,6 +640,9 @@ static void closeCallback(GObject* source, GAsyncResult* res, gpointer data)
     ResourceHandleInternal* d = handle->getInternal();
     g_input_stream_close_finish(d->m_inputStream.get(), res, 0);
     cleanupSoupRequestOperation(handle.get());
+
+    if (handle->client()->isSynchronousClient())
+        handle->client()->didFinishLoading(handle.get(), 0);
 }
 
 static void readCallback(GObject* source, GAsyncResult* asyncResult, gpointer data)
@@ -666,8 +672,11 @@ static void readCallback(GObject* source, GAsyncResult* asyncResult, gpointer da
 
     if (!bytesRead) {
         // We inform WebCore of load completion now instead of waiting for the input
-        // stream to close because the input stream is closed asynchronously.
-        client->didFinishLoading(handle.get(), 0);
+        // stream to close because the input stream is closed asynchronously. If this
+        // is a synchronous request, we wait until the closeCallback, because we don't
+        // want to halt the internal main loop before the input stream closes.
+        if (!handle->client()->isSynchronousClient())
+            client->didFinishLoading(handle.get(), 0);
         g_input_stream_close_async(d->m_inputStream.get(), G_PRIORITY_DEFAULT, 0, closeCallback, handle.get());
         return;
     }
@@ -739,6 +748,7 @@ SoupSession* ResourceHandle::defaultSession()
                      SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_CONTENT_DECODER,
                      SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_CONTENT_SNIFFER,
                      SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_PROXY_RESOLVER_DEFAULT,
+                     SOUP_SESSION_USE_THREAD_CONTEXT, TRUE,
                      NULL);
     }
 
index 9198e69..05c831b 100644 (file)
@@ -1,3 +1,13 @@
+2012-03-05  Martin Robinson  <mrobinson@igalia.com>
+
+        [soup] Crash while loading http://www.jusco.cn
+        https://bugs.webkit.org/show_bug.cgi?id=68238
+
+        Reviewed by Philippe Normand.
+
+        * gtk/jhbuild.modules: Bumped the libsoup and glib dependencies
+        in the jhbuild file.
+
 2012-03-05  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r109748.
index e0c9e38..44f9ed2 100644 (file)
     <dependencies>
       <dep package="libffi"/>
     </dependencies>
-    <branch module="/pub/GNOME/sources/glib/2.31/glib-2.31.2.tar.xz" version="2.31.2"
+    <branch module="/pub/GNOME/sources/glib/2.31/glib-2.31.8.tar.xz" version="2.31.8"
             repo="ftp.gnome.org"
-            hash="sha256:19d7921671a487c3c5759a57df7b8508afdbadd7764d62a47a82fff7b399032b"
-            md5sum="1cbdf314d7c87916a0c3dce83ac0285f"/>
+            hash="sha256:1ce3d275189000e1c50e92efcdb6447bc260b1e5c41699b7a1959e3e1928fbaa"/>
   </autotools>
 
   <autotools id="glib-networking">
     <dependencies>
       <dep package="glib-networking"/>
     </dependencies>
-    <branch module="libsoup" version="2.37.2.1+git"
+    <branch module="libsoup" version="2.37.91+git"
             repo="git.gnome.org"
-            tag="5cbfc48caf76ced2e28ee06c9e40523273601dc6"/>
+            tag="52057510accba49cfc6d1d0e52292368ba2e0c99"/>
   </autotools>
 
   <autotools id="fontconfig" autogen-sh="configure">
index 054e1d6..700015b 100644 (file)
@@ -378,7 +378,7 @@ CAIRO_REQUIRED_VERSION=1.10
 FONTCONFIG_REQUIRED_VERSION=2.4
 FREETYPE2_REQUIRED_VERSION=9.0
 GLIB_REQUIRED_VERSION=2.31.2
-LIBSOUP_REQUIRED_VERSION=2.37.2.1
+LIBSOUP_REQUIRED_VERSION=2.37.90
 LIBXML_REQUIRED_VERSION=2.6
 PANGO_REQUIRED_VERSION=1.21.0