Changeset 113604 in webkit


Ignore:
Timestamp:
Apr 9, 2012 12:28:42 PM (12 years ago)
Author:
commit-queue@webkit.org
Message:

[soup] Crash while loading ​http://www.jusco.cn
​https://bugs.webkit.org/show_bug.cgi?id=68238

Patch by Martin Robinson <​mrobinson@igalia.com> on 2012-04-09
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.

LayoutTests:

  • http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt: Added.
  • http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/ChangeLog

    r113597 r113604  
     12012-04-09  Martin Robinson  <mrobinson@igalia.com>
     2
     3        [soup] Crash while loading http://www.jusco.cn
     4        https://bugs.webkit.org/show_bug.cgi?id=68238
     5
     6        Reviewed by Philippe Normand.
     7
     8        * configure.ac: Bumped the libsoup dependency to 2.37.90.
     9
    1102012-04-09  Abhishek Arya  <inferno@chromium.org>
    211

  • trunk/LayoutTests/ChangeLog

    r113601 r113604  
     12012-04-09  Martin Robinson  <mrobinson@igalia.com>
     2
     3        [soup] Crash while loading http://www.jusco.cn
     4        https://bugs.webkit.org/show_bug.cgi?id=68238
     5
     6        Reviewed by Philippe Normand.
     7
     8        * http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers-expected.txt: Added.
     9        * http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html: Added.
     10
    1112012-04-09  Martin Robinson  <mrobinson@igalia.com>
    212

  • trunk/Source/WebCore/ChangeLog

    r113602 r113604  
     12012-04-09  Martin Robinson  <mrobinson@igalia.com>
     2
     3        [soup] Crash while loading http://www.jusco.cn
     4        https://bugs.webkit.org/show_bug.cgi?id=68238
     5
     6        Reviewed by Philippe Normand.
     7
     8        Test: http/tests/xmlhttprequest/xmlhttprequest-sync-no-timers.html
     9
     10        When running synchronous XMLHttpRequests, push a new inner thread default
     11        context, so that other sources from timers and network activity do not run.
     12        This will make synchronous requests truly synchronous with the rest of
     13        WebCore.
     14
     15        * platform/network/soup/ResourceHandleSoup.cpp:
     16        (WebCoreSynchronousLoader): Clean up the method definitions a bit by writing them inline.
     17        (WebCore::WebCoreSynchronousLoader::WebCoreSynchronousLoader): Push a new thread default
     18        context to prevent other sources from running.
     19        (WebCore::WebCoreSynchronousLoader::~WebCoreSynchronousLoader): Pop the inner thread default context.
     20        (WebCore::closeCallback): If the client is synchronous call didFinishLoading now.
     21        (WebCore::readCallback): Only call didFinishLoading if the client isn't synchronous.
     22        (WebCore::ResourceHandle::defaultSession): Activate use-thread-context so that the soup session
     23        respects the inner thread context.
     24
    1252012-04-09  Dana Jansens  <danakj@chromium.org>
    226

  • trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp

    r113019 r113604  
    7070#define READ_BUFFER_SIZE 8192
    7171
     72static bool loadingSynchronousRequest = false;
     73
    7274class WebCoreSynchronousLoader : public ResourceHandleClient {
    7375    WTF_MAKE_NONCOPYABLE(WebCoreSynchronousLoader);
    7476public:
    75     WebCoreSynchronousLoader(ResourceError&, ResourceResponse &, Vector<char>&);
    76     ~WebCoreSynchronousLoader();
    77 
    78     virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse&);
    79     virtual void didReceiveData(ResourceHandle*, const char*, int, int encodedDataLength);
    80     virtual void didFinishLoading(ResourceHandle*, double /*finishTime*/);
    81     virtual void didFail(ResourceHandle*, const ResourceError&);
    82 
    83     void run();
     77
     78    WebCoreSynchronousLoader(ResourceError& error, ResourceResponse& response, Vector<char>& data)
     79        : m_error(error)
     80        , m_response(response)
     81        , m_data(data)
     82        , m_finished(false)
     83    {
     84        // We don't want any timers to fire while we are doing our synchronous load
     85        // so we replace the thread default main context. The main loop iterations
     86        // will only process GSources associated with this inner context.
     87        loadingSynchronousRequest = true;
     88        GRefPtr<GMainContext> innerMainContext = adoptGRef(g_main_context_new());
     89        g_main_context_push_thread_default(innerMainContext.get());
     90        m_mainLoop = g_main_loop_new(innerMainContext.get(), false);
     91    }
     92
     93    ~WebCoreSynchronousLoader()
     94    {
     95        g_main_context_pop_thread_default(g_main_context_get_thread_default());
     96        loadingSynchronousRequest = false;
     97    }
     98
     99    virtual bool isSynchronousClient()
     100    {
     101        return true;
     102    }
     103
     104    virtual void didReceiveResponse(ResourceHandle*, const ResourceResponse& response)
     105    {
     106        m_response = response;
     107    }
     108
     109    virtual void didReceiveData(ResourceHandle*, const char* data, int length, int)
     110    {
     111        m_data.append(data, length);
     112    }
     113
     114    virtual void didFinishLoading(ResourceHandle*, double)
     115    {
     116        if (g_main_loop_is_running(m_mainLoop.get()))
     117            g_main_loop_quit(m_mainLoop.get());
     118        m_finished = true;
     119    }
     120
     121    virtual void didFail(ResourceHandle* handle, const ResourceError& error)
     122    {
     123        m_error = error;
     124        didFinishLoading(handle, 0);
     125    }
     126
     127    void run()
     128    {
     129        if (!m_finished)
     130            g_main_loop_run(m_mainLoop.get());
     131    }
    84132
    85133private:
    … …  
    90138    GRefPtr<GMainLoop> m_mainLoop;
    91139};
    92 
    93 WebCoreSynchronousLoader::WebCoreSynchronousLoader(ResourceError& error, ResourceResponse& response, Vector<char>& data)
    94     : m_error(error)
    95     , m_response(response)
    96     , m_data(data)
    97     , m_finished(false)
    98 {
    99     m_mainLoop = adoptGRef(g_main_loop_new(0, false));
    100 }
    101 
    102 WebCoreSynchronousLoader::~WebCoreSynchronousLoader()
    103 {
    104 }
    105 
    106 void WebCoreSynchronousLoader::didReceiveResponse(ResourceHandle*, const ResourceResponse& response)
    107 {
    108     m_response = response;
    109 }
    110 
    111 void WebCoreSynchronousLoader::didReceiveData(ResourceHandle*, const char* data, int length, int)
    112 {
    113     m_data.append(data, length);
    114 }
    115 
    116 void WebCoreSynchronousLoader::didFinishLoading(ResourceHandle*, double)
    117 {
    118     g_main_loop_quit(m_mainLoop.get());
    119     m_finished = true;
    120 }
    121 
    122 void WebCoreSynchronousLoader::didFail(ResourceHandle* handle, const ResourceError& error)
    123 {
    124     m_error = error;
    125     didFinishLoading(handle, 0);
    126 }
    127 
    128 void WebCoreSynchronousLoader::run()
    129 {
    130     if (!m_finished)
    131         g_main_loop_run(m_mainLoop.get());
    132 }
    133140
    134141static void cleanupSoupRequestOperation(ResourceHandle*, bool isDestroying);
    … …  
    635642{
    636643    RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(data);
    637 
    638644    ResourceHandleInternal* d = handle->getInternal();
     645
    639646    g_input_stream_close_finish(d->m_inputStream.get(), res, 0);
     647
     648    ResourceHandleClient* client = handle->client();
     649    if (client && loadingSynchronousRequest)
     650        client->didFinishLoading(handle.get(), 0);
     651
    640652    cleanupSoupRequestOperation(handle.get());
    641653}
    … …  
    668680    if (!bytesRead) {
    669681        // We inform WebCore of load completion now instead of waiting for the input
    670         // stream to close because the input stream is closed asynchronously.
    671         client->didFinishLoading(handle.get(), 0);
     682        // stream to close because the input stream is closed asynchronously. If this
     683        // is a synchronous request, we wait until the closeCallback, because we don't
     684        // want to halt the internal main loop before the input stream closes.
     685        if (client && !loadingSynchronousRequest) {
     686            client->didFinishLoading(handle.get(), 0);
     687            handle->setClient(0); // Unset the client so that we do not try to access th
     688                                  // client in the closeCallback.
     689        }
    672690        g_input_stream_close_async(d->m_inputStream.get(), G_PRIORITY_DEFAULT, 0, closeCallback, handle.get());
    673691        return;
    … …  
    741759                     SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_CONTENT_SNIFFER,
    742760                     SOUP_SESSION_ADD_FEATURE_BY_TYPE, SOUP_TYPE_PROXY_RESOLVER_DEFAULT,
     761                     SOUP_SESSION_USE_THREAD_CONTEXT, TRUE,
    743762                     NULL);
    744763    }
Note: See TracChangeset for help on using the changeset viewer.