openhab / openhab-core

Core framework of openHAB
https://www.openhab.org/
Eclipse Public License 2.0
913 stars 421 forks source link

PersistentInbox: NPE (Cannot notify the InboxListener) #3212

Closed lolodomo closed 1 year ago

lolodomo commented 1 year ago

At the startup of OH (snapshot 3215):

2022-12-11 08:35:20.402 [INFO ] [ry.FreeboxServerDiscoveryParticipant] - Created a DiscoveryResult for Freebox Server freebox:server:xxx on IP 192.xxx.x.xxx
2022-12-11 08:35:20.933 [ERROR] [g.discovery.internal.PersistentInbox] - Cannot notify the InboxListener 'org.openhab.core.config.discovery.internal.AutomaticInboxProcessor' about a Thing ADDED event!
java.lang.NullPointerException: null
        at org.openhab.core.config.discovery.internal.PersistentInbox.setFlag(PersistentInbox.java:487) ~[?:?]
        at org.openhab.core.config.discovery.internal.AutomaticInboxProcessor.thingAdded(AutomaticInboxProcessor.java:164) ~[?:?]
        at org.openhab.core.config.discovery.internal.PersistentInbox.notifyListeners(PersistentInbox.java:512) ~[?:?]
        at org.openhab.core.config.discovery.internal.PersistentInbox.internalAdd(PersistentInbox.java:283) ~[?:?]
        at java.lang.Iterable.forEach(Iterable.java:75) ~[?:?]
        at org.openhab.core.config.discovery.internal.PersistentInbox.lambda$0(PersistentInbox.java:174) ~[?:?]
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]
        at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305) [?:?]
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305) [?:?]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
        at java.lang.Thread.run(Thread.java:834) [?:?]
lolodomo commented 1 year ago

This is apparently not systematic, I did not get it after a new OH restart.

J-N-K commented 1 year ago

I can fix the NPE but I wonder how this can happen at all. We first add a discovery result to the storage (in PersistentInbox.internalAdd) BEFORE we notify the listeners. One of these listeners (AutomaticInboxProcessor) then tries to set a flag on exactly that result and the recently added result can't be found in the storage.

kaikreuzer commented 1 year ago

I wonder, if we should rather use discoveryResult.getThingUID().getAsString() instead of discoveryResult.getThingUID().toString() as a key for the results in the PersistentInbox? If we have different instances of the ThingUID class, we might indeed get null on a get, if I see this correctly.

J-N-K commented 1 year ago

But AbstractUID.toString calls AbstractUID.getAsString and the methods in ThingUID and UID call super.toString/super.getAsString, so it should be the same. I already had a look at JsonStorage but didn't see anything wrong there that could produce such an issue.

kaikreuzer commented 1 year ago

Ah, you're right, I missed that. Then I don't have any idea either, how this situation can ever happen...