Commit f0dc5ade authored by antti@apple.com's avatar antti@apple.com

2008-05-05 Antti Koivisto <antti@apple.com>

        Reviewed by Darin.

        Speculative fix for <rdar://problem/5906790> 
        Crash in Loader::servePendingRequests() due to hash table being modified during iteration
        
        I don't know how to reproduce this. It would require the load to fail (or succeed)
        synchronously, something that should not usually happen.

        * loader/loader.cpp:
        (WebCore::Loader::Loader):
        (WebCore::Loader::load):
        (WebCore::Loader::servePendingRequests):
        (WebCore::Loader::cancelRequests):
        (WebCore::Loader::Host::Host):
        * loader/loader.h:
        (WebCore::Loader::Host::name):



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@32871 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent ed762a03
2008-05-05 Antti Koivisto <antti@apple.com>
Reviewed by Darin.
Speculative fix for <rdar://problem/5906790>
Crash in Loader::servePendingRequests() due to hash table being modified during iteration
I don't know how to reproduce this. It would require the load to fail (or succeed)
synchronously, something that should not usually happen.
* loader/loader.cpp:
(WebCore::Loader::Loader):
(WebCore::Loader::load):
(WebCore::Loader::servePendingRequests):
(WebCore::Loader::cancelRequests):
(WebCore::Loader::Host::Host):
* loader/loader.h:
(WebCore::Loader::Host::name):
2008-05-05 Ariya Hidayat <ariya.hidayat@trolltech.com>
Reviewed by Simon.
......@@ -58,7 +58,7 @@ static const unsigned maxRequestsInFlightForNonHTTPProtocols = 10000;
Loader::Loader()
: m_nonHTTPProtocolHost(maxRequestsInFlightForNonHTTPProtocols)
: m_nonHTTPProtocolHost("non-http-protocol-host", maxRequestsInFlightForNonHTTPProtocols)
, m_requestTimer(this, &Loader::requestTimerFired)
{
}
......@@ -102,11 +102,11 @@ void Loader::load(DocLoader* docLoader, CachedResource* resource, bool increment
KURL url(resource->url());
bool isHTTP = url.protocolIs("http") || url.protocolIs("https");
if (isHTTP) {
String hostName = url.host();
host = m_hosts.get(hostName);
AtomicString hostName = url.host();
host = m_hosts.get(hostName.impl());
if (!host) {
host = new Host(maxRequestsInFlightPerHost);
m_hosts.add(hostName, host);
host = new Host(hostName, maxRequestsInFlightPerHost);
m_hosts.add(hostName.impl(), host);
}
} else
host = &m_nonHTTPProtocolHost;
......@@ -141,19 +141,17 @@ void Loader::servePendingRequests(Priority minimumPriority)
m_nonHTTPProtocolHost.servePendingRequests(minimumPriority);
HostMap::iterator end = m_hosts.end();
Vector<String> toRemove;
for (HostMap::iterator it = m_hosts.begin(); it != end; ++it) {
Host* host = it->second;
Vector<Host*> hostsToServe;
copyValuesToVector(m_hosts, hostsToServe);
for (unsigned n = 0; n < hostsToServe.size(); ++n) {
Host* host = hostsToServe[n];
if (host->hasRequests())
host->servePendingRequests(minimumPriority);
else
toRemove.append(it->first);
}
for (unsigned n = 0; n < toRemove.size(); ++n) {
HostMap::iterator it = m_hosts.find(toRemove[n]);
delete it->second;
m_hosts.remove(it);
else {
AtomicString name = host->name();
delete host;
m_hosts.remove(name.impl());
}
}
}
......@@ -162,9 +160,10 @@ void Loader::cancelRequests(DocLoader* docLoader)
if (m_nonHTTPProtocolHost.hasRequests())
m_nonHTTPProtocolHost.cancelRequests(docLoader);
HostMap::iterator end = m_hosts.end();
for (HostMap::iterator it = m_hosts.begin(); it != end; ++it) {
Host* host = it->second;
Vector<Host*> hostsToCancel;
copyValuesToVector(m_hosts, hostsToCancel);
for (unsigned n = 0; n < hostsToCancel.size(); ++n) {
Host* host = hostsToCancel[n];
if (host->hasRequests())
host->cancelRequests(docLoader);
}
......@@ -177,8 +176,9 @@ void Loader::cancelRequests(DocLoader* docLoader)
ASSERT(docLoader->requestCount() == 0);
}
Loader::Host::Host(unsigned maxRequestsInFlight)
: m_maxRequestsInFlight(maxRequestsInFlight)
Loader::Host::Host(const AtomicString& name, unsigned maxRequestsInFlight)
: m_name(name)
, m_maxRequestsInFlight(maxRequestsInFlight)
{
}
......
......@@ -22,8 +22,9 @@
#ifndef loader_h
#define loader_h
#include "AtomicString.h"
#include "AtomicStringImpl.h"
#include "PlatformString.h"
#include "StringHash.h"
#include "SubresourceLoaderClient.h"
#include "Timer.h"
#include <wtf/Deque.h>
......@@ -56,9 +57,10 @@ namespace WebCore {
class Host : private SubresourceLoaderClient {
public:
Host(unsigned maxRequestsInFlight);
Host(const AtomicString& name, unsigned maxRequestsInFlight);
~Host();
const AtomicString& name() const { return m_name; }
void addRequest(Request*, Priority);
void servePendingRequests(Priority minimumPriority = Low);
void cancelRequests(DocLoader*);
......@@ -78,9 +80,10 @@ namespace WebCore {
RequestQueue m_requestsPending[High + 1];
typedef HashMap<RefPtr<SubresourceLoader>, Request*> RequestMap;
RequestMap m_requestsLoading;
const AtomicString m_name;
const int m_maxRequestsInFlight;
};
typedef HashMap<String, Host*> HostMap;
typedef HashMap<AtomicStringImpl*, Host*> HostMap;
HostMap m_hosts;
Host m_nonHTTPProtocolHost;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment