239875 - Fix xpconnect static guards so we can restart

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P1)

Description

[Benjamin Smedberg]

XPconnect currently has static guards to prevent it from restarting. I have a
patch that makes xpconnect restartable (so that I can restart XPCOM for the
semi-single-profile proposal, see attachment 144252 [details]).

I'm not trying to make xpconnect shut down cleanly in this patch... only when we
restart xpconnect through the generic-module-constructor, we re-initialize all
the static. We will end up leaking a couple of xpconnect implementations, but
that can't be helped without reworking the entire xpcom shutdown sequence (bug
180380).

Comment 1

[Benjamin Smedberg]

Attachment: Init static guards on xpconnect restart

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 145610 [details] [diff] [review]
Init static guards on xpconnect restart

I think this looks fine.  If it actually made Mozilla restart xpconnect at some
point, I'd be a lot more nervous.

r=shaver

Comment 3

[David Bradley]

I'm wondering what happens to all the the maps between JS object and natives and
maps between other internal parts of XPConnect are no longer accessible, since
the runtime service has been cut loose. Depending on when this is called, and
how many wrapped objects exist, you could leak a fair amount of data in the
maps. And since the maps are gone, you'd be leaking the objects wrapped by
XPConnect since the GC would never be able to get to them. XPConnect contexts
would be leaked as well as duplicated. You'll probably be duplicating any native
sets as well. That's a few things off the top of my head.

A similar issue raised a while back by an embedder was having two JS engines
running within the same process. The statics in XPConnect presented a hurdle to
doing this, as did some other issues. I'm just wondering if it would worth
taking a look at it from that view and possibly coming up with a cleaner
solution. Unfortunately that probably doesn't meet your time frame.

Comment 4

[Benjamin Smedberg]

I don't know this code well enough to be sure why we're not leaking, but I've
instrumented leakages and the maps are definitely not holding on to lots of
stuff. The only thing we leak is one instance of xpccomponents and one
nsXPConnect, at least in the controlled circumstances of the profile manager and
migration.

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 145610 [details] [diff] [review]
Init static guards on xpconnect restart

Sure, why not?	But I bet we have other statics (perhaps you've checked for
function-scope statics?).  If we ever do decide to do a "hot restart", we
really need a more stylized convention for static data, that groups it into a
"statics" class per XPCOM module, or some such thing.

/be

Comment 6

[Benjamin Smedberg]

Fixed on trunk