please include patch for a highly visible crash in epiphany

[c7d2f5c8667d26fffd5e7772d632c76d]

The patch attached to http://bugzilla.mozilla.org/show_bug.cgi?id=241535 fixes a crash on plugin destruction, http://bugzilla.gnome.org/show_bug.cgi?id=354790 which has many duplicates, almost all of which are from ubuntu edgy users.

[Braden (braden)]

Created attachment 146899
A test XEmbed plug-in

This attachment demonstrates the GLib assertion failure.

[Braden (braden)]

I suspect this assertion failure indicates a situation that can give rise to
memory corruption or somesuch insidiousness. The plug-in I'm actually trying to
write is crashing; and I strongly suspect that crash is linked to this assertion
failure.

[Braden (braden)]

Strike comment #2. There was a shutdown problem in my GTK widget that just
hadn't surfaced in other contexts.

I'll leave this bug open for now. The GLib assertion failure is real; and if it
is in fact benign, then something should probably be done to quell it.

[Blizzard (blizzard)]

What's the assertion, exactly?

[c7d2f5c8667d26fffd5e7772d632c76d (c7d2f5c8667d26fffd5e7772d632c76d-deactivatedaccount)]

I think I'm seeing the same problem with the totem plugin: the plugin embedded viewer crashes, and when closing the browser (shutdown), I get this:

(epiphany:24898): Gtk-CRITICAL **: gtk_widget_destroy: assertion `GTK_IS_WIDGET (widget)' failed

Trace:
#3 0xb6ed0c1a in g_return_if_fail_warning (log_domain=0xbf8697bb "trap", pretty_function=0xb7b43a17 "gtk_widget_destroy", expression=0xbf8697bb "trap")
    at gmessages.c:532
#4 0xb7a5e3a7 in gtk_widget_destroy (widget=0x8625e98) at gtkwidget.c:2156
#5 0xb64b7ae0 in ~nsPluginNativeWindowGtk2 (this=0x8800778) at nsPluginNativeWindowGtk2.cpp:91
#6 0xb64b7b2c in PLUG_DeletePluginNativeWindow (aPluginNativeWindow=0xb7ee3358) at nsPluginNativeWindowGtk2.cpp:107
#7 0xb64a2287 in nsPluginHostImpl::DeletePluginNativeWindow (this=0x8779818, aPluginNativeWindow=0xbf8697bb) at nsPluginHostImpl.cpp:6529
#8 0xb602e5e8 in ~nsPluginInstanceOwner (this=0x87091d8) at nsObjectFrame.cpp:2363
#9 0xb602cc33 in nsPluginInstanceOwner::Release (this=0xb7ee3358) at nsObjectFrame.cpp:2373
#10 0xb64b27a6 in ~nsPluginInstancePeerImpl (this=0x87fa110) at nsPluginInstancePeer.cpp:78
#11 0xb64b3539 in nsPluginInstancePeerImpl::Release (this=0xb7ee3358) at nsPluginInstancePeer.cpp:91
#12 0xb64a5144 in ~nsActivePlugin (this=0x8895180) at nsPluginHostImpl.cpp:413
#13 0xb64a51fd in nsActivePluginList::remove (this=0x8779848, plugin=0x8895180) at nsPluginHostImpl.cpp:532
#14 0xb64a53db in nsActivePluginList::shut (this=0x8779848) at nsPluginHostImpl.cpp:457
#15 0xb64ac2bf in nsPluginHostImpl::Destroy (this=0x8779818) at nsPluginHostImpl.cpp:3214
#16 0xb64a3ebf in nsPluginHostImpl::Observe (this=0x8779818, aSubject=0x81b51dc, aTopic=0xbf8697bb "trap", someData=0x0) at nsPluginHostImpl.cpp:6141
#17 0xb6e270c3 in nsObserverService::NotifyObservers (this=0x82742b0, aSubject=0x81b51dc, aTopic=0xb6e80935 "xpcom-shutdown", someData=0x0)
    at nsObserverService.cpp:233
#18 0xb6e1f196 in NS_ShutdownXPCOM_P (servMgr=0x0) at nsXPComInit.cpp:797
#19 0xb7f3d77d in NS_ShutdownXPCOM (svcMgr=0xbf8697bb) at nsXPComStub.cpp:140
#20 0xb7f4d068 in NS_TermEmbedding () at nsEmbedAPI.cpp:215
[...]

[Braden (braden)]

I wasn't able to respond to Chris' query because this problem stopped showing up for me. I wish I could say exactly what did it; but I was making frequent dramatic changes to my plug-in at the time and I really can't say with any certainty what made this assertion go away.

I do have a suspicion, though. If you look at the test case I attached (which I haven't compiled and run for two years, BTW), a significant thing about it is that it creates the GtkPlug in-process. Some time ago I switched to an out-of-process GtkPlug in my plug-in; and a lot of things just started working better.

Is totem using an in-process GtkPlug?

[c7d2f5c8667d26fffd5e7772d632c76d (c7d2f5c8667d26fffd5e7772d632c76d-deactivatedaccount)]

(In reply to comment #6)
> Is totem using an in-process GtkPlug?

No, the totem plugin starts a separate process that handles the content.

[c7d2f5c8667d26fffd5e7772d632c76d (c7d2f5c8667d26fffd5e7772d632c76d-deactivatedaccount)]

Created attachment 241087
proposed patch

It doesn't seem to make any difference whether the plugin actually displays anything in the socket; a test plugin which just does nothing triggers the same warning for me.
The problem seems to be that mGtkSocket is destroyed when the tab is closed (or the page unloaded?), but the nsPluginNativeWindowGtk2 seems to live longer than that (a leak? or expected?).

[c7d2f5c8667d26fffd5e7772d632c76d (c7d2f5c8667d26fffd5e7772d632c76d-deactivatedaccount)]

The patch attached to http://bugzilla.mozilla.org/show_bug.cgi?id=241535 fixes a crash on plugin destruction, http://bugzilla.gnome.org/show_bug.cgi?id=354790 which has many duplicates, almost all of which are from ubuntu edgy users.

[towsonu2003 (towsonu2003)]

linked bugs from both mozilla and gnome, the latter for documentation purposes. thanks for reporting.

[towsonu2003 (towsonu2003)]

note to devs: bug reporter is an epiphany developer.

[Daniel Holbach (dholbach)]

Hum... trying to apply the patch to the Ubuntu Edgy firefox source (1.99+2.0b2+dfsg-1ubuntu3), I get this:

c++ -o nsStringAPI.o -c -I../../dist/include/system_wrappers -include ../../config/gcc_hidden.h -DMOZILLA_INTERNAL_API -DOSTYPE=\"Linux2.6\" -DOSARCH=\"Linux\" -DBUILD_ID=0000000000 -D_IMPL_NS_COM -D_IMPL_NS_STRINGAPI -DEXPORT_XPT_API -DEXPORT_XPTC_API -DEXPORT_XPTI_API -I. -I.. -I./../glue -I./../base -I./../ds -I./../io -I./../components -I./../threads -I./../threads/_xpidlgen -I./../proxy/src -I../../dist/include/string -I../../dist/include/xpcom -I../../dist/include -I../../dist/include/nspr -I/usr/include -I/usr/include -I/usr/include -fPIC -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -fshort-wchar -pthread -pipe -DNDEBUG -DTRIMMED -pipe -w -O2 -g -fno-strict-aliasing -DMOZILLA_CLIENT -include ../../mozilla-config.h -Wp,-MD,.deps/nsStringAPI.pp nsStringAPI.cpp
rm -f libxpcom_core.so
c++ -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -fshort-wchar -pthread -pipe -DNDEBUG -DTRIMMED -pipe -w -O2 -g -fno-strict-aliasing -fPIC -shared -Wl,-z,defs -Wl,-h,libxpcom_core.so -o libxpcom_core.so pldhash.o nsCOMPtr.o nsComponentManagerUtils.o nsDebug.o nsID.o nsIInterfaceRequestorUtils.o nsINIParser.o nsMemory.o nsTraceRefcnt.o nsWeakReference.o nsGREGlue.o nsVersionComparator.o nsTHashtable.o nsTArray.o nsGenericFactory.o nsXPComInit.o nsStringAPI.o -Wl,--as-needed -Wl,--whole-archive ../../dist/lib/libxpcomds_s.a ../../dist/lib/libxpcomio_s.a ../../dist/lib/libxpcomcomponents_s.a ../../dist/lib/libxpcomthreads_s.a ../../dist/lib/libxpcomproxy_s.a ../../dist/lib/libxpcombase_s.a ../../dist/lib/libxptcall.a ../../dist/lib/libxptinfo.a ../../dist/lib/libxpt.a ../../dist/lib/libxptcmd.a ../../dist/lib/libstring_s.a -Wl,--no-whole-archive -L../../dist/lib -lplds4 -lplc4 -lnspr4 -lpthread -ldl -ldl -lm
/usr/bin/ld: nsCOMPtr.o: relocation R_X86_64_PC32 against `nsGetServiceByContractIDWithError::operator()(nsID const&, void**) const' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: Bad value
collect2: ld returned 1 exit status
make[4]: *** [libxpcom_core.so] Error 1
make[4]: Leaving directory `/tmp/buildd/firefox-1.99+2.0b2+dfsg/xpcom/build'
make[3]: *** [libs] Error 2
make[3]: Leaving directory `/tmp/buildd/firefox-1.99+2.0b2+dfsg/xpcom'
make[2]: *** [tier_2] Error 2
make[2]: Leaving directory `/tmp/buildd/firefox-1.99+2.0b2+dfsg'
make[1]: *** [default] Error 2
make[1]: Leaving directory `/tmp/buildd/firefox-1.99+2.0b2+dfsg'
make: *** [build-stamp] Error 2
pbuilder: Failed autobuilding of package

