[modbus] Fix for NullPointerException

[ssalonen]

Fixes #8205

When connection is invalidated (e.g. after I/O error), we have apache pool creating a new (unconnected) connection object (if there are others waiting for a connection), and instantly passivates it. In this corner case we will have null endpoint in PooledObject wrapper, resulting in NPE.

Signed-off-by: Sami Salonen [email protected]

[TravisBuddy]

Travis tests have failed

Hey @ssalonen,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

[ssalonen]

Pre-built jars (in a zip)
org.openhab.io.transport.modbus-2.5.8-SNAPSHOT.zip

[fwolter]

LGTM

[massimilianocasini]

Problem still:
2020-08-10 17:44:56.321 [WARN ] [rt.modbus.internal.ModbusManagerImpl] - Try 1 out of 3 failed when executing request (ModbusPollerThingHandler.ModbusPollerReadRequest@4138a3[slave
Id=1,functionCode=READ_MULTIPLE_REGISTERS,start=40190,length=5,maxTries=3]). Will try again soon. Error was I/O error, so reseting the connection. Error details: net.wimpi.modbus.M
odbusIOException I/O exception: SocketTimeoutException Read timed out [operation ID 62dfe217-7bb8-4ba8-88d9-4ebc4dec54c2]
2020-08-10 17:44:56.605 [WARN ] [rt.modbus.internal.ModbusManagerImpl] - Error invalidating connection in pool for endpoint ModbusTCPSlaveEndpoint@1f0f6d2[address=192.168.178.6,por
t=502]. Error was: java.lang.NullPointerException null
java.lang.NullPointerException: null
at java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936) ~[?:1.8.0_222]
at org.openhab.io.transport.modbus.internal.pooling.ModbusSlaveConnectionFactoryImpl$PooledConnection.maybeResetConnection(ModbusSlaveConnectionFactoryImpl.java:87) ~[?:?]
at org.openhab.io.transport.modbus.internal.pooling.ModbusSlaveConnectionFactoryImpl.passivateObject(ModbusSlaveConnectionFactoryImpl.java:223) ~[?:?]
at org.openhab.io.transport.modbus.internal.pooling.ModbusSlaveConnectionFactoryImpl.passivateObject(ModbusSlaveConnectionFactoryImpl.java:1) ~[?:?]
at org.apache.commons.pool2.impl.GenericKeyedObjectPool.addIdleObject(GenericKeyedObjectPool.java:1272) ~[?:?]
at org.apache.commons.pool2.impl.GenericKeyedObjectPool.addObject(GenericKeyedObjectPool.java:1255) ~[?:?]
at org.apache.commons.pool2.impl.GenericKeyedObjectPool.invalidateObject(GenericKeyedObjectPool.java:589) ~[?:?]
at org.openhab.io.transport.modbus.internal.ModbusManagerImpl.lambda$1(ModbusManagerImpl.java:407) ~[?:?]
at java.util.Optional.ifPresent(Optional.java:159) ~[?:1.8.0_222]
at org.openhab.io.transport.modbus.internal.ModbusManagerImpl.invalidate(ModbusManagerImpl.java:405) ~[?:?]
at org.openhab.io.transport.modbus.internal.ModbusManagerImpl.lambda$9(ModbusManagerImpl.java:642) ~[?:?]
at org.openhab.io.transport.modbus.internal.SimpleStopWatch.timeConsumer(SimpleStopWatch.java:169) ~[?:?]
at org.openhab.io.transport.modbus.internal.ModbusManagerImpl.executeOperation(ModbusManagerImpl.java:642) ~[?:?]
at org.openhab.io.transport.modbus.internal.ModbusManagerImpl.access$2(ModbusManagerImpl.java:525) ~[?:?]
at org.openhab.io.transport.modbus.internal.ModbusManagerImpl$ModbusCommunicationInterfaceImpl.lambda$1(ModbusManagerImpl.java:782) ~[?:?]
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) [?:1.8.0_222]
at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308) [?:1.8.0_222]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:180) [?:1.8.0_222]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:294) [?:1.8.0_222]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:1.8.0_222]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:1.8.0_222]
at java.lang.Thread.run(Thread.java:748) [?:1.8.0_222]

288 │ Active │ 80 │ 2.5.8.202007261630 │ openHAB Add-ons :: Bundles :: IO :: Modbus Transport

[ssalonen]

Hi, thanks for reporting @massimilianocasini.

Your stack trace does not match the new code

org.openhab.io.transport.modbus.internal.pooling.ModbusSlaveConnectionFactoryImpl$PooledConnection.maybeResetConnection(ModbusSlaveConnectionFactoryImpl.java:87)

ModbusSlaveConnectionFactoryImpl.java Line 87 cannot possibly generate a null pointer exception since it is a comment line.

I think you might have an issue the way you have installed a patched version. Do you have multiple versions "Active" when you list all bundles? Can you post full output of

bundle:list -s|grep modbus

[massimilianocasini]

Hi @ssalonen ,

right I have two modbus transport installed:

297 | Active | 80 | 2.5.7 | openHAB Add-ons :: Bundles :: IO :: Modbus Transport
319 | Active | 80 | 2.5.8.202007261630 | openHAB Add-ons :: Bundles :: IO :: Modbus Transport

I have now removed 2.5.7 version.

Let's see if it works well.

Thanks