hivemq-mqtt-client
hivemq-mqtt-client copied to clipboard
hivemq
Change event loop to use daemon instead of non-daemon threads
The class io.netty.util.concurrent.DefaultThreadFactory defaults to non-daemon thread if not explicitly configured. Threads created with defaults by this ThreadFactory might prevent unaware applcations from quitting.
Type of Change
- [ ] 📚 Examples / docs / tutorials / dependencies update
- [ ] 🔧 Bug fix (non-breaking change which fixes an issue)
- [x] 🥂 Improvement (non-breaking change which improves an existing feature)
- [ ] 🚀 New feature (non-breaking change which adds functionality)
- [ ] 💥 Breaking change (fix or feature that would cause existing functionality to change)
- [ ] 🔐 Security fix
Checklist
- [ ] I've written tests (if applicable) for all new methods and classes that I created.
- [ ] I've added documentation as necessary so users can easily use and understand this feature/fix.
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @ViToni on file. In order for us to review and merge your code, please sign our Contributor License Agreement to get yourself added. You'll find the CLA and more information here: https://github.com/hivemq/hivemq-community/blob/master/CONTRIBUTING.adoc#contributor-license-agreement
![cla-bot[bot] avatar](https://avatars.githubusercontent.com/in/2404?v=4 "Published by a pub.dev verified publisher"){kind=small}
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @ViToni on file. In order for us to review and merge your code, please sign our Contributor License Agreement to get yourself added. You'll find the CLA and more information here: https://github.com/hivemq/hivemq-community/blob/master/CONTRIBUTING.adoc#contributor-license-agreement
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @ViToni on file. In order for us to review and merge your code, please sign our Contributor License Agreement to get yourself added. You'll find the CLA and more information here: https://github.com/hivemq/hivemq-community/blob/master/CONTRIBUTING.adoc#contributor-license-agreement
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @ViToni on file. In order for us to review and merge your code, please sign our Contributor License Agreement to get yourself added. You'll find the CLA and more information here: https://github.com/hivemq/hivemq-community/blob/master/CONTRIBUTING.adoc#contributor-license-agreement
@SgtSilvio @pglombardo Could you please check your CLA signing process? I signed the documents about 3 times now and the state hasn't changed, yet.
Hi @ViToni, thank you for creating the PR. Something in the CLA process seems to be stuck. I reached out internally to see what is going on. Sorry for the inconvenience.
This is a behavioral change. A simple example: if you have a main function that publishes a message via the MQTT client asynchronously, and you don’t block the main thread until the message is fully published/acknowledged, and the MQTT client threads are all daemon thread, the process will exit before successfully sending the message.
Regarding the issue: The MQTT client's threads should stop automatically when not needed anymore. If I remember correctly, the threads are not released while the client still has a session. This might be the cause for your issue. We should rather take a look if this can be improved.
This is a behavioral change. A simple example: if you have a main function that publishes a message via the MQTT client asynchronously, and you don’t block the main thread until the message is fully published/acknowledged, and the MQTT client threads are all daemon thread, the process will exit before successfully sending the message.
Aggreed, this might be a behavioral change, if any user would really rely on this behavior. However I'm asuming users would have more complex applications than just sending individual messages which means there are more dependencies involved, each with their dedicated lifecyle.
IMHO a user himself should take care of, when his application quits and not depend on one of its dependencies to be responsible.
In this case having non-daemon threads makes it actually more difficult to quit the application in case of e.g. a bug or wrong usage of the hivemq-mqtt-client library (such as in #633)
Regarding the issue: The MQTT client's threads should stop automatically when not needed anymore. If I remember correctly, the threads are not released while the client still has a session. This might be the cause for your issue. We should rather take a look if this can be improved.
In #633 we saw that when using cleanSession=false threads can still continue to run. This PR is not meant as a solution or workaround for the behavior seen, but rather a means take such issue less impactful.
In our case we cannot use cleanSession=false and have seen two concurrent sessions (hopefully the right wording) which interfere with each other (means both sessions try to establish a connection) and both sessions have non-daemon threads.
But finally this PR can only be regarded as a suggestion.