Skip to content

fix: shut down executorService thread automatically to allow the JVM to shut down gracefully#502

Merged
thinkingserious merged 2 commits intotwilio:masterfrom
Salil999:thread_fix
Mar 5, 2020
Merged

fix: shut down executorService thread automatically to allow the JVM to shut down gracefully#502
thinkingserious merged 2 commits intotwilio:masterfrom
Salil999:thread_fix

Conversation

@Salil999
Copy link
Copy Markdown
Contributor

@Salil999 Salil999 commented Dec 1, 2019

Fixes #431

Initial attempt at trying to mitigate some threading issues in the Twilio API. I'm a bit new to open source, but I think this should try and help (or at least pave the initial steps) for issue #431

I didn't quite see the reason why a boolean was necessary (as mentioned by @bohnman in #431). Regardless if the executor service is set by the user or if it is generated internally, we should shutdown the executor service anyway, am I right?

Oh, and I think there might have been a small typo in the setUsername method. I updated that as well.

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

@thinkingserious thinkingserious changed the title Thread fix fix: shutdown executorService thread automatically to allow the JVM to shut down gracefully Mar 5, 2020
@thinkingserious thinkingserious changed the title fix: shutdown executorService thread automatically to allow the JVM to shut down gracefully fix: shut down executorService thread automatically to allow the JVM to shut down gracefully Mar 5, 2020
@thinkingserious thinkingserious added difficulty: medium fix is medium in difficulty status: ready for merge code deemed fit for merge type: community enhancement feature request not on Twilio's roadmap type: bug bug in the library and removed type: community enhancement feature request not on Twilio's roadmap labels Mar 5, 2020
Copy link
Copy Markdown
Contributor

@thinkingserious thinkingserious left a comment

Choose a reason for hiding this comment

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

Thank you!

LGTM :shipit:

@thinkingserious thinkingserious merged commit c420ca3 into twilio:master Mar 5, 2020
@Salil999 Salil999 deleted the thread_fix branch April 12, 2020 20:27
FalguniV pushed a commit to FalguniV/twilio-java that referenced this pull request Oct 13, 2020
FalguniV pushed a commit to FalguniV/twilio-java that referenced this pull request Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

difficulty: medium fix is medium in difficulty status: ready for merge code deemed fit for merge type: bug bug in the library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

executor service never gets cleaned up

2 participants