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 deadlock when secor.upload.on.shutdown=true #593

Merged
merged 1 commit into from
Feb 28, 2019

Conversation

glasser
Copy link
Contributor

@glasser glasser commented Feb 27, 2019

The secor.upload.on.shutdown feature worked fine if we were shutting down
because we received a signal, but not if we were shutting down because a
Consumer thread had crashed and the UncaughtExceptionHandler had called
System.exit(1). In that case we'd get a deadlock and the program would never
exit.

Change the shutdown hook to do nothing if any Consumer thread called
System.exit.

The simplest way to implement this is to move the UncaughtExceptionHandler into
the Consumer thread itself as a big try block, so I did that.

The secor.upload.on.shutdown feature worked fine if we were shutting down
because we received a signal, but not if we were shutting down because a
Consumer thread had crashed and the UncaughtExceptionHandler had called
System.exit(1). In that case we'd get a deadlock and the program would never
exit.

Change the shutdown hook to do nothing if any Consumer thread called
System.exit.

The simplest way to implement this is to move the UncaughtExceptionHandler into
the Consumer thread itself as a big try block, so I did that.
@glasser
Copy link
Contributor Author

glasser commented Feb 27, 2019

CI failure is maven.twttr.com being down: twitter-archive/commons#418

@HenryCaiHaiying
Copy link
Contributor

Thanks for fixing the twitter maven issue.

@HenryCaiHaiying HenryCaiHaiying merged commit 9e558fe into pinterest:master Feb 28, 2019
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