1675329 - Add an API for reporting a return code from XPCOM applications

Status: RESOLVED FIXED

(Toolkit :: General, enhancement)

Description

[Nick Alexander :nalexander [he/him]]

Right now, there's no way to ask Firefox (and other XPCOM applications) to exit with a specific exit code. Per Mossop in a review comment, there have been a few issues where such a feature would have been helpful, and he suggests "add[ing] an attribute on nsIAppStartup to let anything set the current exit code". This ticket tracks doing so.

I am aware that there has long been a desire to make Firefox shutdown faster by invoking exit(0) without completing clean up. I don't know if an API such as this would preclude invoking exit(0); in theory, the caller can fairly easily fish any application-defined exit code from nsIAppStartup and exit with that code. For my immediate use case, it's fine to say that in an orderly shutdown that exists through XREMain::XRE_main, any application-defined exit code will be used.

Comment 1

[Nick Alexander :nalexander [he/him]]

dthayer: I think that you are carrying, or have carried, the exit(0) banner. Is what I say in #c0 reasonable? Are there other interactions in this space that preclude doing this?

Comment 2

[Alex Thayer [:alexical] (she/her)]

Should be fine. To be more specific though, we are currently calling _exit(x) and not exit(x) on non-Windows, but on windows we are calling TerminateProcess, but that still takes an exit code parameter, so I can't think of any reason why this wouldn't be trivial.

Comment 3

[Alex Thayer [:alexical] (she/her)]

