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

Fixed ThreadPool keeping standalone application alive #663

Merged
merged 5 commits into from
Jun 8, 2018

Conversation

littleaj
Copy link
Contributor

@littleaj littleaj commented May 8, 2018

The initializer ThreadPool had a few issues:

  1. it was using user threads instead of daemon threads causing the application to wait until all user threads died.
  2. it had a minimum pool size of 1 so a thread in the pool was always available, but this is just a one-shot operation.
  3. it was an unnamed thread and therefore hard to find.

Fix #662.

TODO

  • Test with standalone application.
  • Audit use of ExecutorService and Runnable to see if there are others.
  • Changes in public surface reviewed
  • CHANGELOG.md updated

@littleaj littleaj added this to the 2.1.1 milestone May 8, 2018
@littleaj littleaj self-assigned this May 8, 2018
@littleaj littleaj requested review from grlima and dhaval24 May 8, 2018 21:29
@littleaj littleaj changed the title Fixed ThreadPool Fixed ThreadPool keeping standalone application alive May 8, 2018
Copy link
Contributor

@dhaval24 dhaval24 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 to me assuming you have tested all the scenarios and smoke tests are ofcourse passing :)

@littleaj littleaj modified the milestones: 2.1.1, 2.1.2 Jun 8, 2018
@littleaj littleaj merged commit 1d1c546 into master Jun 8, 2018
@littleaj littleaj deleted the littleaj/fix_user_threadpool branch June 8, 2018 00:48
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