Serge Gautherie (:sgautherie) Reporter
	Description • 16 years ago

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20080911182230 Minefield/3.1b1pre] (home, optim default) (W2Ksp4)

Either call to |env.set("XPCOM_DEBUG_BREAK", "...");| leak "the" string.

Commenting out both of them is enough to stop the leak.
Fwiw, adding a |env = null;| doesn't help.

It would seem we miss some "shutdown release" either in the test or in the |nsIEnvironment| service !?
http://mxr.mozilla.org/mozilla-central/source/xpcom/threads/nsIEnvironment.idl

*****

Note:

The end of the test resets the value to "abort" instead of its initial value of "stack-and-abort" :-/
I think the value should rather be |env.get("XPCOM_DEBUG_BREAK")| then restored !

|// If we've gotten this far, then the test has passed.| makes me wondering:
does it mean that if the test should fail, the value wouldn't be "restored" ?

	Dave Townsend [:mossop] Assignee
	Comment 1 • 16 years ago

xpcshell tests each run in their own process don't they, so resetting the environment variable doesn't really serve much purpose.

	Dave Townsend [:mossop] Assignee
	Comment 2 • 16 years ago

Given that the actual test is not leaking, just something we have to do to make the test pass (due to other bugs) I don't believe that this is a particular issue.

	Alex Vincent [:WeirdAl]
	Comment 3 • 16 years ago

Dave, did we ever get a bug filed for the assertions themselves?  That's a legitimate bug in itself.

Also, suppose we're genuinely leaking when we set an environment variable from JavaScript? Serge, I'd say write an independent xpcshell leak test for precisely that, involving a harmless environment variable (MOZ_FOO).  See if that leaks, or if it's specific to XPCOM_DEBUG_BREAK.  Either way, it wouldn't be an extension manager bug.

	Dave Townsend [:mossop] Assignee
	Comment 4 • 16 years ago

(In reply to comment #3)
> Dave, did we ever get a bug filed for the assertions themselves?  That's a
> legitimate bug in itself.

According to the comments you were going to file but then bsmedberg indicated that it was a known bug.

	Dave Townsend [:mossop] Assignee
	Comment 5 • 16 years ago

The environment change is no longer necessary as the assertions have since been fixed anyway.

	Robert Strong (they/them - no direct email)
	Comment 6 • 16 years ago

Comment on attachment 338200 [details] [diff] [review]
fix
[Checkin: Comment 8]

r=me -  I also verified we don't assert on Windows with a debug build.

	Serge Gautherie (:sgautherie) Reporter
	Comment 7 • 16 years ago

(In reply to comment #3)

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20080912014358 Minefield/3.1b1pre] (home, optim default) (W2Ksp4)

Independent (leaking) test.

|env XPCOM_MEM_LEAK_LOG=1 make SOLO_FILE="test_bug454842.js" -C objdir/xpcom/tests check-one|

*****

http://mxr.mozilla.org/mozilla-central/source/xpcom/threads/nsEnvironment.cpp?mark=135-137,164#134
says the leak is "by design" :-/

I don't know if anything can be done about any aspect of this bug ? :-<

	Dave Townsend [:mossop] Assignee
	Comment 8 • 16 years ago

Landed the test fix http://hg.mozilla.org/mozilla-central/rev/bceda53ebeb7

I suggest that either this bug is now fixed since the leak in the testcase is gone now, or it is a problem with the environment service in which case it should be moved elsewhere.

	Serge Gautherie (:sgautherie) Reporter
	Comment 9 • 16 years ago

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20080917032624 Minefield/3.1b1pre] (home, optim default) (W2Ksp4)

|env XPCOM_MEM_LEAK_LOG=1 make SOLO_FILE="test_bug299716.js" -C objdir/toolkit/mozapps/extensions/test check-one|

V.Fixed

	Serge Gautherie (:sgautherie) Reporter
	Comment 10 • 16 years ago

(In reply to comment #7)
> I don't know if anything can be done about any aspect of this bug ? :-<

(In reply to comment #8)
> it should be moved elsewhere.

I added a comment to bug 203764...