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

GH-3249: Fix RemoteFileTemplate dead lock in send

Browse Source 
Fixes: https://github.com/spring-projects/spring-integration/issues/3249

When the `CachingSessionFactory` is configured with small enough pool
and it is very likely that dead lock may happen when `RemoteFileTemplate.send()`
is used.
The problem happens when we reach the `RemoteFileTemplate.exists()` call
which is done from the internal method called from already pulled from cache
`Session`

* Fix `RemoteFileTemplate` to use a `session.exists()` instead on the provided
into the method `Session`
* Demonstrate the problem in the `SftpRemoteFileTemplateTests.testNoDeadLockOnSend()`

**Cherry-pick to 5.2.x, 5.1.x & 4.3.x**

Fix RemoteFileOutboundGWTests for the proper mock
This commit is contained in:
Artem Bilan
2020-04-18 21:06:15 -04:00
committed by Gary Russell
parent baac0a198b
commit e73e08b968
3 changed files with 50 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
spring-integration-file/src/main/java/org/springframework/integration/file/remote/RemoteFileTemplate.java
View File

@@ -386,7 +386,7 @@ public class RemoteFileTemplate<F> implements RemoteFileOperations<F>, Initializ
public boolean get(Message<?> message, InputStreamCallback callback) {
Assert.notNull(this.fileNameProcessor, "A 'fileNameExpression' is needed to use get");
String remotePath = this.fileNameProcessor.processMessage(message);
return this.get(remotePath, callback);
return get(remotePath, callback);
}
@Override
@@ -571,7 +571,7 @@ public class RemoteFileTemplate<F> implements RemoteFileOperations<F>, Initializ
session.append(stream, tempFilePath);
}
else {
if (exists(remoteFilePath)) {
if (session.exists(remoteFilePath)) {
if (FileExistsMode.FAIL.equals(mode)) {
throw new MessagingException(
"The destination file already exists at '" + remoteFilePath + "'.");

18
spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-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.
@@ -22,6 +22,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.BDDMockito.willReturn;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
@@ -804,14 +805,10 @@ public class RemoteFileOutboundGatewayTests {
SessionFactory<TestLsEntry> sessionFactory = mock(SessionFactory.class);
@SuppressWarnings("unchecked")
Session<TestLsEntry> session = mock(Session.class);
RemoteFileTemplate<TestLsEntry> template = new RemoteFileTemplate<TestLsEntry>(sessionFactory) {
@Override
public boolean exists(String path) {
return true;
}
};
willReturn(Boolean.TRUE)
.given(session)
.exists(anyString());
RemoteFileTemplate<TestLsEntry> template = new RemoteFileTemplate<>(sessionFactory);
template.setRemoteDirectoryExpression(new LiteralExpression("foo/"));
template.setBeanFactory(mock(BeanFactory.class));
template.afterPropertiesSet();
@@ -932,7 +929,7 @@ public class RemoteFileOutboundGatewayTests {
TestRemoteFileOutboundGateway gw = new TestRemoteFileOutboundGateway(template, "mput", null);
assertThatIllegalStateException()
.isThrownBy(() -> gw.setRemoteDirectoryExpression(new LiteralExpression("testRemoteDirectory")))
.withMessageContaining("The 'remoteDirectoryExpression' must be set on the externally provided");
.withMessageContaining("The 'remoteDirectoryExpression' must be set on the externally provided");
}
@Test
@@ -942,6 +939,7 @@ public class RemoteFileOutboundGatewayTests {
Session<TestLsEntry> session = mock(Session.class);
TestRemoteFileOutboundGateway gw = new TestRemoteFileOutboundGateway(sessionFactory, "mput", "payload");
gw.setRemoteDirectoryExpression(new LiteralExpression("foo/"));
gw.setBeanFactory(mock(BeanFactory.class));
gw.afterPropertiesSet();
when(sessionFactory.getSession()).thenReturn(session);
final AtomicReference<String> written = new AtomicReference<>();

47
spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpRemoteFileTemplateTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2019 the original author or authors.
* Copyright 2014-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.
@@ -17,6 +17,8 @@
package org.springframework.integration.sftp.session;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.Mockito.mock;
import java.util.Arrays;
import java.util.List;
@@ -25,6 +27,7 @@ import java.util.stream.Collectors;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
@@ -34,11 +37,13 @@ import org.springframework.integration.file.remote.ClientCallbackWithoutResult;
import org.springframework.integration.file.remote.SessionCallbackWithoutResult;
import org.springframework.integration.file.remote.session.CachingSessionFactory;
import org.springframework.integration.file.remote.session.SessionFactory;
import org.springframework.integration.file.support.FileExistsMode;
import org.springframework.integration.sftp.SftpTestSupport;
import org.springframework.messaging.MessageDeliveryException;
import org.springframework.messaging.MessagingException;
import org.springframework.messaging.support.GenericMessage;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.test.context.junit4.SpringRunner;
import com.jcraft.jsch.ChannelSftp;
import com.jcraft.jsch.ChannelSftp.LsEntry;
@@ -47,11 +52,12 @@ import com.jcraft.jsch.SftpException;
/**
* @author Gary Russell
* @author Artem Bilan
*
* @since 4.1
*
*/
@ContextConfiguration
@RunWith(SpringJUnit4ClassRunner.class)
@RunWith(SpringRunner.class)
@DirtiesContext
public class SftpRemoteFileTemplateTests extends SftpTestSupport {
@@ -63,15 +69,19 @@ public class SftpRemoteFileTemplateTests extends SftpTestSupport {
SftpRemoteFileTemplate template = new SftpRemoteFileTemplate(sessionFactory);
DefaultFileNameGenerator fileNameGenerator = new DefaultFileNameGenerator();
fileNameGenerator.setExpression("'foobar.txt'");
fileNameGenerator.setBeanFactory(mock(BeanFactory.class));
template.setFileNameGenerator(fileNameGenerator);
template.setRemoteDirectoryExpression(new LiteralExpression("foo/"));
template.setUseTemporaryFileName(false);
template.setBeanFactory(mock(BeanFactory.class));
template.afterPropertiesSet();
template.execute(session -> {
session.mkdir("foo/");
return session.mkdir("foo/bar/");
});
template.append(new GenericMessage<String>("foo"));
template.append(new GenericMessage<String>("bar"));
template.append(new GenericMessage<>("foo"));
template.append(new GenericMessage<>("bar"));
assertThat(template.exists("foo/foobar.txt")).isTrue();
template.executeWithClient((ClientCallbackWithoutResult<ChannelSftp>) client -> {
try {
@@ -96,6 +106,29 @@ public class SftpRemoteFileTemplateTests extends SftpTestSupport {
assertThat(template.exists("foo")).isFalse();
}
@Test
public void testNoDeadLockOnSend() {
CachingSessionFactory<LsEntry> cachingSessionFactory = new CachingSessionFactory<>(sessionFactory(), 1);
SftpRemoteFileTemplate template = new SftpRemoteFileTemplate(cachingSessionFactory);
template.setRemoteDirectoryExpression(new LiteralExpression(""));
template.setBeanFactory(mock(BeanFactory.class));
template.setUseTemporaryFileName(false);
DefaultFileNameGenerator fileNameGenerator = new DefaultFileNameGenerator();
fileNameGenerator.setExpression("'test.file'");
fileNameGenerator.setBeanFactory(mock(BeanFactory.class));
template.setFileNameGenerator(fileNameGenerator);
template.afterPropertiesSet();
template.send(new GenericMessage<>(""));
assertThatExceptionOfType(MessageDeliveryException.class)
.isThrownBy(() -> template.send(new GenericMessage<>(""), FileExistsMode.FAIL))
.withCauseInstanceOf(MessagingException.class)
.withStackTraceContaining("he destination file already exists at 'test.file'.");
cachingSessionFactory.destroy();
}
@Configuration
public static class Config {