128978 - Memory leak of 56 bytes from 1 block allocated in nsGlobalHistory::AutoCompleteSearch

Status: RESOLVED INCOMPLETE

(Core Graveyard :: History: Global, defect)

Description

[[e-mail] (gone - use [e-mail] instead)]

Build ID: Latest win32 trunk build - Purify.

Steps to Reproduce:

1.  I just typed a nonsensical domain (www.foobagbabysearch.com) and hit Enter.
2.  Got an alert about the domain being invalid, then I shutdown.

[W] MLK: Memory leak of 56 bytes from 1 block allocated in 
nsGlobalHistory::AutoCompleteSearch(nsAString const&,AutocompleteExclude 
*,nsIAutoCompleteResults *,nsIAutoCompleteResults *)
        Distribution of leaked blocks
        Allocation location
            new(UINT)      [MSVCRT.DLL]
            nsGlobalHistory::AutoCompleteSearch(nsAString 
const&,AutocompleteExclude *,nsIAutoCompleteResults *,nsIAutoCompleteResults *) 
[nsGlobalHistory.cpp:4028]
                                                            kToken_HiddenColumn,
                                                            kToken_TypedColumn,
                                                            
mAutocompleteOnlyTyped,
             =>                                             aSearchString, 
aExclude);
                    rv = enumerator->Init(mEnv, mTable);
                    if (NS_FAILED(rv)) return rv;
                
            nsGlobalHistory::OnStartLookup(WORD const*,nsIAutoCompleteResults 
*,nsIAutoCompleteListener *) [nsGlobalHistory.cpp:3928]
                  AutoCompleteGetExcludeInfo(filtered, &exclude);
                
                  // perform the actual search here
             =>   rv = AutoCompleteSearch(filtered, &exclude, 
previousSearchResult, results);
                
                  // describe the search results
                  if (NS_SUCCEEDED(rv)) {
            XPTC_InvokeByIndex [xptcinvoke.cpp:105]
                        mov     eax,methodIndex
                        shl     eax,2               // *= 4
                        add     edx,eax
             =>         call    [edx]               // stdcall, i.e. callee 
cleans up stack.
                        mov     esp,ebp
                        pop     ebp
                        ret
            XPCWrappedNative::CallMethod
(XPCCallContext&,CallMode::XPCWrappedNative) [xpcwrappednative.cpp:2020]
                
                        // do the invoke
                        invokeResult = XPTC_InvokeByIndex(callee, vtblIndex,
             =>                                           paramCount, 
dispatchParams);
                        // resume non-blocking JS operations now
                    }
                
            XPC_WN_CallMethod(JSContext *,JSObject *,UINT,long *,long *) 
