256983 - Crash [E] NPR: NULL pointer read in PL_HashTableAdd {1 occurrence}

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[timeless]

confirmed by me using 2004081808 talkbackid: 657285

steps:
1. run mfcembed (under purify or normal)

    [I] Starting Purify'd R:\mozilla\rel-i586-pc-msvc.1\dist\bin\mfcembed.exe 
at 08/26/2004 03:53:54
    [I] Starting main
    [E] NPR: NULL pointer read in PL_HashTableAdd {1 occurrence}
        Reading 4 bytes from 0x0000000c (4 bytes at 0x0000000c illegal)
        Address 0x0000000c points into invalid memory 
        Thread ID: 0x173e4
        Error location
        PL_HashTableAdd+0x41 [r:\mozilla\nsprpub\lib\ds\plhash.c:309 
ip=0x02602d91]
        
        PR_IMPLEMENT(PLHashEntry *)
        PL_HashTableAdd(PLHashTable *ht, const void *key, void *value)
        {
            PLHashNumber keyHash;
            PLHashEntry *he, **hep;
        
     =>     keyHash = (*ht->keyHash)(key);
            hep = PL_HashTableRawLookup(ht, keyHash, key);
            if ((he = *hep) != 0) {
                /* 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;
            }
            return PL_HashTableRawAdd(ht, hep, keyHash, key, value);
        }
        PL_HashTableAdd+0x37 [r:\mozilla\nsprpub\lib\ds\plhash.c:309 
ip=0x02602d87]
        
        PR_IMPLEMENT(PLHashEntry *)
        PL_HashTableAdd(PLHashTable *ht, const void *key, void *value)
        {
            PLHashNumber keyHash;
            PLHashEntry *he, **hep;
                    
                 =>     keyHash = (*ht->keyHash)(key);
                        hep = PL_HashTableRawLookup(ht, keyHash, key);
                        if ((he = *hep) != 0) {
                            /* 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;
                        }
                        return PL_HashTableRawAdd(ht, hep, keyHash, key, value);
                    }
                tmTransactionService::Attach(nsACString 
const&,ipcITransactionObserver *,int)+0x17e 
[r:\mozilla\ipc\ipcd\extensions\transmngr\src\tmtransactionservice.cpp:165 
ip=0x0510aed6]
                        return TM_ERROR_QUEUE_EXISTS;
                    
                      // create the full queue name: namespace + queue
                      nsCString jQName;
                      jQName.Assign(mNamespace);
                      jQName.Append(aDomainName);
                    
                      // this char* has two homes, make sure it gets PL_free()
ed properly
                      char* joinedQueueName = ToNewCString(jQName);
                      if (!joinedQueueName)
                        return NS_ERROR_OUT_OF_MEMORY;
                    
                      // link the observer to the joinedqueuename.  home #1 for 
joinedQueueName
                      // these currently don't get removed until the destructor 
on this is called.
                 =>   PL_HashTableAdd(mObservers, joinedQueueName, aObserver);
                    
                      // store the domainName and JoinedQueueName, create a 
place to store the ID
                      tm_queue_mapping *qm = new tm_queue_mapping();
                      if (!qm)
                        return NS_ERROR_OUT_OF_MEMORY;
                      qm->queueID = TM_NO_ID;                   // initially no 
ID for the queue
                      qm->joinedQueueName = joinedQueueName;    // home #2, 
owner of joinedQueueName
                      qm->domainName = ToNewCString(aDomainName);
                      if (!qm->domainName) {
                        PL_HashTableRemove(mObservers, joinedQueueName);
                        delete qm;
                        return NS_ERROR_OUT_OF_MEMORY;
                      }
                      mQueueMaps.Append(qm);
                nsSharedPrefHandler::OnSessionBegin(void)+0xfe 
[r:\mozilla\modules\libpref\src\nssharedprefhandler.cpp:120 ip=0x03dc5777]
                nsPrefService::Observe(nsISupports *,char const*,WORD const*)
+0x2e2 [r:\mozilla\modules\libpref\src\nsprefservice.cpp:197 ip=0x03dbfe08]
                nsObserverService::NotifyObservers(nsISupports *,char 
const*,WORD const*)+0x277 [r:\mozilla\xpcom\ds\nsobserverservice.cpp:230 
ip=0x024bc68a]
                nsProfile::SetCurrentProfile(WORD const*)+0xc54 
[r:\mozilla\profile\src\nsprofile.cpp:1282 ip=0x0505c81c]
                CProfileMgr::StartUp(void)+0x3f7 
[r:\mozilla\embedding\tests\mfcembed\profilemgr.cpp:112 ip=0x0041acca]
                CMfcEmbedApp::InitializeProfiles(void)+0x469 
[r:\mozilla\embedding\tests\mfcembed\mfcembed.cpp:697 ip=0x004085f3]
                CMfcEmbedApp::InitInstance(void)+0x442 
[r:\mozilla\embedding\tests\mfcembed\mfcembed.cpp:467 ip=0x00408b8d]
    [E] EXU: Unhandled exception in PL_HashTableAdd {1 occurrence}
    [I] Starting thread 0x788c

Comment 1

[timeless]

Attachment: null check mObservers

Comment 2

[Darin Fisher]

Did you have to do anything special to make this happen?  Any patches in your
build that I should know about?  It would seem that PL_NewHashTable returned
NULL.  How did that happen?  Simple out-of-memory condition?  But,
tmTransactionService::Init returns a failure code when PL_NewHashTable fails, so
how could anyone ever call Attach?  hmm... or was mObservers null'd out
someplace else?  hmm... I don't understand how Attach could be reached with
mObservers null.  Can you explain how that happens?

Comment 3

[timeless]

this relates to running mfcembed a couple of times, the ipc daemon may be
starting or stopping, i don't know much else. i've managed to trigger it a
couple of times, both w/ release builds (hence talkback) and w/ my cvs builds....

Comment 4

[Darin Fisher]

ok... thx.  the builds were unpatched right?  ;-)

Comment 5

[timeless]

kinda hard to build talkback enabled builds here ;-)

Comment 6

[Darin Fisher]

Comment on attachment 157065 [details] [diff] [review]
null check mObservers

>Index: tmTransactionService.cpp

>+  lockService = do_GetService("@mozilla.org/ipc/lock-service;1", &rv);
>   if (!lockService)
>+    return rv;

how about changing this to:

    if (NS_FAILED(rv))
      return rv;


>   // if the queue already exists, then someone else is attached to it. must
>   //   return an error here. Only one module attached to a queue per app.
>   if (GetQueueID(aDomainName) != TM_NO_ID)
>     return TM_ERROR_QUEUE_EXISTS;
>+  if (!mObservers)
>+    return NS_ERROR_NOT_INITIALIZED;

maybe move this up to the top of the function?	should the other methods
be similarly tested?  why is someone calling Attach when Init failed?

do you have a stack trace for this?


r+sr=darin

Comment 7

[timeless]

mozilla/ipc/ipcd/extensions/transmngr/src/tmTransactionService.cpp 	1.6