633457 - Firefox hangs at startup

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Ginn Chen]

It's not 100% reproducible.
It happens if $HOME/.mozilla doesn't exist and memory is low (maybe).

I see this problem a lot with LiveCD on VirtualBox or VMWare.

Stack attached.

Comment 1

[Benjamin Smedberg]

Stack?

Comment 2

[Ginn Chen]

Attachment: hang stack

Comment 3

[Ginn Chen]

In lwp# 1, it is shutting down sCollectorThread. sCollectorRunner should have already been shutdown.

Why do we still have lwp# 3 ?

Comment 4

[Ginn Chen]

I believe the reason is
lwp# 1 did Shutdown() before lwp # 3 setting mRunning to TRUE.

i.e.

lwp# 1 returns from Shutdown() at
http://hg.mozilla.org/mozilla-central/file/1ed3464aaa92/xpcom/base/nsCycleCollector.cpp#l3372
then
lwp#3 starts running at
http://hg.mozilla.org/mozilla-central/file/1ed3464aaa92/xpcom/base/nsCycleCollector.cpp#l3302

Comment 5

[Ginn Chen]

Attachment: patch

Comment 6

[Benjamin Smedberg]

Yeah, this looks like a recent regression from the cycle-collector threading stuff.

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Hm, why are we shutting down so early?

Patch looks good btw.

Comment 8

[Ginn Chen]

(In reply to comment #7)
> Hm, why are we shutting down so early?

Because we're restarting.

Comment 9

[Mike Shaver (:shaver emeritus)]

Does this bug only affect OpenSolaris?

Comment 10

[Ginn Chen]

In theory, it affects all platforms.
It is a race condition.

Comment 11

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #8)
> (In reply to comment #7)
> > Hm, why are we shutting down so early?
> 
> Because we're restarting.

Huh, I thought we got rid of all that restart-on-startup stuff...

Comment 12

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Ginn, I'd be happy to review this, but can you attach a patch with -P and a little more context? 'diff -U8 -P' is what i usually use.

Comment 13

[Ben Turner (not reading bugmail, use the needinfo flag!)]

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

This looks fine to me! Thanks.

Comment 14

[Ginn Chen]

(In reply to comment #12)
> Ginn, I'd be happy to review this, but can you attach a patch with -P and a
> little more context? 'diff -U8 -P' is what i usually use.

Sorry, I made the patch in VM so I missed my .hgrc.

Comment 15

[Mike Shaver (:shaver emeritus)]

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

I think dbaron wanted bent||peterv, rather than bent&&peterv, so clearing other review req.  I'll push this soon.

Comment 16

[Mike Shaver (:shaver emeritus)]

http://hg.mozilla.org/mozilla-central/rev/afba7603a946