1357805 - When enabling Screenshots in tests, debug builds fail with WARNING: YOU ARE LEAKING THE WORLD

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P1)

Description

[Mark Banner (:standard8)]

I'm working on enabling Screenshots by default during our test runs. Currently, all debug test runs are failing due to an assertion. I believe this is a regression, as I think earlier try runs didn't show an issue.

Could be a regression from bug 1317697 but I'm not sure.

Try builds:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d49a34bf9b82c8719f2648e542df9e9754e87594&selectedJob=92426186

Specific link:

https://treeherder.mozilla.org/logviewer.html#?job_id=92426186&repo=try&lineNumber=3682

Assertion failed at /home/worker/workspace/build/src/gfx/cairo/cairo/src/cairo-hash.c:196: hash_table->live_entries == 0
WARNING: YOU ARE LEAKING THE WORLD (at least one JSRuntime and everything alive inside it, that is) AT JS_ShutDown TIME.  FIX THIS!
###!!! ASSERTION: Component Manager being held past XPCOM shutdown.: 'cnt == 0', file /home/worker/workspace/build/src/xpcom/build/XPCOMInit.cpp, line 1045
#01: ScopedXPCOMStartup::~ScopedXPCOMStartup [toolkit/xre/nsAppRunner.cpp:1439]
#02: mozilla::DefaultDelete<ScopedXPCOMStartup>::operator() [mfbt/UniquePtr.h:529]
#03: XREMain::XRE_main [toolkit/xre/nsAppRunner.cpp:4751]
#04: XRE_main [toolkit/xre/nsAppRunner.cpp:4808]
#05: do_main [browser/app/nsBrowserApp.cpp:236]
#06: main [browser/app/nsBrowserApp.cpp:309]
#07: libc.so.6 + 0x217ed
#08: _start

Comment 1

[Mark Banner (:standard8)]

NI to Kris/Shane as this could be a regression from bug 1317697

Comment 2

[Kris Maglione [:kmag]]

This is because you're never shutting your embedded webextension down. That means that the windowless browser that holds your background page is never destroyed. And since windowless browsers aren't cycle collected, that means that it can't be freed at shutdown.

Comment 3

[Mark Banner (:standard8)]

(In reply to Kris Maglione [:kmag] from comment #2)
> This is because you're never shutting your embedded webextension down. That
> means that the windowless browser that holds your background page is never
> destroyed. And since windowless browsers aren't cycle collected, that means
> that it can't be freed at shutdown.

Ah, thanks for the pointers. I'm currently investigating adding the shutdown, and I've got one of the basic tests to pass locally, so I'm now trying on the try server.

One thing that I did find was that because the WebExtension part is slow to start up (it can be ~5 seconds after the window appears on a debug build), if you startup and shutdown the browser quickly, it still leaks the world (even with Screenshots calling webextension.shutdown(). This has me a little concerned that we could still see issues.

Comment 4

[Mark Banner (:standard8)]

Try server confirms that the debug tests are looking better. There's still a few failures, but I need to reconfirm once we export the latest version of the add-on which has slightly better shutdown handling that what I pushed to try earlier.

Comment 5

[Kris Maglione [:kmag]]

(In reply to Mark Banner (:standard8) from comment #3)
> One thing that I did find was that because the WebExtension part is slow to
> start up (it can be ~5 seconds after the window appears on a debug build),
> if you startup and shutdown the browser quickly, it still leaks the world
> (even with Screenshots calling webextension.shutdown(). This has me a little
> concerned that we could still see issues.

Yeah, we've run into that in other tests. I've been planning to fix it, so that we block shutdown until startup has finished, and block startup until the last shutdown has finished.

But in the mean time, if this is causing you trouble, you can just wait for the startup promise to resolve before starting shutdown, and block async shutdown on the shutdown promise if necessary.

Comment 6

[Mark Banner (:standard8)]

From some try runs I've been doing, I believe this is now fixed by https://github.com/mozilla-services/screenshots/pull/2712

There are still some debug leak failures, for which I've filed bug 1360512.