Skip to content
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

Avoid requiring EXIT_RUNTIME when running threaded programs under node #22445

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Aug 23, 2024

Prior to this change the thread calling exit() would itself keep the runtime alive.

This change makes it easy to run arbitrary tests under PROXY_TO_PTHREAD without also needing to remember to add EXIT_RUNTIME

@sbc100 sbc100 changed the title vi srAvoid requiring EXIT_RUNTIME when running threaded programs under node Avoid requiring EXIT_RUNTIME when running threaded programs under node Aug 23, 2024
@sbc100 sbc100 requested a review from kripken August 23, 2024 23:28
@sbc100 sbc100 force-pushed the pthread_exit_handling branch 2 times, most recently from c8aa2b7 to 1507300 Compare August 23, 2024 23:30
@sbc100 sbc100 requested a review from dschuff August 26, 2024 16:51
src/library.js Outdated Show resolved Hide resolved
@@ -582,6 +582,7 @@ var LibraryPThread = {
#if PTHREADS_DEBUG
dbg(`cleanupThread: ${ptrToString(pthread_ptr)}`)
#endif
if (!PThread.pthreads[pthread_ptr]) return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this possible now? Maybe add a comment?

src/library.js Outdated Show resolved Hide resolved
src/library.js Outdated
// When EXIT_RUNTIME is enabled the main thread will take care of shutting
// down the runtime, and killing all the workers/threads.
// However when EXIT_RUNTIME is no enabled we don't want the existance
// of this running thread to prevent node from exiting, do we use
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// of this running thread to prevent node from exiting, do we use
// of this running thread to prevent node from exiting, so we use

Prior to this change the thread calling `exit()` would itself keep
the runtime alive.

This change makes it easy to run arbitrary tests under
`PROXY_TO_PTHREAD` without also needing to remember to add
`EXIT_RUNTIME`
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.

3 participants