[c7d2f5c8667d26fffd5e7772d632c76d (c7d2f5c8667d26fffd5e7772d632c76d-deactivatedaccount)]

The build error isn't related to the patch. It looks more like bug #61104 to me, i.e. due to symbol hiding not working correctly with edgy's gcc...

[Ian Jackson (ijackson)]

I will attempt to apply this patch myself and see what it does for me, next time I'm doing a firefox version.

[Ian Jackson (ijackson)]

I have applied the suggested patch (attachment 241087 from the Mozilla Bugzilla).

[Reinout van Schouwen (reinout-gmail)]

Is someone able to review the proposed patch?

[Timeless-bemail (timeless-bemail)]

Comment on attachment 241087
proposed patch

doubtful, we lack gtk experts

[Caillon (caillon)]

Comment on attachment 241087
proposed patch

r=caillon, but there should be a bug on file to find out why nsPluginNativeWindowGtk2 is not getting destroyed properly. This patch should be reverted when that is fixed. A FIXME comment would be great, as well.

[Karlt (karlt)]

Is this assertion still firing on trunk?

[c7d2f5c8667d26fffd5e7772d632c76d (c7d2f5c8667d26fffd5e7772d632c76d-deactivatedaccount)]

Comment on attachment 241087
proposed patch

Bug 397992 fixed the crash/critical warning in a different but equivalent way, so I think this doesn't crash anymore. Still, it would be good to figure out why this object is only destroyed on shutdown (comment 11)...