From e3b08a61ebf979811325e079b468f10989349563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Morten=20S=C3=B8rvig?= Date: Wed, 5 Jul 2023 14:46:13 +0200 Subject: [PATCH] wasm: clarify qtloader onExit behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit onExit is called whenever the application exits, i.e. when the app canvas should no longer be displayed and the loader/embedder code should take some action. Emscripten provides two callbacks which can be used here: - onExit, called when the app exits (but see EXIT_RUNTIME) - onAbort, called on abort errors. These map to the two cases Qt's onExit supports. onExit is not called when EXIT_RUNTIME is disabled, which means we don't need the special case for exit code 0. In addition call onExit on any exception. The second call to showUi() in html_shell.html is then not needed any more and we avoid duplicating the UI state handling in user code. Update the qtloader_integration test to handle changes in behavior (we no longer set the error text on exit). Use emscripten_force_exit() to simulate application exit - using this function makes Emscripten call onExit even when EXIT_RUNTIME is disabled. Change-Id: I72b5463c1836e8d5054e594abbd304fbc67032b7 Reviewed-by: Piotr Wierciński Reviewed-by: Mikołaj Boc (cherry picked from commit a4d1c30a1b52e797cce504f90bcf20d7943dd1f9) Reviewed-by: Qt Cherry-pick Bot --- src/plugins/platforms/wasm/qtloader.js | 54 +++++++++---------- src/plugins/platforms/wasm/wasm_shell.html | 2 - .../manual/wasm/qtloader_integration/main.cpp | 4 +- .../wasm/qtloader_integration/test_body.js | 8 ++- 4 files changed, 32 insertions(+), 36 deletions(-) diff --git a/src/plugins/platforms/wasm/qtloader.js b/src/plugins/platforms/wasm/qtloader.js index ee6c91180bb..4b6cee80f5e 100644 --- a/src/plugins/platforms/wasm/qtloader.js +++ b/src/plugins/platforms/wasm/qtloader.js @@ -9,9 +9,14 @@ * - environment: { [name:string] : string } * environment variables set on the instance * - onExit: (exitStatus: { text: string, code?: number, crashed: bool }) => void - * called when the application has exited for any reason. exitStatus.code is defined in - * case of a normal application exit. This is not called on exit with return code 0, as - * the program does not shutdown its runtime and technically keeps running async. + * called when the application has exited for any reason. There are two cases: + * aborted: crashed is true, text contains an error message. + * exited: crashed is false, code contians the exit code. + * + * Note that by default Emscripten does not exit when main() returns. This behavior + * is controlled by the EXIT_RUNTIME linker flag; set "-s EXIT_RUNTIME=1" to make + * Emscripten tear down the runtime and exit when main() returns. + * * - containerElements: HTMLDivElement[] * Array of host elements for Qt screens. Each of these elements is mapped to a QScreen on * launch. @@ -161,26 +166,14 @@ async function qtLoad(config) return originalLocatedFilename; } - // This is needed for errors which occur right after resolving the instance promise but - // before exiting the function (i.e. on call to main before stack unwinding). - let loadTimeException = undefined; - // We don't want to issue onExit when aborted - let aborted = false; - const originalQuit = config.quit; - config.quit = (code, exception) => - { - originalQuit?.(code, exception); - - if (exception) - loadTimeException = exception; - if (!aborted && code !== 0) { - config.qt.onExit?.({ - text: exception.message, - code, - crashed: false - }); - } - }; + const originalOnExit = config.onExit; + config.onExit = code => { + originalOnExit?.(); + config.qt.onExit?.({ + code, + crashed: false + }); + } const originalOnAbort = config.onAbort; config.onAbort = text => @@ -212,10 +205,17 @@ async function qtLoad(config) // Call app/emscripten module entry function. It may either come from the emscripten // runtime script or be customized as needed. - const instance = await Promise.race( - [circuitBreaker, config.qt.entryFunction(config)]); - if (loadTimeException && loadTimeException.name !== 'ExitStatus') - throw loadTimeException; + let instance; + try { + instance = await Promise.race( + [circuitBreaker, config.qt.entryFunction(config)]); + } catch (e) { + config.qt.onExit?.({ + text: e.message, + crashed: true + }); + throw e; + } return instance; } diff --git a/src/plugins/platforms/wasm/wasm_shell.html b/src/plugins/platforms/wasm/wasm_shell.html index 26d914ea230..f9b322754d8 100644 --- a/src/plugins/platforms/wasm/wasm_shell.html +++ b/src/plugins/platforms/wasm/wasm_shell.html @@ -65,8 +65,6 @@ } catch (e) { console.error(e); console.error(e.stack); - status.innerHTML = e.message; - showUi(spinner); } } diff --git a/tests/manual/wasm/qtloader_integration/main.cpp b/tests/manual/wasm/qtloader_integration/main.cpp index b9bed0d49b0..ee032e9952d 100644 --- a/tests/manual/wasm/qtloader_integration/main.cpp +++ b/tests/manual/wasm/qtloader_integration/main.cpp @@ -71,7 +71,7 @@ void crash() void exitApp() { - exit(ExitValueFromExitApp); + emscripten_force_exit(ExitValueFromExitApp); } void produceOutput() @@ -135,7 +135,7 @@ int main(int argc, char **argv) std::find(arguments.begin(), arguments.end(), QStringLiteral("--exit-immediately")) != arguments.end(); if (exitImmediately) - return ExitValueImmediateReturn; + emscripten_force_exit(ExitValueImmediateReturn); const bool crashImmediately = std::find(arguments.begin(), arguments.end(), QStringLiteral("--crash-immediately")) diff --git a/tests/manual/wasm/qtloader_integration/test_body.js b/tests/manual/wasm/qtloader_integration/test_body.js index d075361a313..b75ef36a893 100644 --- a/tests/manual/wasm/qtloader_integration/test_body.js +++ b/tests/manual/wasm/qtloader_integration/test_body.js @@ -323,9 +323,7 @@ export class QtLoaderIntegrationTests caughtException = e; } - // An exception should have been thrown from load() - assert.equal('RuntimeError', caughtException.name); - + assert.isUndefined(caughtException); assert.equal(1, onExitMock.calls.length); const exitStatus = onExitMock.calls[0][0]; assert.isTrue(exitStatus.crashed); @@ -373,7 +371,7 @@ export class QtLoaderIntegrationTests const exitStatus = onExitMock.calls[0][0]; assert.isFalse(exitStatus.crashed); assert.equal(instance.EXIT_VALUE_FROM_EXIT_APP, exitStatus.code); - assert.isNotUndefined(exitStatus.text); + assert.isUndefined(exitStatus.text); } async exitImmediately() @@ -392,7 +390,7 @@ export class QtLoaderIntegrationTests assert.isFalse(exitStatusFromOnExit.crashed); assert.equal(instance.EXIT_VALUE_IMMEDIATE_RETURN, exitStatusFromOnExit.code); - assert.isNotUndefined(exitStatusFromOnExit.text); + assert.isUndefined(exitStatusFromOnExit.text); } async userQuitCallbackCalled()