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

[cbus] fix exception from threadpool at startup #11846

Merged
merged 2 commits into from
Jan 2, 2022

Conversation

jpharvey
Copy link
Contributor

Fixes #11845
After changes in 3.2 to the ThreadPool implementation the return from getpool can no longer be cast to ThreadPoolExecutor.
Changing this to ExecutorService fixes this problem and the binding works again.

…ying threadpool implementation

Signed-off-by: John Harvey <john.p.harvey@btinternet.com>
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/c-bus-binding/130366/11

@lolodomo lolodomo added the bug An unexpected problem or unintended behavior of an add-on label Dec 27, 2021
@lolodomo
Copy link
Contributor

@jpharvey : no feedback to review comment?

@lolodomo lolodomo added the awaiting feedback Awaiting feedback from the pull request author label Dec 30, 2021
@jpharvey
Copy link
Contributor Author

@lolodomo I dont understand your question. I added a not that the suggestion in the review doesn't work because the method suggested is private and can't be called.
What else should i do?
I did accidentally start a review at the same time with the same comment in it.

@jlaur
Copy link
Contributor

jlaur commented Dec 30, 2021

doesn't work because the method suggested is private and can't be called.

You are right, I missed that.

@lolodomo
Copy link
Contributor

So we can forget your comment @jlaur , right?

@lolodomo lolodomo removed the awaiting feedback Awaiting feedback from the pull request author label Dec 30, 2021
@jlaur
Copy link
Contributor

jlaur commented Dec 31, 2021

So we can forget your comment @jlaur , right?

Yes, sorry for late reply, a router firmware upgrade didn't go smooth, so had to use the night for fixing my network. Currently I don't have any comments, didn't have time to go back to it. Something was itching me as the way the binding was broken could happen again, so was looking for a way to make it more robust. Anyway, please disregard my comment.


public CBusThreadPoolExecutor(@Nullable String poolName) {
threadPool = (ThreadPoolExecutor) ThreadPoolManager.getPool("binding.cbus-" + poolName);
threadPool = (ExecutorService) ThreadPoolManager.getPool("binding.cbus-" + poolName);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think casting is needed as getPool returns ExecutorService:
https://www.openhab.org/javadoc/latest/org/openhab/core/common/threadpoolmanager#getPool(java.lang.String)
?

Signed-off-by: John Harvey <john.p.harvey@btinternet.com>
@jpharvey
Copy link
Contributor Author

jpharvey commented Jan 2, 2022

@jlaur Thank you for the review. I have remove the cast from that line now,

Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTN

@lolodomo lolodomo merged commit 8eda555 into openhab:main Jan 2, 2022
@lolodomo lolodomo added this to the 3.3 milestone Jan 2, 2022
mischmidt83 pushed a commit to mischmidt83/openhab-addons that referenced this pull request Jan 9, 2022
* update to use executorservice to fix crash caused by change in underlying threadpool implementation

* Remove explicit cast

Signed-off-by: John Harvey <john.p.harvey@btinternet.com>
Signed-off-by: Michael Schmidt <mi.schmidt.83@gmail.com>
moesterheld pushed a commit to moesterheld/openhab-addons that referenced this pull request Jan 18, 2022
* update to use executorservice to fix crash caused by change in underlying threadpool implementation

* Remove explicit cast

Signed-off-by: John Harvey <john.p.harvey@btinternet.com>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Jan 28, 2022
* update to use executorservice to fix crash caused by change in underlying threadpool implementation

* Remove explicit cast

Signed-off-by: John Harvey <john.p.harvey@btinternet.com>
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Apr 27, 2022
* update to use executorservice to fix crash caused by change in underlying threadpool implementation

* Remove explicit cast

Signed-off-by: John Harvey <john.p.harvey@btinternet.com>
Signed-off-by: Nick Waterton <n.waterton@outlook.com>
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
* update to use executorservice to fix crash caused by change in underlying threadpool implementation

* Remove explicit cast

Signed-off-by: John Harvey <john.p.harvey@btinternet.com>
andrasU pushed a commit to andrasU/openhab-addons that referenced this pull request Nov 12, 2022
* update to use executorservice to fix crash caused by change in underlying threadpool implementation

* Remove explicit cast

Signed-off-by: John Harvey <john.p.harvey@btinternet.com>
Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CBUS] Exception after upgrade to OH 3.2.0 release
4 participants