993747 - nsAppRunner.cpp should use NS_FREE_PERMANENT_DATA rather than making up its own variant of the same concept

Status: NEW

(Core :: Widget: Gtk, defect, P4)

Description

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

nsAppRunner.cpp should use NS_FREE_PERMANENT_DATA rather than making up its own variant of the same concept.

Comment 1

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

Attachment: nsAppRunner.cpp should use NS_FREE_PERMANENT_DATA rather than making up its own variant of the same concept

Comment 2

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/244e4088c1b9

(and try run at https://tbpl.mozilla.org/?tree=Try&rev=8b414d954c01 )

Comment 3

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

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/901d300bb441 for Linux mochitest ASAN orange.


22:53:50     INFO -  [Parent 2643] WARNING: IA__gtk_clipboard_get_for_display: assertion `!display->closed' failed: 'glib warning', file /builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsSigHandlers.cpp, line 127
22:53:50     INFO -  (firefox:2643): Gtk-CRITICAL **: IA__gtk_clipboard_get_for_display: assertion `!display->closed' failed
22:53:50     INFO -  [Parent 2643] WARNING: IA__gtk_clipboard_get_for_display: assertion `!display->closed' failed: 'glib warning', file /builds/slave/m-in-l64-asan-0000000000000000/build/toolkit/xre/nsSigHandlers.cpp, line 127
22:53:50     INFO -  (firefox:2643): Gtk-CRITICAL **: IA__gtk_clipboard_get_for_display: assertion `!display->closed' failed
22:53:50     INFO -  firefox: fccache.c:507: FcCacheFini: Assertion `fcCacheChains[i] == ((void *)0)' failed.
22:53:50     INFO -  TEST-INFO | Main app process: killed by out-of-range signal, number 122
22:53:50  WARNING -  TEST-UNEXPECTED-FAIL | Shutdown | application terminated with exit code -6

Comment 4

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

I'm guessing that's because there's shutdown code somewhere *else* that's #ifdef DEBUG instead of #ifdef NS_FREE_PERMANENT_DATA, and that leads a leak to end up in this code.

Comment 5

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

(For what it's worth, the original reason I brought this issue up was because of its existence as a pattern to copy from; I was trying to remember the name of NS_FREE_PERMANENT_DATA, and the place I looked to find a use of it was nsAppRunner.cpp, which turned out not to be a use of it.)

Comment 6

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

Attachment: nsAppRunner.cpp should use NS_FREE_PERMANENT_DATA rather than making up its own variant of the same concept

TODO: Fix ASAN mochitest orange.

Comment 7

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

Comment on attachment 8521765 [details] [diff] [review]
nsAppRunner.cpp should use NS_FREE_PERMANENT_DATA rather than making up its own variant of the same concept

Er, sorry, I hg bzexport'ed on more patch than intended.

Comment 8

[Jesse Ruderman]

After landing bug 1024259, I sent this patch through Try to see if it's happier now. It isn't. https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d1075d69127