JS Component Registration leaks atoms

RESOLVED FIXED

Status

()

Core
XPConnect
RESOLVED FIXED
16 years ago
9 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

Trunk
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 1 obsolete attachment)

(Assignee)

Description

16 years ago
Ok, so I noticed that running xpcshell after a full build resulted in something
like this:
JS engine warning: 1 atoms remain after destroying the JSRuntime.
                   These atoms may point to freed memory. Things reachable
                   through them have not been finalized.
but running it again after running xpcshell didn't give that output.

My guess was that the problem was js components, but i wasn't sure, so my first
check was of non components, and my second was of js components (I was going to
check c++ components last but i didn't need to).

Here's what I ran:
I:\build\mozilla\debug-i686-pc-cygwin\dist\bin>echo quit() > quit.js

I:\build\mozilla\debug-i686-pc-cygwin\dist\bin>for %a in (components\*.js)  do
@(echo %a && touch %a && xpcshell quit.js)

Expected output is:
components\<filename.js>
Type Manifest File:
I:\build\mozilla\debug-i686-pc-cygwin\dist\bin\components\xpti.dat
+++ JavaScript debugging hooks installed.
nsNativeComponentLoader: autoregistering begins.
nsNativeComponentLoader: autoregistering succeeded
*** Registering <whatever it registered>
nNCL: registering deferred (0)
+++ JavaScript debuging hooks removed.
<nothing past this point>
(Assignee)

Comment 1

16 years ago
Created attachment 112082 [details]
for %a in (components\*.js)  do @(echo %a && touch %a && xpcshell quit.js)
(Assignee)

Comment 2

16 years ago
fwiw, you can substitute |regxpcom| for |xpcshell quit.js|
Status: NEW → ASSIGNED
(Assignee)

Comment 3

16 years ago
watches:
	he->keyHash	2083175
	(void*)2083175	0x001fc967
	(void*)keyHash	0x001fc967
Stack from allocation:
JS_HashTableRawAdd(JSHashTable * 0x0046d4c0, JSHashEntry * * 0x01007810,
unsigned long 2083175, const void * 0x00fe4b38, void * 0x00491720) line 246
js_GetStringBytes(JSString * 0x00fe4b38) line 2876 + 25 bytes
JS_GetStringBytes(JSString * 0x00fe4b38) line 3607 + 9 bytes
pref_HashJSPref(unsigned int 2, long * 0x00ff9eb0, PrefAction PREF_SETDEFAULT)
line 1307 + 14 bytes
pref_NativeDefaultPref(JSContext * 0x0047e210, JSObject * 0x00fe41a8, unsigned
int 2, long * 0x00ff9eb0, long * 0x0012ec68) line 1102 + 15 bytes
js_Invoke(JSContext * 0x0047e210, unsigned int 2, unsigned int 0) line 839 + 23
bytes
js_Interpret(JSContext * 0x0047e210, long * 0x0012f5dc) line 2803 + 15 bytes
js_Execute(JSContext * 0x0047e210, JSObject * 0x00fe41a8, JSScript * 0x010214c0,
JSStackFrame * 0x00000000, unsigned int 0, long * 0x0012f5dc) line 1020 + 13 bytes
JS_EvaluateUCScriptForPrincipals(JSContext * 0x0047e210, JSObject * 0x00fe41a8,
JSPrincipals * 0x00000000, const unsigned short * 0x01012028, unsigned int
26796, const char * 0x00000000, unsigned int 0, long * 0x0012f5dc) line 3382 +
25 bytes
JS_EvaluateUCScript(JSContext * 0x0047e210, JSObject * 0x00fe41a8, const
unsigned short * 0x01012028, unsigned int 26796, const char * 0x00000000,
unsigned int 0, long * 0x0012f5dc) line 3364 + 35 bytes
JS_EvaluateScript(JSContext * 0x0047e210, JSObject * 0x00fe41a8, const char *
0x00ff35a0, unsigned int 26796, const char * 0x00000000, unsigned int 0, long *
0x0012f5dc) line 3331 + 33 bytes
PREF_EvaluateConfigScript(const char * 0x00ff35a0, unsigned int 26796, const
char * 0x00000000, int 0, int 1, int 0) line 486 + 34 bytes
openPrefFile(nsIFile * 0x0046c9e0, int 0, int 0, int 0) line 455 + 25 bytes
pref_InitInitialObjects() line 591 + 21 bytes
PREF_Init(const char * 0x00000000) line 362 + 5 bytes
nsPrefService::Init() line 127 + 7 bytes
nsPrefServiceConstructor(nsISupports * 0x00000000, const nsID & {...}, void * *
0x0012f898) line 48 + 125 bytes
nsGenericFactory::CreateInstance(nsGenericFactory * const 0x0047e6b0,
nsISupports * 0x00000000, const nsID & {...}, void * * 0x0012f898) line 83 + 21
bytes
nsComponentManagerImpl::CreateInstanceByContractID(nsComponentManagerImpl *
const 0x00416990, const char * 0x03853254, nsISupports * 0x00000000, const nsID
& {...}, void * * 0x0012f898) line 1875 + 24 bytes
nsComponentManagerImpl::GetServiceByContractID(nsComponentManagerImpl * const
0x00416994, const char * 0x03853254, const nsID & {...}, void * * 0x0012f8fc)
line 2274 + 50 bytes
nsGetServiceByContractID::operator()(const nsID & {...}, void * * 0x0012f8fc)
line 121 + 38 bytes
nsCOMPtr<nsIPrefService>::assign_from_helper(const nsCOMPtr_helper & {...},
const nsID & {...}) line 922 + 18 bytes
nsCOMPtr<nsIPrefService>::nsCOMPtr<nsIPrefService>(const nsCOMPtr_helper &
{...}) line 554
nsScriptSecurityManager::InitPrefs() line 3048 + 30 bytes
nsScriptSecurityManager::nsScriptSecurityManager() line 2545
nsScriptSecurityManager::GetScriptSecurityManager() line 2587 + 27 bytes
Construct_nsIScriptSecurityManager(nsISupports * 0x00000000, const nsID & {...},
void * * 0x0012faa4) line 336 + 5 bytes
nsGenericFactory::CreateInstance(nsGenericFactory * const 0x0047b7a0,
nsISupports * 0x00000000, const nsID & {...}, void * * 0x0012faa4) line 83 + 21
bytes
nsComponentManagerImpl::CreateInstanceByContractID(nsComponentManagerImpl *
const 0x00416990, const char * 0x02cae9e4 kScriptSecurityManagerContractID,
nsISupports * 0x00000000, const nsID & {...}, void * * 0x0012faa4) line 1875 +
24 bytes
nsComponentManagerImpl::GetServiceByContractID(nsComponentManagerImpl * const
0x00416994, const char * 0x02cae9e4 kScriptSecurityManagerContractID, const nsID
& {...}, void * * 0x0012fb08) line 2274 + 50 bytes
nsGetServiceByContractID::operator()(const nsID & {...}, void * * 0x0012fb08)
line 121 + 38 bytes
nsCOMPtr<nsIScriptSecurityManager>::assign_from_helper(const nsCOMPtr_helper &
{...}, const nsID & {...}) line 922 + 18 bytes
nsCOMPtr<nsIScriptSecurityManager>::nsCOMPtr<nsIScriptSecurityManager>(const
nsCOMPtr_helper & {...}) line 554
mozJSComponentLoader::ReallyInit() line 494 + 28 bytes
mozJSComponentLoader::ModuleForLocation(const char * 0x0047b9f0, nsIFile *
0x0047bf00) line 889 + 23 bytes
mozJSComponentLoader::AttemptRegistration(nsIFile * 0x0047bf00, int 0) line 735
+ 25 bytes
mozJSComponentLoader::AutoRegisterComponent(mozJSComponentLoader * const
0x0047bde0, int 0, nsIFile * 0x0047bf00, int * 0x0012fdf8) line 661 + 14 bytes
mozJSComponentLoader::RegisterComponentsInDir(int 0, nsIFile * 0x00448600) line
569 + 24 bytes
mozJSComponentLoader::AutoRegisterComponents(mozJSComponentLoader * const
0x0047bde0, int 0, nsIFile * 0x00448600) line 526
nsComponentManagerImpl::AutoRegisterImpl(int 0, nsIFile * 0x00000000, int 1)
line 3087 + 50 bytes
nsComponentManagerImpl::AutoRegister(nsComponentManagerImpl * const 0x00416998,
nsIFile * 0x00000000) line 3229 + 19 bytes
main(int 1, char * * 0x00414340) line 195 + 25 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e87903()

I don't have the stack or when we complain about it atm but you set a breakpoint
for js_FinishAtomState inside the if (args.leaks != 0). and then you set a
breakpoint at free(he) in js_free_atom for (*(char*)he->key!=0). The he->keyHash
is the same in both places so that's the condition for the breakpoint to catch
the creation.
(Assignee)

Comment 4

16 years ago
Created attachment 112101 [details]
stack for the leak complaint
(Assignee)

Comment 5

16 years ago
Created attachment 112102 [details]
the right stack for the hashkey

ok. ignore the stack i pasted in the bug. it can't possibly be right because
only some js components trigger the complaint
Created attachment 112692 [details] [diff] [review]
leak fix in AutoRegisterComponent
Attachment #112692 - Flags: superreview?(bzbarsky)
Attachment #112692 - Flags: review?(dougt)
Attachment #112692 - Flags: superreview?(bzbarsky) → superreview+
(Assignee)

Comment 7

16 years ago
Created attachment 112693 [details]
the leak tree dbaron used to find the problem
(Assignee)

Comment 8

16 years ago
Created attachment 112695 [details] [diff] [review]
demo patch (Backout jband  1.36) in conjunction w/ dbaron's stops the component manager from being held past shutdown

In order to get stop the component manager from being held past shutdown, we
need to undo jband's change for 1.36 of nsXPConnect.cpp (this patch). With that
additional change, I no longer get the jsatom leaks, unfortunately i get a
jsusage error:
JS API usage error: 1 contexts left in runtime upon JS_DestroyRuntime.
JS engine warning: leaking GC root 'res->input' at 0x81073c8

I've played with those before. The warning means that a context is left. I'll
try to spend some time this week figuring out what the context is and where it
should be released.
(Assignee)

Updated

16 years ago
Attachment #112695 - Attachment is obsolete: true

Comment 9

16 years ago
Comment on attachment 112692 [details] [diff] [review]
leak fix in AutoRegisterComponent

yuck.  loaders should be factored out a bit so that we dont have init
constructs like this.

thanks for fixing this leak.
Attachment #112692 - Flags: review?(dougt) → review+
(Assignee)

Updated

16 years ago
Attachment #112692 - Flags: approval1.3b?

Comment 10

16 years ago
Comment on attachment 112692 [details] [diff] [review]
leak fix in AutoRegisterComponent

a=asa (on behalf of drivers) for checkin to 1.3beta.
Attachment #112692 - Flags: approval1.3b? → approval1.3b+
Comment on attachment 112692 [details] [diff] [review]
leak fix in AutoRegisterComponent

This was checked in to the trunk, 2003-01-27 17:42 PST.  Leaving open for the
other leaks timeless is seeing.
QA Contact: pschwartau → xpconnect
Leaving it open doesn't seem to have had the desired effect, and all his open bugs untouched for six years with an r+ patch in them just make my life more difficult trying to find his forgotten and never checked in patches.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.