Closed Bug 28960 Opened 25 years ago Closed 25 years ago

Memory leak in JS_HashTableAdd (jshash.c)

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: russell, Assigned: rogerl)

Details

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
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
Closed: 25 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.