73623 - crash in nsCacheService::SearchCacheDevices() due to faulty error-checking

Status: RESOLVED INVALID

(Core :: Networking: Cache, defect)

Description

[Håkan Waara]

I crashed in nsCacheService::SearchCacheDevices, and debugged the code. Here is
a snippet:

-----------------------------------------------
        if (!mDiskDevice) {
            nsresult rv = CreateDiskDevice();
            if (NS_FAILED(rv))
                return nsnull;
        }

        entry = mDiskDevice->FindEntry(key);
-----------------------------------------------

The reason I crashed was because mDiskDevice was null, and even though there was
a nullcheck a few lines above the call, it didn't return. Instead it re-tried
(or something) to do CreateDiskDevice() and if that succeeded then it didn't return.

So (obviously), rv succeeded while mDiskDevice still was as null as ever, which
lead to a crash.

A possible fix could be:

-           if (NS_FAILED(rv))
+           if (NS_FAILED(rv) || nsnull == mDiskDevice)

Assuming that the function call to CreateDiskDevice() is an attempt to
un-nullify mDiskDevice.

Comment 1

[neeti]

-->gordon

Comment 2

[gordon]

Here is the the source for CreateDiskDevice().


nsresult
nsCacheService::CreateDiskDevice()
{
    nsresult rv = NS_OK;
    if (!mDiskDevice) {
    // create disk cache lazily
        mDiskDevice = new nsDiskCacheDevice;
        if (mDiskDevice) {
            rv = mDiskDevice->Init();
            if (NS_FAILED(rv)) {
                delete mDiskDevice;
                mDiskDevice = nsnull;
            }
        } else {
            rv = NS_ERROR_OUT_OF_MEMORY;
        }
    }
    return rv;
 }


I don't see how it can succeed while leaving mDiskDevice == nsnull.  How did you 
verify that mDiskDevice was nsnull?  Could you attach a stack crawl?

Comment 3

[Håkan Waara]

I verified this in the debugger, when I launched during runtime the little green
pointer pointed at the line after "entry = mDiskDevice->FindEntry(key);", and
that was why it had crashed.

I looked and mDiskDevice's value was 0x0000000. I'm as clueless as you on how
this can work out, but I did crash, and mDiskDevice crashed because it was null.

Sorry, I don't have a stacktrace left. :(

Comment 4

[gordon]

This code has changed a bit, so I'm going to close this bug since I can't 
reproduce it, and it doesn't appear to be possible.

If you see this again, please reopen the bug and attach the stack crawl.

Thanks.