Skip to content

onRuntimeInitialized no longer being called #23626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jeswr opened this issue Feb 8, 2025 · 10 comments
Closed

onRuntimeInitialized no longer being called #23626

jeswr opened this issue Feb 8, 2025 · 10 comments

Comments

@jeswr
Copy link
Contributor

jeswr commented Feb 8, 2025

As of v4 it appears that onRuntimeInitialized is no longer invoked after initialising a module.

Is this expected behavior?

Observed in SWI-Prolog/swipl-devel#1343 (comment)

@sbc100
Copy link
Collaborator

sbc100 commented Feb 8, 2025

No that is not intended. We do have quite a few test for onRuntimeInitialized (spelt with a z BTW) so something odd must be going on.

Can you confirm that version in which this stopped working for you? Was it 4.0.0 exactly? Did it work in the previous version (3.1.74)?

@jeswr
Copy link
Contributor Author

jeswr commented Feb 8, 2025

Yes, 4.0.0 exactly. You can see:

The implementation of onRuntimeInitialized is upstream at https://github.com/SWI-Prolog/swipl-devel/blob/5f00677154dfcde00381fdaf501d72d2b985008d/src/wasm/prolog.js#L1919-L1921 (and uses z)

@jeswr jeswr changed the title onRuntimeInitialised no longer being called onRuntimeInitialized no longer being called Feb 8, 2025
@sbc100
Copy link
Collaborator

sbc100 commented Feb 8, 2025

Can you share the full list of linker flags that you use? Are you using -sSTRICT by any chance?

@jeswr
Copy link
Contributor Author

jeswr commented Feb 8, 2025

I don't believe so this is the CMAKE file used @JanWielemaker would be the one who can confirm this.

@jeswr
Copy link
Contributor Author

jeswr commented Feb 8, 2025

Specifically these are the linker flags used

@sbc100
Copy link
Collaborator

sbc100 commented Feb 8, 2025

Can you show me how/where you inject your onRuntimeInitialized call? If you are adding it in a --post-js file that might explain the issue. We switch to using async/await during initialization in #23157. See the changelog entry from that change:

emscripten/ChangeLog.md

Lines 109 to 114 in 242af00

- The factory function exposed in `-sMODULARIZE` mode is now marked as `async`
when `WASM_ASYNC_COMPILATION` is enabled (the default). This allows us to use
`await` during module creation. One side effect of this is that code in
`--post-js` files will now be delayed until after module creation and after
`main` runs. This matches the existing behaviour when using sync instantation
(`-sWASM_ASYNC_COMPILATION=0`) but is an observable difference. (#23157)

I don't see any --post-js flag, but I suspect this is somehow the reason. Somebody else also reported a similar issue: #23420.

One thing I was think of doing is adding an assertion if these things are added to the Module too late..

@jeswr
Copy link
Contributor Author

jeswr commented Feb 8, 2025

Aha thankyou! Looks like @JanWielemaker very recently converted --pre-js to --post-js in SWI-Prolog/swipl-devel@abec205; more recently than the release of v4.0.3.

Updating to include those changes indeed fixes things.

One thing I was think of doing is adding an assertion if these things are added to the Module too late..

Yes, I think this would be useful

@jeswr jeswr closed this as completed Feb 8, 2025
@JanWielemaker
Copy link

converted --pre-js to --post-js in SWI-Prolog/swipl-devel@abec205; more recently than the release of v4.0.3.

Nope. It is the other way around. There used to be --pre-js and --post-js code. These two files have been merged and are used as--pre-js. So far that seems fine.

I think you should be able to update to 9.3.20. If you want to replicate my build

  • create ~/wasm
  • Install Emscripten in ~/wasm/emsdk
  • Build the zlib and pcre2 libs such that they are installed with prefix ~/wasm
  • create a build dir in SWI-Prolog called build.wasm
  • In this dir, run ../scripts/configure. This creates, based on the build dir name, a script configure that sets the necessary environment and runs cmake.

@jeswr
Copy link
Contributor Author

jeswr commented Feb 8, 2025

I think you should be able to update to 9.3.20

Have done - new version of swipl-wasm now released https://github.com/swI-Prolog/npm-swipl-wasm. Thanks for making the fix 🥳

sbc100 added a commit to sbc100/emscripten that referenced this issue Feb 8, 2025
We had some bug reports recently when we switched to using async/await
in MODULUARIZE mode.  One of the side effects was the `--post-js` code
then runs after module instantiation and startup, which means assignment
of properties like `onRuntimeInitialized` is too late if it happens in
`--post-js`.

This was always the case for sync instantiation too.

We now detect this too-late-assignment at abort with a useful error.

See emscripten-core#23626 and emscripten-core#23420
sbc100 added a commit to sbc100/emscripten that referenced this issue Feb 8, 2025
We had some bug reports recently when we switched to using async/await
in MODULUARIZE mode.  One of the side effects was the `--post-js` code
then runs after module instantiation and startup, which means assignment
of properties like `onRuntimeInitialized` is too late if it happens in
`--post-js`.

This was always the case for sync instantiation too.

We now detect this too-late-assignment at abort with a useful error.

See emscripten-core#23626 and emscripten-core#23420
sbc100 added a commit to sbc100/emscripten that referenced this issue Feb 8, 2025
We had some bug reports recently when we switched to using async/await
in MODULUARIZE mode.  One of the side effects was the `--post-js` code
then runs after module instantiation and startup, which means assignment
of properties like `onRuntimeInitialized` is too late if it happens in
`--post-js`.

This was always the case for sync instantiation too.

We now detect this too-late-assignment at abort with a useful error.

See emscripten-core#23626 and emscripten-core#23420
@sbc100
Copy link
Collaborator

sbc100 commented Feb 8, 2025

After #23629 lands you should see a clear error in debug build when these properties are set too late.

sbc100 added a commit that referenced this issue Feb 11, 2025
We had some bug reports recently when we switched to using async/await
in `MODULUARIZE` mode.  One of the side effects was the `--post-js` code
then runs after module instantiation and startup, which means assignment
of properties like `onRuntimeInitialized` is too late if it happens in
`--post-js`.

This was always the case for sync instantiation mode.

When assertions are enabled, we now detect this too-late-assignment and
abort
with a useful error.

Code size regression in this PR, as expected, only for debug builds.

See #23626 and #23420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants