[modbus] Fix possible NPE when connection construction fails (#3490)

Signed-off-by: Sami Salonen <ssalonen@gmail.com>
This commit is contained in:
Sami Salonen 2023-04-15 17:29:36 +03:00 committed by GitHub
parent 7a9b76dfdf
commit fdea66ae75
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 46 additions and 33 deletions

View File

@ -30,14 +30,15 @@ import net.wimpi.modbus.net.ModbusSlaveConnection;
*
*/
@NonNullByDefault
public class ModbusConnectionPool extends GenericKeyedObjectPool<ModbusSlaveEndpoint, ModbusSlaveConnection> {
public class ModbusConnectionPool extends GenericKeyedObjectPool<ModbusSlaveEndpoint, @Nullable ModbusSlaveConnection> {
public ModbusConnectionPool(KeyedPooledObjectFactory<ModbusSlaveEndpoint, ModbusSlaveConnection> factory) {
public ModbusConnectionPool(
KeyedPooledObjectFactory<ModbusSlaveEndpoint, @Nullable ModbusSlaveConnection> factory) {
super(factory, new ModbusPoolConfig());
}
@Override
public void setConfig(@Nullable GenericKeyedObjectPoolConfig<ModbusSlaveConnection> conf) {
public void setConfig(@Nullable GenericKeyedObjectPoolConfig<@Nullable ModbusSlaveConnection> conf) {
if (conf == null) {
return;
} else if (!(conf instanceof ModbusPoolConfig)) {

View File

@ -347,7 +347,7 @@ public class ModbusManagerImpl implements ModbusManager {
* - https://community.openhab.org/t/connection-pooling-in-modbus-binding/5246/
*/
private volatile @Nullable KeyedObjectPool<ModbusSlaveEndpoint, ModbusSlaveConnection> connectionPool;
private volatile @Nullable KeyedObjectPool<ModbusSlaveEndpoint, @Nullable ModbusSlaveConnection> connectionPool;
private volatile @Nullable ModbusSlaveConnectionFactoryImpl connectionFactory;
private volatile Map<PollTask, ScheduledFuture<?>> scheduledPollTasks = new ConcurrentHashMap<>();
/**
@ -360,7 +360,7 @@ public class ModbusManagerImpl implements ModbusManager {
private void constructConnectionPool() {
ModbusSlaveConnectionFactoryImpl connectionFactory = new ModbusSlaveConnectionFactoryImpl(
DEFAULT_POOL_CONFIGURATION);
GenericKeyedObjectPool<ModbusSlaveEndpoint, ModbusSlaveConnection> genericKeyedObjectPool = new ModbusConnectionPool(
GenericKeyedObjectPool<ModbusSlaveEndpoint, @Nullable ModbusSlaveConnection> genericKeyedObjectPool = new ModbusConnectionPool(
connectionFactory);
genericKeyedObjectPool.setSwallowedExceptionListener(new SwallowedExceptionListener() {
@ -379,7 +379,7 @@ public class ModbusManagerImpl implements ModbusManager {
private Optional<ModbusSlaveConnection> borrowConnection(ModbusSlaveEndpoint endpoint) {
Optional<ModbusSlaveConnection> connection = Optional.empty();
KeyedObjectPool<ModbusSlaveEndpoint, ModbusSlaveConnection> pool = connectionPool;
KeyedObjectPool<ModbusSlaveEndpoint, @Nullable ModbusSlaveConnection> pool = connectionPool;
if (pool == null) {
return connection;
}
@ -405,7 +405,7 @@ public class ModbusManagerImpl implements ModbusManager {
}
private void invalidate(ModbusSlaveEndpoint endpoint, Optional<ModbusSlaveConnection> connection) {
KeyedObjectPool<ModbusSlaveEndpoint, ModbusSlaveConnection> pool = connectionPool;
KeyedObjectPool<ModbusSlaveEndpoint, @Nullable ModbusSlaveConnection> pool = connectionPool;
if (pool == null) {
return;
}
@ -423,7 +423,7 @@ public class ModbusManagerImpl implements ModbusManager {
}
private void returnConnection(ModbusSlaveEndpoint endpoint, Optional<ModbusSlaveConnection> connection) {
KeyedObjectPool<ModbusSlaveEndpoint, ModbusSlaveConnection> pool = connectionPool;
KeyedObjectPool<ModbusSlaveEndpoint, @Nullable ModbusSlaveConnection> pool = connectionPool;
if (pool == null) {
return;
}
@ -454,7 +454,7 @@ public class ModbusManagerImpl implements ModbusManager {
*/
private <R, C extends ModbusResultCallback, F extends ModbusFailureCallback<R>, T extends TaskWithEndpoint<R, C, F>> Optional<ModbusSlaveConnection> getConnection(
AggregateStopWatch timer, boolean oneOffTask, @NonNull T task) throws PollTaskUnregistered {
KeyedObjectPool<ModbusSlaveEndpoint, ModbusSlaveConnection> connectionPool = this.connectionPool;
KeyedObjectPool<ModbusSlaveEndpoint, @Nullable ModbusSlaveConnection> connectionPool = this.connectionPool;
if (connectionPool == null) {
return Optional.empty();
}
@ -983,7 +983,7 @@ public class ModbusManagerImpl implements ModbusManager {
@Deactivate
protected void deactivate() {
synchronized (this) {
KeyedObjectPool<ModbusSlaveEndpoint, ModbusSlaveConnection> connectionPool = this.connectionPool;
KeyedObjectPool<ModbusSlaveEndpoint, @Nullable ModbusSlaveConnection> connectionPool = this.connectionPool;
if (connectionPool != null) {
for (ModbusCommunicationInterface commInterface : this.communicationInterfaces) {
try {

View File

@ -32,7 +32,7 @@ import net.wimpi.modbus.net.ModbusSlaveConnection;
*
*/
@NonNullByDefault
public class ModbusPoolConfig extends GenericKeyedObjectPoolConfig<ModbusSlaveConnection> {
public class ModbusPoolConfig extends GenericKeyedObjectPoolConfig<@Nullable ModbusSlaveConnection> {
@SuppressWarnings("unused")
private EvictionPolicy<ModbusSlaveConnection> evictionPolicy = new DefaultEvictionPolicy<>();

View File

@ -59,14 +59,14 @@ import net.wimpi.modbus.net.UDPMasterConnection;
*/
@NonNullByDefault
public class ModbusSlaveConnectionFactoryImpl
extends BaseKeyedPooledObjectFactory<ModbusSlaveEndpoint, ModbusSlaveConnection> {
extends BaseKeyedPooledObjectFactory<ModbusSlaveEndpoint, @Nullable ModbusSlaveConnection> {
class PooledConnection extends DefaultPooledObject<ModbusSlaveConnection> {
class PooledConnection extends DefaultPooledObject<@Nullable ModbusSlaveConnection> {
private volatile long lastConnected;
private volatile @Nullable ModbusSlaveEndpoint endpoint;
public PooledConnection(ModbusSlaveConnection object) {
public PooledConnection(@Nullable ModbusSlaveConnection object) {
super(object);
}
@ -97,6 +97,9 @@ public class ModbusSlaveConnectionFactoryImpl
long localLastConnected = lastConnected;
ModbusSlaveConnection connection = getObject();
if (connection == null) {
return false;
}
EndpointPoolConfiguration configuration = getEndpointPoolConfiguration(localEndpoint);
long reconnectAfterMillis = configuration.getReconnectAfterMillis();
@ -147,8 +150,8 @@ public class ModbusSlaveConnectionFactoryImpl
}
@Override
public ModbusSlaveConnection create(ModbusSlaveEndpoint endpoint) throws Exception {
return endpoint.accept(new ModbusSlaveEndpointVisitor<ModbusSlaveConnection>() {
public @Nullable ModbusSlaveConnection create(ModbusSlaveEndpoint endpoint) throws Exception {
return endpoint.accept(new ModbusSlaveEndpointVisitor<@Nullable ModbusSlaveConnection>() {
@Override
public @Nullable ModbusSlaveConnection visit(ModbusSerialSlaveEndpoint modbusSerialSlavePoolingKey) {
SerialConnection connection = new SerialConnection(modbusSerialSlavePoolingKey.getSerialParameters());
@ -183,27 +186,34 @@ public class ModbusSlaveConnectionFactoryImpl
}
@Override
public PooledObject<ModbusSlaveConnection> wrap(ModbusSlaveConnection connection) {
public PooledObject<@Nullable ModbusSlaveConnection> wrap(@Nullable ModbusSlaveConnection connection) {
return new PooledConnection(connection);
}
@Override
public void destroyObject(ModbusSlaveEndpoint endpoint, @Nullable PooledObject<ModbusSlaveConnection> obj) {
if (obj == null) {
return;
}
logger.trace("destroyObject for connection {} and endpoint {} -> closing the connection", obj.getObject(),
endpoint);
obj.getObject().resetConnection();
}
@Override
public void activateObject(ModbusSlaveEndpoint endpoint, @Nullable PooledObject<ModbusSlaveConnection> obj)
throws Exception {
public void destroyObject(ModbusSlaveEndpoint endpoint,
@Nullable PooledObject<@Nullable ModbusSlaveConnection> obj) {
if (obj == null) {
return;
}
ModbusSlaveConnection connection = obj.getObject();
if (connection == null) {
return;
}
logger.trace("destroyObject for connection {} and endpoint {} -> closing the connection", connection, endpoint);
connection.resetConnection();
}
@Override
public void activateObject(ModbusSlaveEndpoint endpoint,
@Nullable PooledObject<@Nullable ModbusSlaveConnection> obj) throws Exception {
if (obj == null) {
return;
}
ModbusSlaveConnection connection = obj.getObject();
if (connection == null) {
return;
}
try {
EndpointPoolConfiguration config = getEndpointPoolConfiguration(endpoint);
if (!connection.isConnected()) {
@ -226,7 +236,8 @@ public class ModbusSlaveConnectionFactoryImpl
}
@Override
public void passivateObject(ModbusSlaveEndpoint endpoint, @Nullable PooledObject<ModbusSlaveConnection> obj) {
public void passivateObject(ModbusSlaveEndpoint endpoint,
@Nullable PooledObject<@Nullable ModbusSlaveConnection> obj) {
if (obj == null) {
return;
}
@ -238,8 +249,9 @@ public class ModbusSlaveConnectionFactoryImpl
}
@Override
public boolean validateObject(ModbusSlaveEndpoint key, @Nullable PooledObject<ModbusSlaveConnection> p) {
boolean valid = p != null && p.getObject().isConnected();
public boolean validateObject(ModbusSlaveEndpoint key, @Nullable PooledObject<@Nullable ModbusSlaveConnection> p) {
@SuppressWarnings("null") // p.getObject() cannot be null due to short-circuiting boolean condition
boolean valid = p != null && p.getObject() != null && p.getObject().isConnected();
ModbusSlaveConnection slaveConnection = p != null ? p.getObject() : null;
logger.trace("Validating endpoint {} connection {} -> {}", key, slaveConnection, valid);
return valid;
@ -272,7 +284,7 @@ public class ModbusSlaveConnectionFactoryImpl
.orElseGet(() -> defaultPoolConfigurationFactory.apply(endpoint));
}
private void tryConnect(ModbusSlaveEndpoint endpoint, PooledObject<ModbusSlaveConnection> obj,
private void tryConnect(ModbusSlaveEndpoint endpoint, PooledObject<@Nullable ModbusSlaveConnection> obj,
ModbusSlaveConnection connection, EndpointPoolConfiguration config) throws Exception {
if (connection.isConnected()) {
return;