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

Use pthread_exit to terminate raft service. #536

Merged
merged 4 commits into from
Sep 18, 2024

Conversation

szmyd
Copy link
Contributor

@szmyd szmyd commented Sep 12, 2024

Allow the process to handle termination of raft service rather than force process wide exit()

Addresses #535

Allow the process to handle termination of raft service rather than
force process wide `exit()`

Addresses eBay#535
@szmyd szmyd requested a review from greensky00 September 12, 2024 23:37
@szmyd szmyd linked an issue Sep 12, 2024 that may be closed by this pull request
@szmyd
Copy link
Contributor Author

szmyd commented Sep 12, 2024

I dunno if this should have a compile/runtime condition around it similar to:

#ifdef USE_PTHREAD_EXIT
pthread_exit(nullptr);
#else 
exit(-1)
#endif

to avoid behavior change for those relying on this?

@greensky00
Copy link
Contributor

Does it work correctly on Mac? I believe it doesn't work at least on Windows. We should write a helper function that works differently according to compiler flag, as you mentioned.

@szmyd
Copy link
Contributor Author

szmyd commented Sep 16, 2024

Does it work correctly on Mac? I believe it doesn't work at least on Windows. We should write a helper function that works differently according to compiler flag, as you mentioned.

It definitely doesn't work on Windows; but should on Mac...Mac has pthread_exit(3). I guess the question is, if it's compile time determined what should be the default? exit()?

Copy link
Contributor

@greensky00 greensky00 left a comment

Choose a reason for hiding this comment

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

Looks good, let's revisit later.

@szmyd szmyd merged commit 6741a45 into eBay:master Sep 18, 2024
1 check passed
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.

Use pthread_exit instead of exit to allow proper process shutdown.
2 participants