Skip to content

Conversation

@kainino0x
Copy link
Collaborator

Discussion: #15251 (comment)

@kainino0x kainino0x requested a review from sbc100 October 7, 2021 20:30
@kainino0x
Copy link
Collaborator Author

Leaving here for posterity but not sure this is the right change.

@sbc100
Copy link
Collaborator

sbc100 commented Oct 7, 2021

Seem like the right thing to do to me.. at the very least we don't want to actually call into the user code in this case.

@kainino0x
Copy link
Collaborator Author

What made me unsure about this change is that it would affect e.g. emscripten_set_timeout. It perhaps makes sense that would assert/abort if the runtime wasn't kept alive, rather than earlying-out as in this PR. But at least in that particular case, that's not a concern since it uses runtimeKeepalivePush.

@kainino0x kainino0x reopened this Oct 7, 2021
@kainino0x kainino0x marked this pull request as ready for review October 8, 2021 00:21
@kainino0x
Copy link
Collaborator Author

Ready for review; after working through things I think this change does make sense.

@sbc100 sbc100 merged commit b1907ce into emscripten-core:main Oct 8, 2021
@kainino0x kainino0x deleted the callUserCallback branch October 8, 2021 01:27
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

Successfully merging this pull request may close these issues.

2 participants