From d7017bfdec9ffad9ee647eaf0a0d8852003c13f4 Mon Sep 17 00:00:00 2001 From: "darin%meer.net" Date: Fri, 1 Apr 2005 18:35:14 +0000 Subject: [PATCH] fixes bug 220566 "Randomly hangs in PL_HashTableRawLookup at start of page load" r=brendan --- netwerk/protocol/http/src/nsHttp.cpp | 215 +++++++++++--------- netwerk/protocol/http/src/nsHttp.h | 7 +- netwerk/protocol/http/src/nsHttpHandler.cpp | 6 +- netwerk/test/Makefile.in | 4 +- netwerk/test/unit/test_http_headers.js | 3 + 5 files changed, 139 insertions(+), 96 deletions(-) diff --git a/netwerk/protocol/http/src/nsHttp.cpp b/netwerk/protocol/http/src/nsHttp.cpp index 6efa4e3b1ae0..c95f01280bb2 100644 --- a/netwerk/protocol/http/src/nsHttp.cpp +++ b/netwerk/protocol/http/src/nsHttp.cpp @@ -1,4 +1,5 @@ /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* vim:set ts=4 sw=4 sts=4 et cin: */ /* ***** BEGIN LICENSE BLOCK ***** * Version: MPL 1.1/GPL 2.0/LGPL 2.1 * @@ -37,8 +38,8 @@ * ***** END LICENSE BLOCK ***** */ #include "nsHttp.h" -#include "nscore.h" -#include "plhash.h" +#include "nsAutoLock.h" +#include "pldhash.h" #include "nsCRT.h" #if defined(PR_LOGGING) @@ -50,138 +51,166 @@ PRLogModuleInfo *gHttpLog = nsnull; #include "nsHttpAtomList.h" #undef HTTP_ATOM -// we keep a linked list of atoms allocated on the heap for easy clean up -// when the atom table is destroyed. -struct HttpHeapAtom { - char *value; - struct HttpHeapAtom *next; +// find out how many atoms we have +#define HTTP_ATOM(_name, _value) Unused_ ## _name, +enum { +#include "nsHttpAtomList.h" + NUM_HTTP_ATOMS +}; +#undef HTTP_ATOM - HttpHeapAtom(const char *v) : value(PL_strdup(v)), next(0) {} - ~HttpHeapAtom() { PL_strfree(value); } +// we keep a linked list of atoms allocated on the heap for easy clean up when +// the atom table is destroyed. The structure and value string are allocated +// as one contiguous block. + +struct HttpHeapAtom { + struct HttpHeapAtom *next; + char value[1]; }; -static struct PLHashTable *gHttpAtomTable = nsnull; -static struct HttpHeapAtom *gHeapAtomsHead = nsnull; -static struct HttpHeapAtom *gHeapAtomsTail = nsnull; +static struct PLDHashTable sAtomTable = {0}; +static struct HttpHeapAtom *sHeapAtoms = nsnull; +static PRLock *sLock = nsnull; + +HttpHeapAtom * +NewHeapAtom(const char *value) { + int len = strlen(value); + + HttpHeapAtom *a = + NS_REINTERPRET_CAST(HttpHeapAtom *, malloc(sizeof(*a) + len)); + if (!a) + return nsnull; + memcpy(a->value, value, len + 1); + + // add this heap atom to the list of all heap atoms + a->next = sHeapAtoms; + sHeapAtoms = a; + + return a; +} // Hash string ignore case, based on PL_HashString -static PLHashNumber -StringHash(const PRUint8 *key) +PR_STATIC_CALLBACK(PLDHashNumber) +StringHash(PLDHashTable *table, const void *key) { - PLHashNumber h; - const PRUint8 *s; - - h = 0; - for (s = key; *s; s++) - h = (h >> 28) ^ (h << 4) ^ nsCRT::ToLower((char)*s); + PLDHashNumber h = 0; + for (const char *s = NS_REINTERPRET_CAST(const char*, key); *s; ++s) + h = (h >> 28) ^ (h << 4) ^ nsCRT::ToLower(*s); return h; } -static PRIntn -StringCompare(const char *a, const char *b) +PR_STATIC_CALLBACK(PRBool) +StringCompare(PLDHashTable *table, const PLDHashEntryHdr *entry, + const void *testKey) { - return PL_strcasecmp(a, b) == 0; + const void *entryKey = + NS_REINTERPRET_CAST(const PLDHashEntryStub *, entry)->key; + + return PL_strcasecmp(NS_REINTERPRET_CAST(const char *, entryKey), + NS_REINTERPRET_CAST(const char *, testKey)) == 0; } -#if 0 -#define NBUCKETS(ht) (1 << (PL_HASH_BITS - (ht)->shift)) -static void -DumpAtomTable() -{ - if (gHttpAtomTable) { - PLHashEntry *he, **hep; - PRUint32 i, nbuckets = NBUCKETS(gHttpAtomTable); - for (i=0; ibuckets[i]; - while ((he = *hep) != 0) { - printf("(%s,%x,%x) ", (const char *) he->key, he->keyHash, he->value); - hep = &he->next; - } - printf("\n"); - } - } -} -#endif +static const PLDHashTableOps ops = { + PL_DHashAllocTable, + PL_DHashFreeTable, + PL_DHashGetKeyStub, + StringHash, + StringCompare, + PL_DHashMoveEntryStub, + PL_DHashClearEntryStub, + PL_DHashFinalizeStub, + nsnull +}; // We put the atoms in a hash table for speedy lookup.. see ResolveAtom. -static nsresult -CreateAtomTable() +nsresult +nsHttp::CreateAtomTable() { - LOG(("CreateAtomTable\n")); + NS_ASSERTION(!sAtomTable.ops, "atom table already initialized"); - if (gHttpAtomTable) - return NS_OK; + if (!sLock) { + sLock = PR_NewLock(); + if (!sLock) + return NS_ERROR_OUT_OF_MEMORY; + } - gHttpAtomTable = PL_NewHashTable(128, (PLHashFunction) StringHash, - (PLHashComparator) StringCompare, - (PLHashComparator) 0, 0, 0); - if (!gHttpAtomTable) + // The capacity for this table is initialized to a value greater than the + // number of known atoms (NUM_HTTP_ATOMS) because we expect to encounter a + // few random headers right off the bat. + if (!PL_DHashTableInit(&sAtomTable, &ops, nsnull, sizeof(PLDHashEntryStub), + NUM_HTTP_ATOMS + 10)) { + sAtomTable.ops = nsnull; return NS_ERROR_OUT_OF_MEMORY; + } -#define HTTP_ATOM(_name, _value) \ - PL_HashTableAdd(gHttpAtomTable, _value, (void *) nsHttp::_name.get()); + // fill the table with our known atoms + const char *const atoms[] = { +#define HTTP_ATOM(_name, _value) nsHttp::_name._val, #include "nsHttpAtomList.h" #undef HTTP_ATOM + nsnull + }; + + for (int i = 0; atoms[i]; ++i) { + PLDHashEntryStub *stub = NS_REINTERPRET_CAST(PLDHashEntryStub *, + PL_DHashTableOperate(&sAtomTable, atoms[i], PL_DHASH_ADD)); + if (!stub) + return NS_ERROR_OUT_OF_MEMORY; + + NS_ASSERTION(!stub->key, "duplicate static atom"); + stub->key = atoms[i]; + } - //DumpAtomTable(); return NS_OK; } void nsHttp::DestroyAtomTable() { - if (gHttpAtomTable) { - PL_HashTableDestroy(gHttpAtomTable); - gHttpAtomTable = nsnull; + if (sAtomTable.ops) { + PL_DHashTableFinish(&sAtomTable); + sAtomTable.ops = nsnull; } - while (gHeapAtomsHead) { - gHeapAtomsTail = gHeapAtomsHead->next; - delete gHeapAtomsHead; - gHeapAtomsHead = gHeapAtomsTail; + + while (sHeapAtoms) { + HttpHeapAtom *next = sHeapAtoms->next; + free(sHeapAtoms); + sHeapAtoms = next; + } + + if (sLock) { + PR_DestroyLock(sLock); + sLock = nsnull; } - gHeapAtomsTail = nsnull; } +// this function may be called from multiple threads nsHttpAtom nsHttp::ResolveAtom(const char *str) { - if (!gHttpAtomTable) - CreateAtomTable(); - nsHttpAtom atom = { nsnull }; - if (gHttpAtomTable && str) { - atom._val = (const char *) PL_HashTableLookup(gHttpAtomTable, str); + if (!str || !sAtomTable.ops) + return atom; - // if the atom could not be found in the atom table, then we'll go - // and allocate a new atom on the heap. - if (!atom) { - HttpHeapAtom *heapAtom = new HttpHeapAtom(str); - if (!heapAtom) - return atom; - if (!heapAtom->value) { - delete heapAtom; - return atom; - } + nsAutoLock lock(sLock); - // append this heap atom to the list of all heap atoms - if (!gHeapAtomsHead) { - gHeapAtomsHead = heapAtom; - gHeapAtomsTail = heapAtom; - } - else { - gHeapAtomsTail->next = heapAtom; - gHeapAtomsTail = heapAtom; - } + PLDHashEntryStub *stub = NS_REINTERPRET_CAST(PLDHashEntryStub *, + PL_DHashTableOperate(&sAtomTable, str, PL_DHASH_ADD)); + if (!stub) + return atom; // out of memory - // now insert the heap atom into the atom table - PL_HashTableAdd(gHttpAtomTable, heapAtom->value, heapAtom->value); - - // now assign the value to the atom - atom._val = (const char *) heapAtom->value; - } + if (stub->key) { + atom._val = NS_REINTERPRET_CAST(const char *, stub->key); + return atom; } + // if the atom could not be found in the atom table, then we'll go + // and allocate a new atom on the heap. + HttpHeapAtom *heapAtom = NewHeapAtom(str); + if (!heapAtom) + return atom; // out of memory + + stub->key = atom._val = heapAtom->value; return atom; } diff --git a/netwerk/protocol/http/src/nsHttp.h b/netwerk/protocol/http/src/nsHttp.h index 2a4c93e78214..051604ea4dc0 100644 --- a/netwerk/protocol/http/src/nsHttp.h +++ b/netwerk/protocol/http/src/nsHttp.h @@ -1,4 +1,5 @@ /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ +/* vim:set ts=4 sw=4 sts=4 et cin: */ /* ***** BEGIN LICENSE BLOCK ***** * Version: MPL 1.1/GPL 2.0/LGPL 2.1 * @@ -133,11 +134,15 @@ struct nsHttpAtom struct nsHttp { + static nsresult CreateAtomTable(); static void DestroyAtomTable(); // will dynamically add atoms to the table if they don't already exist static nsHttpAtom ResolveAtom(const char *); - static nsHttpAtom ResolveAtom(const nsACString &s) { return ResolveAtom(PromiseFlatCString(s).get()); } + static nsHttpAtom ResolveAtom(const nsACString &s) + { + return ResolveAtom(PromiseFlatCString(s).get()); + } /* Declare all atoms * diff --git a/netwerk/protocol/http/src/nsHttpHandler.cpp b/netwerk/protocol/http/src/nsHttpHandler.cpp index 057672ee107a..0285d20b903d 100644 --- a/netwerk/protocol/http/src/nsHttpHandler.cpp +++ b/netwerk/protocol/http/src/nsHttpHandler.cpp @@ -196,10 +196,14 @@ nsHttpHandler::~nsHttpHandler() nsresult nsHttpHandler::Init() { - nsresult rv = NS_OK; + nsresult rv; LOG(("nsHttpHandler::Init\n")); + rv = nsHttp::CreateAtomTable(); + if (NS_FAILED(rv)) + return rv; + mIOService = do_GetService(kIOServiceCID, &rv); if (NS_FAILED(rv)) { NS_WARNING("unable to continue without io service"); diff --git a/netwerk/test/Makefile.in b/netwerk/test/Makefile.in index 3aa8534dd925..d1c1cfb045f7 100644 --- a/netwerk/test/Makefile.in +++ b/netwerk/test/Makefile.in @@ -94,7 +94,9 @@ include $(topsrcdir)/config/rules.mk _UNIT_FILES = unit/test_all.sh \ unit/head.js \ unit/tail.js \ - unit/test_protocolproxyservice.js + unit/test_protocolproxyservice.js \ + unit/test_http_headers.js \ + $(NULL) libs:: $(_UNIT_FILES) $(INSTALL) $^ $(DIST)/bin/necko_unit_tests diff --git a/netwerk/test/unit/test_http_headers.js b/netwerk/test/unit/test_http_headers.js index 753afefabd08..b722139238a6 100644 --- a/netwerk/test/unit/test_http_headers.js +++ b/netwerk/test/unit/test_http_headers.js @@ -8,6 +8,9 @@ function run_test() { var chan = ios.newChannel("http://www.mozilla.org/", null, null) .QueryInterface(Components.interfaces.nsIHttpChannel); + check_header(chan, "host", "www.mozilla.org"); + check_header(chan, "Host", "www.mozilla.org"); + chan.setRequestHeader("foopy", "bar", false); check_header(chan, "foopy", "bar");