DATAREDIS-1104 - Suppress connection close failures when unbinding a connection.

We now no longer propagate exceptions that happen during connection disposal but instead we log them on DEBUG level to avoid exception masking.
This commit is contained in:
Mark Paluch
2020-02-25 13:53:46 +01:00
parent 784e6bcb6a
commit f5f3d5de1f
2 changed files with 65 additions and 12 deletions

View File

@@ -24,8 +24,10 @@ import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.aop.framework.ProxyFactory;
import org.springframework.cglib.proxy.MethodProxy;
import org.springframework.dao.DataAccessException;
import org.springframework.data.redis.connection.RedisConnection;
import org.springframework.data.redis.connection.RedisConnectionFactory;
import org.springframework.lang.Nullable;
@@ -236,12 +238,8 @@ public abstract class RedisConnectionUtils {
log.debug("Leaving bound Redis Connection attached.");
}
}
} else {
if (log.isDebugEnabled()) {
log.debug("Closing Redis Connection.");
}
conn.close();
doCloseConnection(conn);
}
}
@@ -264,11 +262,7 @@ public abstract class RedisConnectionUtils {
log.debug("Redis Connection will be closed when outer transaction finished.");
}
} else {
if (log.isDebugEnabled()) {
log.debug("Closing bound connection.");
}
RedisConnection connection = connHolder.getConnection();
connection.close();
doCloseConnection(connHolder.getConnection());
}
}
@@ -290,6 +284,21 @@ public abstract class RedisConnectionUtils {
return (connHolder != null && conn == connHolder.getConnection());
}
private static void doCloseConnection(RedisConnection connection) {
if (log.isDebugEnabled()) {
log.debug("Closing Redis Connection.");
}
try {
connection.close();
} catch (DataAccessException ex) {
log.debug("Could not close Redis Connection", ex);
} catch (Throwable ex) {
log.debug("Unexpected exception on closing Redis Connection", ex);
}
}
/**
* A {@link TransactionSynchronizationAdapter} that makes sure that the associated RedisConnection is released after
* the transaction completes.
@@ -326,7 +335,7 @@ public abstract class RedisConnectionUtils {
}
connHolder.setTransactionSyncronisationActive(false);
connection.close();
doCloseConnection(connection);
TransactionSynchronizationManager.unbindResource(factory);
}
}
@@ -370,7 +379,7 @@ public abstract class RedisConnectionUtils {
} finally {
// properly close the unbound connection after executing command
if (!connection.isClosed()) {
connection.close();
doCloseConnection(connection);
}
}
}

View File

@@ -0,0 +1,44 @@
/*
* Copyright 2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.redis.core;
import static org.mockito.Mockito.*;
import org.junit.Test;
import org.springframework.data.redis.connection.RedisConnection;
import org.springframework.data.redis.connection.RedisConnectionFactory;
/**
* Unit tests for {@link RedisConnectionUtils}.
*
* @author Mark Paluch
*/
public class RedisConnectionUtilsUnitTests {
@Test // DATAREDIS-1104
public void shouldSilentlyCloseRedisConnection() {
RedisConnection connectionMock = mock(RedisConnection.class);
RedisConnectionFactory factoryMock = mock(RedisConnectionFactory.class);
doThrow(new IllegalStateException()).when(connectionMock).close();
RedisConnectionUtils.releaseConnection(connectionMock, factoryMock, false);
verify(connectionMock).close();
}
}