Memory leak in JS_HashTableAdd (jshash.c)

RESOLVED WONTFIX

Status

()

Core
JavaScript Engine
P3
normal
RESOLVED WONTFIX
18 years ago
16 years ago

People

(Reporter: Russell Pflughaupt, Assigned: rogerl (gone))

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

18 years ago
The JS_HashTableAdd checks for hash table keys and data already being present.  
If this is found to be true, the function re-uses the existing key/data and 
returns to the caller.  This results in memory leaks because the caller won't 
free its just-passed-in key/data because they're assuming that JS_HashTableAdd 
is holding onto it.

Here is a modified version of the funciton that correctly handles adding a key 
to the hash table, even if that key is already present:

JS_EXPORT_API(JSHashEntry *)
JS_HashTableAdd(JSHashTable *ht, const void *key, void *value)
{
    JSHashNumber keyHash;
    JSHashEntry *he, **hep;

    keyHash = (*ht->keyHash)(key);
    hep = JS_HashTableRawLookup(ht, keyHash, key);
    if ((he = *hep) != NULL) {
#ifdef ORIGINAL_VERSION
        // I just don't understand how this code could possibly
        // work.  The caller to PL_HashTableAdd might have just
        // allocated the passed-in key and value, and then this
        // code checks for the key/value for already being in
        // the table AND DOESN'T INFORM THE CALLER OF THIS!
        // It looks like this code was pulled from an environment
        // that has garbage collection.
        //
        // The only right thing to do is free the old key/value
        // and install the new ones, even if they're identical.

        /* Hit; see if values match */
        if ((*ht->valueCompare)(he->value, value)) {
            /* key,value pair is already present in table */
            return he;
        }
        if (he->value)
            (*ht->allocOps->freeEntry)(ht->allocPriv, he, HT_FREE_VALUE);
        he->value = value;
        return he;
#else
        JS_HashTableRawRemove(ht, hep, he);
        hep = NULL;
#endif
    }
    return JS_HashTableRawAdd(ht, hep, keyHash, key, value);
}
confirming so that someone with mozilla-coding experience will have a look at
this...
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 2

18 years ago
Thanks for the suggested patch - as it turns out, the JS engine is using a GC 
and so doesn't need to handle the situation you're addressing here. We've not 
intended the individual engine components to be used other than in the context 
of an executing engine, so we wouldn't need to support this case.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.