(To make things easier, the code in question is here: https://searchfox.org/mozilla-central/rev/c938c7416c633639a5c8ce4412be586eefb48005/xpcom/base/AppShutdown.cpp#224)

Comment 4

[Dave Townsend [:mossop]]

Attachment: Bug 1675329: Add an API for returning an exit code from the application. r=dthayer

Callers can pass an exit code to nsIAppStartup::Quit and it will be returned from the process when
it exits.

Note that I have using uint16_t as the exit code because on Windows the exit code can be a uint and
elsewhere it is an int. A uint16_t will safely convert to either of those and no-one will ever need
more than 64k exit codes!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d57553438d94
Add an API for returning an exit code from the application. r=dthayer,necko-reviewers,dragana,nika

Comment 6

[Sandor Molnar[:smolnar]]

Backed out for causing bustages in nsAppShell.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/4bc0bdcd7889aab2994b4500d41e6e35216e4848

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=Ed8Xi2ZvREqGIZL_EDBYoQ.0&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Crunning%2Cpending%2Crunnable&revision=d57553438d94c340a3c617449aab27665bd665fa

Failure log: https://treeherder.mozilla.org/logviewer?job_id=322463603&repo=autoland&lineNumber=29889

"INFO - Finished processing /builds/worker/workspace/obj-build/mfbt/tests/TestAtomics in 0.14s
[task 2020-11-20T20:35:09.622Z] 20:35:09 INFO - make[4]: Leaving directory '/builds/worker/workspace/obj-build/mfbt/tests'
[task 2020-11-20T20:35:09.764Z] 20:35:09 INFO - make[4]: Entering directory '/builds/worker/workspace/obj-build/widget/android'
[task 2020-11-20T20:35:09.767Z] 20:35:09 INFO - /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 --target=arm-linux-androideabi -o Unified_cpp_widget_android0.o -c -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -fstack-protector-strong -ftrivial-auto-var-init=pattern -DDEBUG=1 -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/widget/android -I/builds/worker/workspace/obj-build/widget/android -I/builds/worker/workspace/obj-build/ipc/ipdl/ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/ipc/glue -I/builds/worker/checkouts/gecko/docshell/base -I/builds/worker/checkouts/gecko/dom/base -I/builds/worker/checkouts/gecko/dom/system/android -I/builds/worker/checkouts/gecko/gfx/2d -I/builds/worker/checkouts/gecko/gfx/vr -I/builds/worker/checkouts/gecko/layout/forms -I/builds/worker/checkouts/gecko/layout/painting -I/builds/worker/checkouts/gecko/netwerk/base -I/builds/worker/checkouts/gecko/netwerk/cache -I/builds/worker/checkouts/gecko/toolkit/components/telemetry -I/builds/worker/checkouts/gecko/widget -I/builds/worker/checkouts/gecko/xpcom/threads -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -isystem /builds/worker/fetches/android-ndk/sysroot/usr/include/arm-linux-androideabi -isystem /builds/worker/fetches/android-ndk/sysroot/usr/include -gcc-toolchain /builds/worker/fetches/android-ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64 -D__ANDROID_API_=16 -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wno-error=deprecated-copy -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-psabi -Wno-unknown-warning-option -mno-unaligned-access -fno-sized-deallocation -fno-aligned-new -fno-short-enums -fno-exceptions -fcrash-diagnostics-dir=/builds/worker/artifacts -stdlib=libstdc++ -I/builds/worker/fetches/android-ndk/sources/cxx-stl/llvm-libc++/include -I/builds/worker/fetches/android-ndk/sources/android/support/include -I/builds/worker/fetches/android-ndk/sources/cxx-stl/llvm-libc++abi/include -march=armv7-a -mthumb -mfpu=neon -mfloat-abi=softfp -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Oz -mno-outline -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow -fexperimental-new-pass-manager -MD -MP -MF .deps/Unified_cpp_widget_android0.o.pp Unified_cpp_widget_android0.cpp
[task 2020-11-20T20:35:09.767Z] 20:35:09 INFO - In file included from Unified_cpp_widget_android0.cpp:128:
[task 2020-11-20T20:35:09.768Z] 20:35:09 ERROR - /builds/worker/checkouts/gecko/widget/android/nsAppShell.cpp:225:67: error: too few arguments to function call, expected 3, have 2
[task 2020-11-20T20:35:09.768Z] 20:35:09 INFO - appStartup->Quit(nsIAppStartup::eForceQuit, &userAllowedQuit);
[task 2020-11-20T20:35:09.768Z] 20:35:09 INFO - ~~~~~~~~~~~~~~~~ ^
[task 2020-11-20T20:35:09.768Z] 20:35:09 INFO - /builds/worker/workspace/obj-build/dist/include/nsIAppStartup.h:79:14: note: 'Quit' declared here
[task 2020-11-20T20:35:09.768Z] 20:35:09 INFO - NS_IMETHOD Quit(uint32_t aMode, int32_t aExitCode, bool *_retval) = 0;
[task 2020-11-20T20:35:09.768Z] 20:35:09 INFO - ^
[task 2020-11-20T20:35:09.768Z] 20:35:09 INFO - 1 error generated.
[task 2020-11-20T20:35:09.768Z] 20:35:09 ERROR - make[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:674: Unified_cpp_widget_android0.o] Error 1
[task 2020-11-20T20:35:09.768Z] 20:35:09 INFO - make[4]: Leaving directory '/builds/worker/workspace/obj-build/widget/android'
[task 2020-11-20T20:35:09.769Z] 20:35:09 ERROR - make[3]: *** [/builds/worker/checkouts/gecko/config/recurse.mk:72: widget/android/target-objects] Error 2
[task 2020-11-20T20:35:09.769Z] 20:35:09 INFO - make[3]: *** Waiting for unfinished jobs....
[task 2020-11-20T20:35:10.613Z] 20:35:10 INFO - make[4]: Entering directory '/builds/worker/workspace/obj-build/mfbt/tests'
[task 2020-11-20T20:35:10.613Z] 20:35:10 INFO - /builds/worker/workspace/obj-build/_virtualenvs/init_py3/bin/python -m mozbuild.action.dumpsymbols /builds/worker/workspace/obj-build/mfbt/tests/TestBinarySearch /builds/worker/workspace/obj-build/mfbt/tests/TestBinarySearch_syms.track
[task 2020-11-20T20:35:10.613Z] 20:35:10 INFO - Running: /builds/worker/workspace/obj-build/_virtualenvs/init_py3/bin/python /builds/worker/checkouts/gecko/toolkit/crashreporter/tools/symbolstore.py -c --vcs-info --install-manifest=/builds/worker/workspace/obj-build/_build_manifests/install/dist_include,/builds/worker/workspace/obj-build/dist/include -s /builds/worker/checkouts/gecko /builds/worker/fetches/dump_syms/dump_syms /builds/worker/workspace/obj-build/dist/crashreporter-symbols /builds/worker/workspace/obj-build/mfbt/tests/TestBinarySearch
[task 2020-11-20T20:35:10.613Z] 20:35:10 INFO - Beginning work for file: /builds/worker/workspace/obj-build/mfbt/tests/TestBinarySearch
[task 2020-11-20T20:35:10.614Z] 20:35:10 INFO - Processing file: /builds/worker/workspace/obj-build/mfbt/tests/TestBinarySearch
[task 2020-11-20T20:35:10.614Z] 20:35:10 INFO - /builds/worker/fetches/dump_syms/dump_syms /builds/worker/workspace/obj-build/mfbt/tests/TestBinarySearch
[task 2020-11-20T20:35:10.614Z] 20:35:10 INFO - Finished processing /builds/worker/workspace/obj-build/mfbt/tests/TestBinarySearch in 0.13s"

Comment 7

[Pulse]

Not sure I understand this but I'm getting unwatchable corruption when playing video (Youtube/Gftcat). I ran a Mozregression and it pointed to this bug.

To reproduce, open any video on Youtube and switch to cinema mode and 1080p. The corruption also happens when watching the video with PIP.

I'm running openSUSE Tumbleweed/KDE X11.

app_name: firefox
build_date: 2020-11-20 20:05:24.644000
build_file: /home/mango/.mozilla/mozregression/persist/39c13de70afd-shippable--autoland--target.tar.bz2
build_type: integration
build_url: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/TvS60eLoQq2gbu6VxKL-vA/runs/0/artifacts/public%2Fbuild%2Ftarget.tar.bz2
changeset: 39c13de70afdf4f39d771ed3266f99ea63c3228e
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=39c13de70afdf4f39d771ed3266f99ea63c3228e&tochange=98e8b23c627e0839acf9f5b87f1e881d3e6a40b0
repo_name: autoland
repo_url: https://hg.mozilla.org/integration/autoland
task_id: TvS60eLoQq2gbu6VxKL-vA

2020-11-25T21:23:06.418000: INFO : Narrowed integration regression window from [39c13de7, 4bc0bdcd] (4 builds) to [81361995, 4bc0bdcd] (2 builds) (~1 steps left)
2020-11-25T21:23:06.426000: DEBUG : Starting merge handling...
2020-11-25T21:23:06.426000: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=4bc0bdcd7889aab2994b4500d41e6e35216e4848&full=1
2020-11-25T21:23:06.426000: DEBUG : redo: attempt 1/3
2020-11-25T21:23:06.426000: DEBUG : redo: retry: calling _default_get with args: ('https://hg.mozilla.org/integration/autoland/json-pushes?changeset=4bc0bdcd7889aab2994b4500d41e6e35216e4848&full=1',), kwargs: {}, attempt #1
2020-11-25T21:23:06.431000: DEBUG : urllib3.connectionpool: Resetting dropped connection: hg.mozilla.org
2020-11-25T21:23:08.091000: DEBUG : urllib3.connectionpool: https://hg.mozilla.org:443 "GET /integration/autoland/json-pushes?changeset=4bc0bdcd7889aab2994b4500d41e6e35216e4848&full=1 HTTP/1.1" 200 None
2020-11-25T21:23:08.134000: DEBUG : Found commit message:
Backed out changeset d57553438d94 (bug 1675329) for causing bustages in nsAppShell. CLOSED TREE

2020-11-25T21:23:08.134000: DEBUG : Did not find a branch, checking all integration branches
2020-11-25T21:23:08.136000: INFO : The bisection is done.
2020-11-25T21:23:08.136000: INFO : Stopped

Comment 8

[Dave Townsend [:mossop]]

(In reply to Pulse from comment #7)

Not sure I understand this but I'm getting unwatchable corruption when playing video (Youtube/Gftcat). I ran a Mozregression and it pointed to this bug.

Since this bug hasn't landed it cannot be the cause of your issues.

Comment 9

[Pulse]

(In reply to Dave Townsend [:mossop] from comment #8)

(In reply to Pulse from comment #7)

Not sure I understand this but I'm getting unwatchable corruption when playing video (Youtube/Gftcat). I ran a Mozregression and it pointed to this bug.

Since this bug hasn't landed it cannot be the cause of your issues.

Which is why I said I didn't understand why Mozregression came up with this bug. I also created a new bug for this 1679343

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/55899ad1d955
Add an API for returning an exit code from the application. r=dthayer,necko-reviewers,dragana,nika

Comment 11

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/55899ad1d955

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/91e625d9643c
Port bug 1675329: Add a second argument when calling nsIAppStartup::Quit. rs=bustage-fix