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

[keba] Don't use selector if it has been closed #7776

Merged
merged 2 commits into from
May 25, 2020

Conversation

kaikreuzer
Copy link
Member

On bundle shutdown, I have seen errors like:

Exception in thread "openHAB-Keba-Transceiver" java.nio.channels.ClosedSelectorException
        at java.base/sun.nio.ch.SelectorImpl.ensureOpen(SelectorImpl.java:75)
        at java.base/sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:118)
        at java.base/sun.nio.ch.SelectorImpl.selectNow(SelectorImpl.java:146)
        at org.openhab.binding.keba.internal.handler.KeContactTransceiver.lambda$0(KeContactTransceiver.java:223)
        at java.base/java.lang.Thread.run(Thread.java:834)

We hence must make sure to only use the selector if it hasn't yet been closed.

Signed-off-by: Kai Kreuzer [email protected]

@kaikreuzer kaikreuzer requested a review from kgoderis as a code owner May 24, 2020 15:18
@TravisBuddy
Copy link

Travis tests were successful

Hey @kaikreuzer,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

Comment on lines 222 to 224
if (!selector.isOpen()) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I can tell ClosedSelectorException could be thrown from any of the selector operations and since the selector is closed from another thread it could be closed on this thread at any time. I don't think that simply checking if it is open at this point is sufficient to prevent ClosedSelectorException from getting thrown.

I think the easiest way to deal with this error is to just catch the ClosedSelectorException.

But a more proper way to prevent this error from getting thrown is to make the transceiverThread responsible for opening and closing the selector and to add logic to allow outside threads to instruct the transceiverThread to close the selector on its own accord.

I'm ok with either solution but I think the first one is better because there are fewer things that can go wrong with it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @cpmeister, this makes a lot of sense indeed. I have updated the PR accordingly.

Signed-off-by: Kai Kreuzer <[email protected]>
@TravisBuddy
Copy link

Travis tests were successful

Hey @kaikreuzer,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

Copy link
Contributor

@cpmeister cpmeister left a comment

Choose a reason for hiding this comment

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

LGTM

@cpmeister cpmeister merged commit 03b8d51 into openhab:2.5.x May 25, 2020
@cpmeister cpmeister added this to the 2.5.6 milestone May 25, 2020
@cpmeister cpmeister added the bug An unexpected problem or unintended behavior of an add-on label May 25, 2020
@kaikreuzer kaikreuzer deleted the kebaclose branch May 26, 2020 06:57
LoungeFlyZ pushed a commit to LoungeFlyZ/openhab2-addons that referenced this pull request Jun 8, 2020
* don't use selector if it has been closed
* addressed review comment

Signed-off-by: Kai Kreuzer <[email protected]>
J-N-K pushed a commit to J-N-K/openhab-addons that referenced this pull request Jul 14, 2020
* don't use selector if it has been closed
* addressed review comment

Signed-off-by: Kai Kreuzer <[email protected]>
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Jul 26, 2020
* don't use selector if it has been closed
* addressed review comment

Signed-off-by: Kai Kreuzer <[email protected]>
Signed-off-by: CSchlipp <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* don't use selector if it has been closed
* addressed review comment

Signed-off-by: Kai Kreuzer <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* don't use selector if it has been closed
* addressed review comment

Signed-off-by: Kai Kreuzer <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* don't use selector if it has been closed
* addressed review comment

Signed-off-by: Kai Kreuzer <[email protected]>
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Aug 31, 2020
* don't use selector if it has been closed
* addressed review comment

Signed-off-by: Kai Kreuzer <[email protected]>
DaanMeijer pushed a commit to DaanMeijer/openhab-addons that referenced this pull request Sep 1, 2020
* don't use selector if it has been closed
* addressed review comment

Signed-off-by: Kai Kreuzer <[email protected]>
Signed-off-by: Daan Meijer <[email protected]>
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
* don't use selector if it has been closed
* addressed review comment

Signed-off-by: Kai Kreuzer <[email protected]>
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.

3 participants