28569 - nsLoadCookie::Init leaking an nsDocLoaderImpl

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

DESCRIPTION:  nsLoadCookie::Init is leaking the nsDocLoaderImpl parentDocLoader
the first time it is run, because nsDocLoaderImpl's don't go away until their
::Destroy() is called.  I'm not sure what the right way to fix this is.

This causes (at least) a leak of 1 nsDocLoaderImpl and 1 nsLoadGroup.

STEPS TO REPRODUCE:
 * start mozilla (with whatever leak logging you like)
 * at profile manager, hit exit

ADDITIONAL INFORMATION:
I tried a patch to call Destroy(), and it worked to fix the leak, except it
caused a crash-on-exit.  I'm not exactly sure how do_GetService() works.  The
time that the leak occurs, do_GetService() creates a new nsDocLoaderImpl, keeps
a reference to it in its own hashtable, and addrefs it for the caller (and the
nsDocLoaderImpl also gets a reference from its loadgroup).  Will it then return
the same one in the future?  (In other words, I don't know how to fix this,
although it would seem like things would work if nsLoadGroup's only kept weak
references to the nsDocLoaderImpl's.)

Anyway, I'll attach the patch that caused a crash-on-exit with the stack trace:
#0  0x40e75c0f in nsDocLoaderImpl::RemoveChildGroup (this=0x408dc6f0, 
    aLoader=0x40873818) at nsDocLoader.cpp:452
#1  0x40e758bb in nsDocLoaderImpl::Destroy (this=0x40873818)
    at nsDocLoader.cpp:354
#2  0x4028269f in nsWebShell::Destroy (this=0x40894368) at nsWebShell.cpp:3648

I'll also attach some refcount balancer logs.

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: patch that fixes leak *AND* causes crash on exit, for informational purposes only :-)

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Attachment: refcount balancer log for leaked nsDocLoaderImpl after hitting "Exit" in profile manager

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

If one actually starts mozilla rather than hit exit, the refcount balancer log
looks similar if I use --ignore-balanced-subtrees (no other net leaks/releases),
but if I don't, the same nsDocLoaderImpl is used in other places.  I guess
that's why the patch causes a crash.

Comment 4

[Stephen P. Morse]

This code was written by mscott.  Reassigning.

Comment 5

[Scott MacGregor]

We were leaking the global doc loader due to a circular reference that wasn't
broken in the case where the doc loader was created by the service manager.

Fix checked in.

Comment 6

[Bruce Mitchener]

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

Comment 7

[Tom Everingham]

verified:
Linux 2000070420