Skip to content

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 1, 2020

We have pthread_exit() for users who only want to exit from the one thread (this
is the same as returning from the thread entry point).

The C exit function on the other hand is supposed to bring down the whole process.

@sbc100 sbc100 force-pushed the exit_from_pthread branch from d547c58 to ff943b3 Compare December 1, 2020 09:49
@sbc100 sbc100 force-pushed the remove_duplicate_exit_code branch from 18fd850 to 37b298a Compare December 1, 2020 22:44
Base automatically changed from remove_duplicate_exit_code to master December 1, 2020 23:47
@sbc100 sbc100 force-pushed the exit_from_pthread branch from ff943b3 to 68fb6e5 Compare December 1, 2020 23:51
@sbc100 sbc100 changed the title Handle exit() from pthread correctly by exiting the whole process Handle exit() from pthread by exiting the whole process Dec 1, 2020
@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 1, 2020

Its not clear to me how this should interact with EXIT_RUNTIME. So we want
threaded code to be able to call exit and keep the thread alive like they can on
the main thread?

I'm not sure how to interpret the comment on this line:

if (!ENVIRONMENT_IS_PTHREAD) // EXIT_RUNTIME=0 only applies to default behavior of the main browser thread

// EXIT_RUNTIME=0 only applies to default behavior of the main browser thread

But the net effect of that line is that noExitRuntime is always left as undefined` on pthreads.

@sbc100 sbc100 force-pushed the exit_from_pthread branch from 68fb6e5 to bfd5c07 Compare December 2, 2020 00:08
@sbc100 sbc100 requested review from kripken and juj December 2, 2020 00:09
@sbc100 sbc100 force-pushed the exit_from_pthread branch from bfd5c07 to 2171282 Compare December 2, 2020 02:15
@sbc100 sbc100 changed the title Handle exit() from pthread by exiting the whole process Handle exit() from pthread by posting exitProcess back to main thread Dec 2, 2020
@sbc100 sbc100 force-pushed the exit_from_pthread branch from 2171282 to 97602fb Compare December 2, 2020 03:52
@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 2, 2020

Needed for #12928

@kripken
Copy link
Member

kripken commented Dec 2, 2020

I think when EXIT_RUNTIME we should properly handle exit on pthreads too, the same way native builds do. I think we've just never fully supported that mode in pthreads builds yet.

@sbc100 sbc100 force-pushed the exit_from_pthread branch from 97602fb to 198e39b Compare December 2, 2020 19:50
@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 2, 2020

I think when EXIT_RUNTIME we should properly handle exit on pthreads too, the same way native builds do. I think we've just never fully supported that mode in pthreads builds yet.

I think the reason why we don't honor the EXIT_RUNTIME setting by default in pthread code is because it is on by default and it would break almost all pthread code if threads were not joinable once they returned from their entry point. Code can still opt out by explictly setting noExitRuntime or called on of several functions that does that.

This change means that exit() always honors EXIT_RUNTIME, even when called from threads. Because we post the exit request back to the main thread it allows the main thread to make the decision.

@sbc100 sbc100 force-pushed the exit_from_pthread branch from 198e39b to a7767ba Compare December 2, 2020 19:56
@kripken
Copy link
Member

kripken commented Dec 2, 2020

I think the reason why we don't honor the EXIT_RUNTIME setting by default in pthread code is because it is on by default and it would break almost all pthread code if threads were not joinable once they returned from their entry point.

Hmm, it's off by default, isn't it, not on - maybe a typo?

And I'm not sure why it would break code. How does joinability relate to EXIT_RUNTIME?

@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 2, 2020

I think the reason why we don't honor the EXIT_RUNTIME setting by default in pthread code is because it is on by default and it would break almost all pthread code if threads were not joinable once they returned from their entry point.

Hmm, it's off by default, isn't it, not on - maybe a typo?

And I'm not sure why it would break code. How does joinability relate to EXIT_RUNTIME?

Because if you do return from your thread main or if you just fall of the then of it, or of your call pthread_exit, if noExitRuntime is true then the thread is still alive and can't then be joined.

I think this is why, we don't set noExitRuntime = true on pthreads. Its not the behaviour folks expect.

If we did set noExitRuntime = true for pthread just (like we do for the main thread) then they only for threads to exit would be explictly set noExitRuntime back to false again on each thread.

Unlike the main thread there is often somebody waiting for a thread to exit.

@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 2, 2020

This change does change the existing behaviour or the existing setup of noExitRuntime on pthreads.

All this change does is proxy the C exit() calls back to the main thread so it can decide how to handle it.

@sbc100 sbc100 force-pushed the exit_from_pthread branch from a7767ba to 168ccf5 Compare December 2, 2020 22:06
@sbc100 sbc100 merged commit f28260b into master Dec 2, 2020
@sbc100 sbc100 deleted the exit_from_pthread branch December 2, 2020 22:08
@kripken
Copy link
Member

kripken commented Jan 7, 2021

test_pthread_exit_process that was added here seems to fail intermittently on windows, e.g. https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket.appspot.com/8858741851102763552/+/steps/Emscripten_testsuite__upstream__wasm2_/0/stdout

That is the second time I've seen it fail, and last time the next build was fine (still waiting on the next one this time).

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