60709 - allow dll unloading at shutdown

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect, P3)

Description

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

This bug is meant to be a more doable first (baby) step to solving bug 45613.

Currently the code to unload shared libraries (the call to nsDll::Unload from
nsDll::~nsDll) is disabled.  If the current code is enabled, it causes shared
libraries to be unloaded at shutdown, almost at the very end of the shutdown
sequence.  I've been investigating what happens when this code is enabled, so I
thought I'd file a bug to cover what needs to be done to enable it (at least on
Linux).

In a very simple start-and-exit case with the unloading at shutdown code
enabled, I crash in exit() when exit() tries to call some of the functions that
have been given to it by atexit().  (I was somewhat impressed I made it this
far.)  In particular, every static object with a destructor created in function
scope leads to a call to atexit that is no longer valid once the library has
been unloaded.  I'll attach a list of the atexit() calls that were made in a
simple start-and-exit run of the browser.

In more complicated scenarios, I suspect we'd have much more trouble with things
like static nsCOMPtrs (see bug 60697) introducing weird dependencies that aren't
cleaned out by module destructors.  (I'm assuming that global static objects
have their destructors called when the library is unloaded, but I haven't tested
this by hand yet.)  I know from investigating the leak stats that on a simple
start-and-exit we don't have any XPCOM objects held by static nsCOMPtrs by the
time the leak stats are printed.

Comment 1

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

Attachment: calls to atexit starting and exiting browser

Comment 2

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

If I get rid of the other problems, GDK crashes for us:

#0  0x400097b8 in _dl_lookup_versioned_symbol (
    undef_name=0x4098cf38 "shutdown", undef_map=0x80ac578, ref=0xbffff568, 
    symbol_scope=0x80ac6cc, version=0x80ac460, reloc_type=7, explicit=0)
    at do-lookup.h:68
#1  0x4000ceb0 in fixup (l=0x80ac578, reloc_offset=0) at dl-runtime.c:89
#2  0x4000d030 in _dl_runtime_resolve () at dl-runtime.c:203
#3  0x409d2951 in _X11TransDisconnect () from /usr/X11R6/lib/libX11.so.6
#4  0x40999afc in _XDisconnectDisplay () from /usr/X11R6/lib/libX11.so.6
#5  0x4099905f in XCloseDisplay () from /usr/X11R6/lib/libX11.so.6
#6  0x4091c2aa in gdk_exit_func () from /usr/lib/libgdk-1.2.so.0
#7  0x4037efff in exit (status=0) at exit.c:57
#8  0x4036ba85 in __libc_start_main (main=0x8054b94 <main>, argc=1, 
    ubp_av=0xbffff6bc, init=0x804f0c4 <_init>, fini=0x8060c90 <_fini>, 
    rtld_fini=0x4000d684 <_dl_fini>, stack_end=0xbffff6b4)
    at ../sysdeps/generic/libc-start.c:111

Comment 3

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

Attachment: some more calls to atexit()

Comment 4

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

I filed http://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=23552 about gcc's
handling of function-scope static destructors.

Comment 5

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

Since I'm the only person commenting on / working on this bug, why don't I just
assign it to myself.  I probably should have done that when I filed it.

That's not to say I think I'm going to succeed...

Comment 6

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

Using the gcc option -fuse-cxa-atexit fixes gcc's messed up behavior.

Comment 7

[Doug Turner (:dougt)]

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

Comment 8

[Doug Turner (:dougt)]

dbaron, I have been running this code for quite a while, and haven't seen
anything really bad at shutdown.  (debug only)

Maybe we should try something like:

Index: xcDll.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpcom/components/xcDll.cpp,v
retrieving revision 1.60
diff -u -r1.60 xcDll.cpp
--- xcDll.cpp	21 Jun 2002 19:58:47 -0000	1.60
+++ xcDll.cpp	12 Jul 2002 19:45:30 -0000
@@ -152,7 +152,7 @@
 
 nsDll::~nsDll(void)
 {
-#if DEBUG_dougt
+    //#if DEBUG_dougt
     // The dll gets deleted when the dllStore is destroyed. This happens on
     // app shutdown. At that point, unloading dlls can cause crashes if we have
     // - dll dependencies
@@ -160,7 +160,7 @@
     // - static dtors
     // Hence turn it back on after all the above have been removed.
     Unload();
-#endif
+    //#endif
     if (m_dllName)
         nsCRT::free(m_dllName);
     if (m_registryLocation)

Comment 9

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

Have you been running with it on Linux?

Comment 10

[Doug Turner (:dougt)]

yeah.  not as much as windows, but I don't see any crashes at shutdown.

Comment 11

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

Hmm.  I really thought that:
 1. we still had some function-scope static variables in component libraries
with destructors, which gcc runs atexit().
 2. gtk+/gdk runs stuff atexit(), and we only load gtk+ through component
libraries linking with it.

Perhaps newer versions of gtk+/gdk don't bother calling at empty function
atexit(), and perhaps we've eliminated the static destructors in question.

Comment 12

[Bradley Baetz (:bbaetz)]

-fuse-cxa-atexit is probably going to be on by default for g++ >= 3.2 since its
a requirement for the common ABI; see http://gcc.gnu.org/gcc-3.2/c++-abi.html

Comment 13

[Judson Valeski]

per discussion w/ dougt, topembed minusing this bug.

Comment 14

[Michael Kohler [:mkohler]]

This is a mass change. Every comment has "assigned-to-new" in it.

I didn't look through the bugs, so I'm sorry if I change a bug which shouldn't be changed. But I guess these bugs are just bugs that were once assigned and people forgot to change the Status back when unassigning.

Comment 15

[Benjamin Smedberg]

At this point I don't think we need to support XPCOM binary component unloading: restartless extensions shouldn't be using them, and unloading would introduce risk of instability for little/no reward.