Skip to content

is exception handling thread safe? #11233

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
malytomas opened this issue May 23, 2020 · 10 comments
Closed

is exception handling thread safe? #11233

malytomas opened this issue May 23, 2020 · 10 comments
Labels

Comments

@malytomas
Copy link

I have just read this blog post:
https://brionv.com/log/2019/10/24/exception-handling-in-emscripten-how-it-works-and-why-its-disabled-by-default/
And there is a mention of a global flag variable.
Is that variable actually global and can it cause issues in multithreaded applications?

@sbc100
Copy link
Collaborator

sbc100 commented May 23, 2020

I think its probably not thread safe today for the reason you point out.

I believe the globals in question are here:

int __threwValue = 0;

I think the solution is to make these variables into wasm globals rather than living in linear memory (similar to how we handle __stack_pointer).

@kripken
Copy link
Member

kripken commented May 23, 2020

There is also some support code in JS in src/library_exceptions.js. I believe that and extras.c is all unneeded in the new wasm exceptions support @aheejin is working on, so I hope that will be properly threadsafe?

@aheejin
Copy link
Member

aheejin commented May 24, 2020

Yes, the wasm EH will need neither of extras.c and JS exception libraries, and I can't think of a reason why it wouldn't be thread safe.

@malytomas
Copy link
Author

Thank you all for the information.

I have removed the most common place where an exception has been thrown and it significantly improved stability for the application. But I cannot replace all exceptions.

I would like to ask if there is a roadmap or a rough estimate when the new exception handling would be available? Thanks.

@aheejin
Copy link
Member

aheejin commented May 28, 2020

We are actively working on the new EH and it is being tested and stabilized, but I think it will need several more months before being reliable enough to be tried by normal users. That being said, you can try it now by using -fwasm-exceptions flag to emcc command line (You shouldn't use -s DISABLE_EXCEPTION_CATCHING=0 with it). But it is likely that there are many bugs remaining. And currently only V8 has an experimental support for this feature among the major web VMs.

@sbc100
Copy link
Collaborator

sbc100 commented May 28, 2020

We can possible make the current emscripten exceptions thread safe if there is strong need for it before native exception handling lands.

In the mean time we should not allow builds to use exceptions and threads at the same time.. since these are not compatible today.

@malytomas
Copy link
Author

We can possible make the current emscripten exceptions thread safe if there is strong need for it before native exception handling lands.

I personally would prefer the effort to be put into the new native exception handling.

In the mean time we should not allow builds to use exceptions and threads at the same time.. since these are not compatible today.

Warning would have been nice, but completely failing builds is an overreaction, in my opinion.

@kripken
Copy link
Member

kripken commented Aug 25, 2020

#11518 fixed most of this, but see the comment in the code there, there remains a corner case not fully handled.

@stale
Copy link

stale bot commented Aug 28, 2021

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Aug 28, 2021
@stale stale bot closed this as completed Apr 18, 2022
@aheejin
Copy link
Member

aheejin commented May 3, 2022

Wasm EH proposal hasn't been fully standardized yet, but it's been implemented in Emscripten and (I believe) Safari and FireFox too, and is thread safe: https://emscripten.org/docs/porting/exceptions.html#webassembly-exception-handling-proposal

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

No branches or pull requests

4 participants