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

fix: prevent shutdown from hanging forever #494

Closed
wants to merge 1 commit into from

Conversation

docwhat
Copy link

@docwhat docwhat commented Nov 14, 2023

Ruby SDK

What did you accomplish?

I prevented SDKShutdownException from being silently eaten by rescue StandardError, littered throughout the code.

If eaten by a thread then that thread will never be able to thread.join and will hang forever.

How to test new changes?

Do you have ideas?

Extra Notes

I ran across this problem in 7.0.3 where def post_metrics caught and threw away SDKShutdownException.

This is still a potential problem in the current version, 8.2.0.

`rescue StandardError` is a common idiom to prevent errors from stopping
long running processes.

To prevent `SDKShutdownException` from being silently eaten by such a
`rescue` we should not inherit from `StandardError`.
@docwhat docwhat requested a review from a team as a code owner November 14, 2023 22:10
@docwhat
Copy link
Author

docwhat commented Nov 29, 2023

I don't understand the CI failures. Can someone tell me how to fix them?

@sanzmauro
Copy link
Contributor

Hi @docwhat, I will take a look at the changes and failures.

cc @agustinona

@besh
Copy link

besh commented Dec 5, 2023

@sanzmauro thanks for taking a look! Do we happen to have an update here?

@chillaq
Copy link
Contributor

chillaq commented Dec 14, 2023

@docwhat @besh I am able to repro the issue and currently pushing the fix, it will be in next release

Thanks
Bilal

@chillaq chillaq closed this Dec 14, 2023
@docwhat
Copy link
Author

docwhat commented Dec 18, 2023

@chillaq Hello!

Please let me know why you removed my name and comment from commit 6d3a3fc.

I followed the contribution instructions, filled out the PR template, and made sure to document (as a comment) why I violated Ruby best practices by not using StandardError as the base class for SDKShutdownException.

Please tell me if I did something wrong so I can fix it.

This tiny change (one word) took around 16 hours of analyzing the code and reading the git history to understand the source of this randomly happening bug. And over 10 years of Ruby experience.

On your end, this bug impacted every customer who ever used the Ruby client. I can't begin to guess the cost in time and money to your company and your customers.

I use my contributions on GitHub as a portfolio to get work and contribute to other open-source projects.

Please consider restoring me as the "author" of commit 6d3a3fc.

You can do that by restoring the original commit message from 37ef0cb or by forcing the author with the --author='Christian Höltje <[email protected]>' flag. You may need to do a git fetch github.com/docwhat/fix-hang-on-shutdown first to get the commit into your checkout.

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.

4 participants