Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Protect JMS connection creation against prepareConnection errors

Browse Source 
This commit uses a local variable for the creation of a new JMS
Connection so that a rare failure in prepareConnection(...) does not
leave the connection field in a partially initialized state.

If such a JMSException occurs, the intermediary connection is closed.
This commit further defends against close() failures at that point,
by logging the close exception at DEBUG level. As a result, the original
JMSException is always re-thrown.

See gh-29116
Closes gh-30051
This commit is contained in:
Radek Kraus
2023-02-28 16:10:05 +01:00
committed by Simon Baslé
parent 28d11aaf64
commit 8a879c6fed
2 changed files with 97 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

78
spring-jms/src/test/java/org/springframework/jms/connection/SingleConnectionFactoryTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2023 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.
@@ -16,6 +16,9 @@
package org.springframework.jms.connection;
import java.lang.reflect.Field;
import java.util.concurrent.atomic.AtomicInteger;
import javax.jms.Connection;
import javax.jms.ConnectionFactory;
import javax.jms.ExceptionListener;
@@ -30,7 +33,10 @@ import javax.jms.TopicSession;
import org.junit.jupiter.api.Test;
import org.springframework.util.ReflectionUtils;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
@@ -342,6 +348,76 @@ public class SingleConnectionFactoryTests {
assertThat(listener.getCount()).isEqualTo(1);
}
@Test
public void testWithConnectionFactoryAndExceptionListenerAndReconnectOnExceptionWithJMSException() throws Exception {
// Throws JMSException on setExceptionListener() method, but only at the first time
class FailingTestConnection extends TestConnection {
private int setExceptionListenerInvocationCounter;
@Override
public void setExceptionListener(ExceptionListener exceptionListener) throws JMSException {
setExceptionListenerInvocationCounter++;
// Throw JMSException on first invocation
if (setExceptionListenerInvocationCounter == 1) {
throw new JMSException("Test JMSException (setExceptionListener())");
}
super.setExceptionListener(exceptionListener);
}
}
// Prepare base JMS ConnectionFactory
// - createConnection(1st) -> TestConnection,
// - createConnection(2nd and next) -> FailingTestConnection
TestConnection testCon = new TestConnection();
FailingTestConnection failingCon = new FailingTestConnection();
AtomicInteger createConnectionMethodCounter = new AtomicInteger();
ConnectionFactory cf = mock(ConnectionFactory.class);
given(cf.createConnection()).willAnswer(invocation -> {
int methodInvocationCounter = createConnectionMethodCounter.incrementAndGet();
return methodInvocationCounter == 1 ? testCon : failingCon;
});
// Prepare SingleConnectionFactory (setReconnectOnException())
// - internal connection exception listener should be registered
SingleConnectionFactory scf = new SingleConnectionFactory(cf);
scf.setReconnectOnException(true);
Field conField = ReflectionUtils.findField(SingleConnectionFactory.class, "connection");
conField.setAccessible(true);
// Get connection (1st)
Connection con1 = scf.getConnection();
assertThat(createConnectionMethodCounter.get()).isEqualTo(1);
assertThat(con1).isNotNull();
assertThat(con1.getExceptionListener()).isNotNull();
assertThat(con1).isSameAs(testCon);
// Get connection again, the same should be returned (shared connection till some problem)
Connection con2 = scf.getConnection();
assertThat(createConnectionMethodCounter.get()).isEqualTo(1);
assertThat(con2.getExceptionListener()).isNotNull();
assertThat(con2).isSameAs(con1);
// Invoke reset connection to simulate problem with connection
// - SCF exception listener should be invoked -> connection should be set to null
// - next attempt to invoke getConnection() must create new connection
scf.resetConnection();
assertThat(conField.get(scf)).isNull();
// Attempt to get connection again
// - JMSException should be returned from FailingTestConnection
// - connection should be still null (no new connection without exception listener like before fix)
assertThatExceptionOfType(JMSException.class).isThrownBy(() -> scf.getConnection());
assertThat(createConnectionMethodCounter.get()).isEqualTo(2);
assertThat(conField.get(scf)).isNull();
// Attempt to get connection again -> FailingTestConnection should be returned
// - no JMSException is thrown, exception listener should be present
Connection con3 = scf.getConnection();
assertThat(createConnectionMethodCounter.get()).isEqualTo(3);
assertThat(con3).isNotNull();
assertThat(con3).isSameAs(failingCon);
assertThat(con3.getExceptionListener()).isNotNull();
}
@Test
public void testWithConnectionFactoryAndLocalExceptionListenerWithCleanup() throws JMSException {
ConnectionFactory cf = mock(ConnectionFactory.class);