[xpcwrappednativejsops.cpp:1266]
                    if(!XPCNativeMember::GetCallInfo(ccx, funobj, &iface, 
&member))
                        return Throw(NS_ERROR_XPC_CANT_GET_METHOD_INFO, cx);
                    ccx.SetCallInfo(iface, member, JS_FALSE);
             =>     return XPCWrappedNative::CallMethod(ccx);
                }
                
                JSBool JS_DLL_CALLBACK
            js_Invoke      [jsinterp.c:788]
                        /* If native, use caller varobj and scopeChain for 
eval. */
                        frame.varobj = fp->varobj;
                        frame.scopeChain = fp->scopeChain;
             =>         ok = native(cx, frame.thisp, argc, frame.argv, 
&frame.rval);
                        JS_RUNTIME_METER(cx->runtime, nativeCalls);
                    } else if (script) {
                        /* Use parent scope so js_GetCallObject can find the 
right "Call". */
            js_Interpret   [jsinterp.c:2745]
                                goto out;
                            }
                
             =>             ok = js_Invoke(cx, argc, 0);
                            RESTORE_SP(fp);
                            if (!ok)
                                goto out;
            js_Invoke      [jsinterp.c:805]
                            frame.scopeChain = funobj;
                #endif
                        }
             =>         ok = js_Interpret(cx, &v);
                    } else {
                        /* fun might be onerror trying to report a syntax error 
in itself. */
                        frame.scopeChain = NULL;
            js_InternalInvoke [jsinterp.c:880]
                    for (i = 0; i < argc; i++)
                        PUSH(argv[i]);
                    fp->sp = sp;
             =>     ok = js_Invoke(cx, argc, flags | JSINVOKE_INTERNAL);
                    if (ok) {
                        RESTORE_SP(fp);
                        *rval = POP_OPND();
            JS_CallFunctionValue [jsapi.c:3388]
                                     jsval *argv, jsval *rval)
                {
                    CHECK_REQUEST(cx);
             =>     if (!js_InternalCall(cx, obj, fval, argc, argv, rval)) {
                #if JS_HAS_EXCEPTIONS
                        if (!cx->fp)
                            js_ReportUncaughtException(cx);
            nsJSContext::CallEventHandler(void *,void *,UINT,void *,int *,int) 
[nsJSEnvironment.cpp:1016]
                    jsval val;
                    jsval funval = OBJECT_TO_JSVAL(aHandler);
                    PRBool ok = ::JS_CallFunctionValue(mContext, (JSObject *)
aTarget, funval,
             =>                                 argc, (jsval *)argv, &val);
                    *aBoolResult = ok
                                   ? !JSVAL_IS_BOOLEAN(val) || 
(aReverseReturnResult ? !JSVAL_TO_BOOLEAN(val) : JSVAL_TO_BOOLEAN(val))
                                   : JS_TRUE;
            GlobalWindowImpl::RunTimeout(nsTimeoutImpl *) 
[nsGlobalWindow.cpp:4095]
                        PRBool bool_result;
                        rv = mContext->CallEventHandler(mJSObject, timeout-
>funobj,
                                                        timeout->argc + 1, 
timeout->argv,
             =>                                         &bool_result, PR_FALSE);
                      }
                
                      --mTimeoutFiringDepth;
            GlobalWindowImpl::TimerCallback(nsITimer *,void *) 
[nsGlobalWindow.cpp:4408]
                  // timeout. From now on 'timeout' has the reference to the 
timeout.
                  timeout->timer = nsnull;
                
             =>   timeout->window->RunTimeout(timeout);
                
                  // Drop timeout's reference to the timeout.
                  timeout->window->DropTimeout(timeout);
            ???            [ip=0x08c5f290]
            nsTimerImpl::Process(void) [nsTimerImpl.cpp:283]
                
                  if (mCallback.c) {
                    if (mCallbackType == CALLBACK_TYPE_FUNC)
             =>       (*mCallback.c)(this, mClosure);
                    else if (mCallbackType == CALLBACK_TYPE_INTERFACE)
                      mCallback.i->Notify(this);
                    else if (mCallbackType == CALLBACK_TYPE_OBSERVER)
            ???            [ip=0x0894c5b0]
            handleMyEvent(MyEventType *) [nsTimerImpl.cpp:320]
                
                
                void* handleMyEvent(MyEventType* event)
             => {
                #ifdef DEBUG_TIMERS
                  if (PR_LOG_TEST(gTimerLog, PR_LOG_DEBUG)) {
                    PRIntervalTime now = PR_IntervalNow();
            PL_HandleEvent [plevent.c:590]
                    /* This event better not be on an event queue anymore. */
                    PR_ASSERT(PR_CLIST_IS_EMPTY(&self->link));
                
             =>     result = (*self->handler)(self);
                    if (NULL != self->synchronousResult) {
                      PR_Lock(self->lock);
                      self->synchronousResult = result;
            PL_ProcessPendingEvents [plevent.c:520]
                        break;
                
                      PR_LOG(event_lm, PR_LOG_DEBUG, ("$$$ processing event"));
             =>       PL_HandleEvent(event);
                      PR_LOG(event_lm, PR_LOG_DEBUG, ("$$$ done processing 
event"));
                    }

Comment 1

[Alec Flett]

stephend, can you do a bunch of autocomplete runs, and see if you get lots of
leaks? you don't actually have to visit a site, just type a couple of unrelated
urls.

it would be good to know if this is a one-time-initialize-autocomplete leak, or
a happens-ever-time-you-type leak.

Comment 2

[[e-mail] (gone - use [e-mail] instead)]

This appears to be a one time initialize leak, as I ran several autocomplete 
urls (all bogus) and saw only one instance in Purify's output.

Comment 3

[Tanu Mutreja]

Attachment: Proposed Changes...

Comment 4

[Alec Flett]

Comment on attachment 106195 [details] [diff] [review]
Proposed Changes...

the out-of-memory ones are good, but the reason for the kungFuDeathGrip is to
make sure that the enumerator is deleted automatically when kungFuDeathGrip
goes out of scope. You should not delete it manually.

Comment 5

[Daniel Bratell]

Comment on attachment 106195 [details] [diff] [review]
Proposed Changes...

Why not rename kungFuDeathGrip enumeratorOwner to make that clear?

(The deathgrip didn't exist when this leak was discovered. It was checked in
for bug 161701 according to CVS blame.)

Comment 6

[Pascal Chevrel:pascalc (PTO until Sept 2)]

*** Bug 152055 has been marked as a duplicate of this bug. ***

Comment 7

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: patch

I don't understand how kungFooDeathGrip works, and I don't have Purify, so I
don't know if this is right.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 191822 [details] [diff] [review]
patch

why make the constructor public?

also, this patch won't fix any leaks. but I guess the addition of the
kungFuDeathGrip fixed it.

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

er, the destructor. (does it need to be virtual?)

Comment 10

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

<NeilZZZ> biesi: anyway, I think that bug was "fixed" by alecf as part of bug 161701

stephend: can you see if the leak still exists?

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

No.  I don't have Purify.

Comment 12

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Attachment: add some null checks

Comment 13

[Martijn Wargers (dead)]

Crhis, maybe you should ask someone else to review the patch. Or maybe the patch isn't necessary anymore?

Comment 14

[Ryan VanderMeulen [:RyanVM]]

This code doesn't exist anymore.