From f207bcad28a7831e08f831497df65d600073a12d Mon Sep 17 00:00:00 2001 From: Jonathan Oddy Date: Mon, 2 Oct 2017 15:57:03 +0100 Subject: [PATCH 1/6] Fix connection leak when a status is retryable (#253) --- .../client/loadbalancer/RetryLoadBalancerInterceptor.java | 6 ++++-- .../loadbalancer/RetryLoadBalancerInterceptorTest.java | 5 ++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java index cabf8d45..cbeb58b5 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java @@ -103,8 +103,10 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto ClientHttpResponse response = RetryLoadBalancerInterceptor.this.loadBalancer.execute( serviceName, serviceInstance, requestFactory.createRequest(request, body, execution)); - if(retryPolicy != null && retryPolicy.retryableStatusCode(response.getRawStatusCode())) { - throw new RetryableStatusCodeException(serviceName, response.getRawStatusCode()); + int statusCode = response.getRawStatusCode(); + if(retryPolicy != null && retryPolicy.retryableStatusCode(statusCode)) { + response.close(); + throw new RetryableStatusCodeException(serviceName, statusCode); } return response; } diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java index 307ee8d6..47dc1c1a 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java @@ -1,6 +1,7 @@ package org.springframework.cloud.client.loadbalancer; import java.io.IOException; +import java.io.InputStream; import java.net.URI; import org.junit.After; import org.junit.Before; @@ -130,7 +131,8 @@ public class RetryLoadBalancerInterceptorTest { public void interceptRetryOnStatusCode() throws Throwable { HttpRequest request = mock(HttpRequest.class); when(request.getURI()).thenReturn(new URI("http://foo")); - ClientHttpResponse clientHttpResponseNotFound = new MockClientHttpResponse(new byte[]{}, HttpStatus.NOT_FOUND); + InputStream notFoundStream = mock(InputStream.class); + ClientHttpResponse clientHttpResponseNotFound = new MockClientHttpResponse(notFoundStream, HttpStatus.NOT_FOUND); ClientHttpResponse clientHttpResponseOk = new MockClientHttpResponse(new byte[]{}, HttpStatus.OK); LoadBalancedRetryPolicy policy = mock(LoadBalancedRetryPolicy.class); when(policy.retryableStatusCode(eq(HttpStatus.NOT_FOUND.value()))).thenReturn(true); @@ -148,6 +150,7 @@ public class RetryLoadBalancerInterceptorTest { ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); verify(client, times(2)).execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class)); + verify(notFoundStream, times(1)).close(); assertThat(rsp, is(clientHttpResponseOk)); verify(lbRequestFactory, times(2)).createRequest(request, body, execution); } From 714747d24046382f693ecde7870d0cf5ca5d3a41 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 2 Oct 2017 14:23:03 -0400 Subject: [PATCH 2/6] Update SNAPSHOT to 1.2.4.RELEASE --- docs/pom.xml | 2 +- pom.xml | 4 ++-- spring-cloud-commons-dependencies/pom.xml | 4 ++-- spring-cloud-commons/pom.xml | 2 +- spring-cloud-context/pom.xml | 2 +- spring-cloud-starter/pom.xml | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/pom.xml b/docs/pom.xml index f1301322..8db15e9c 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.BUILD-SNAPSHOT + 1.2.4.RELEASE pom Spring Cloud Commons Docs diff --git a/pom.xml b/pom.xml index d7a21dad..d36a1eb3 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.springframework.cloud spring-cloud-commons-parent - 1.2.4.BUILD-SNAPSHOT + 1.2.4.RELEASE pom Spring Cloud Commons Parent Spring Cloud Commons Parent @@ -11,7 +11,7 @@ org.springframework.cloud spring-cloud-build - 1.3.5.BUILD-SNAPSHOT + 1.3.5.RELEASE diff --git a/spring-cloud-commons-dependencies/pom.xml b/spring-cloud-commons-dependencies/pom.xml index 55e95e5a..1a1da911 100644 --- a/spring-cloud-commons-dependencies/pom.xml +++ b/spring-cloud-commons-dependencies/pom.xml @@ -5,11 +5,11 @@ spring-cloud-dependencies-parent org.springframework.cloud - 1.3.5.BUILD-SNAPSHOT + 1.3.5.RELEASE spring-cloud-commons-dependencies - 1.2.4.BUILD-SNAPSHOT + 1.2.4.RELEASE pom spring-cloud-commons-dependencies Spring Cloud Commons Dependencies diff --git a/spring-cloud-commons/pom.xml b/spring-cloud-commons/pom.xml index e6c6de0c..0f9fa293 100644 --- a/spring-cloud-commons/pom.xml +++ b/spring-cloud-commons/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.BUILD-SNAPSHOT + 1.2.4.RELEASE .. spring-cloud-commons diff --git a/spring-cloud-context/pom.xml b/spring-cloud-context/pom.xml index b5583cc5..87e7d9c3 100644 --- a/spring-cloud-context/pom.xml +++ b/spring-cloud-context/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.BUILD-SNAPSHOT + 1.2.4.RELEASE .. spring-cloud-context diff --git a/spring-cloud-starter/pom.xml b/spring-cloud-starter/pom.xml index 3e2fe7a0..19b43d6d 100644 --- a/spring-cloud-starter/pom.xml +++ b/spring-cloud-starter/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.BUILD-SNAPSHOT + 1.2.4.RELEASE spring-cloud-starter spring-cloud-starter From 3c05d9754c72bcb6afbb0dd7ced0e919a69fad21 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 2 Oct 2017 14:25:47 -0400 Subject: [PATCH 3/6] Going back to snapshots --- docs/pom.xml | 2 +- pom.xml | 4 ++-- spring-cloud-commons-dependencies/pom.xml | 4 ++-- spring-cloud-commons/pom.xml | 2 +- spring-cloud-context/pom.xml | 2 +- spring-cloud-starter/pom.xml | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/pom.xml b/docs/pom.xml index 8db15e9c..f1301322 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.RELEASE + 1.2.4.BUILD-SNAPSHOT pom Spring Cloud Commons Docs diff --git a/pom.xml b/pom.xml index d36a1eb3..d7a21dad 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.springframework.cloud spring-cloud-commons-parent - 1.2.4.RELEASE + 1.2.4.BUILD-SNAPSHOT pom Spring Cloud Commons Parent Spring Cloud Commons Parent @@ -11,7 +11,7 @@ org.springframework.cloud spring-cloud-build - 1.3.5.RELEASE + 1.3.5.BUILD-SNAPSHOT diff --git a/spring-cloud-commons-dependencies/pom.xml b/spring-cloud-commons-dependencies/pom.xml index 1a1da911..55e95e5a 100644 --- a/spring-cloud-commons-dependencies/pom.xml +++ b/spring-cloud-commons-dependencies/pom.xml @@ -5,11 +5,11 @@ spring-cloud-dependencies-parent org.springframework.cloud - 1.3.5.RELEASE + 1.3.5.BUILD-SNAPSHOT spring-cloud-commons-dependencies - 1.2.4.RELEASE + 1.2.4.BUILD-SNAPSHOT pom spring-cloud-commons-dependencies Spring Cloud Commons Dependencies diff --git a/spring-cloud-commons/pom.xml b/spring-cloud-commons/pom.xml index 0f9fa293..e6c6de0c 100644 --- a/spring-cloud-commons/pom.xml +++ b/spring-cloud-commons/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.RELEASE + 1.2.4.BUILD-SNAPSHOT .. spring-cloud-commons diff --git a/spring-cloud-context/pom.xml b/spring-cloud-context/pom.xml index 87e7d9c3..b5583cc5 100644 --- a/spring-cloud-context/pom.xml +++ b/spring-cloud-context/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.RELEASE + 1.2.4.BUILD-SNAPSHOT .. spring-cloud-context diff --git a/spring-cloud-starter/pom.xml b/spring-cloud-starter/pom.xml index 19b43d6d..3e2fe7a0 100644 --- a/spring-cloud-starter/pom.xml +++ b/spring-cloud-starter/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.RELEASE + 1.2.4.BUILD-SNAPSHOT spring-cloud-starter spring-cloud-starter From deb18d2eb9ba62fe24ae9a76845b27e0e01ced26 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 2 Oct 2017 14:25:47 -0400 Subject: [PATCH 4/6] Bumping versions to 1.2.5.BUILD-SNAPSHOT after release --- docs/pom.xml | 2 +- pom.xml | 2 +- spring-cloud-commons-dependencies/pom.xml | 2 +- spring-cloud-commons/pom.xml | 2 +- spring-cloud-context/pom.xml | 2 +- spring-cloud-starter/pom.xml | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/pom.xml b/docs/pom.xml index f1301322..52876400 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.BUILD-SNAPSHOT + 1.2.5.BUILD-SNAPSHOT pom Spring Cloud Commons Docs diff --git a/pom.xml b/pom.xml index d7a21dad..c8cd2c3a 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ 4.0.0 org.springframework.cloud spring-cloud-commons-parent - 1.2.4.BUILD-SNAPSHOT + 1.2.5.BUILD-SNAPSHOT pom Spring Cloud Commons Parent Spring Cloud Commons Parent diff --git a/spring-cloud-commons-dependencies/pom.xml b/spring-cloud-commons-dependencies/pom.xml index 55e95e5a..5ff5d2f9 100644 --- a/spring-cloud-commons-dependencies/pom.xml +++ b/spring-cloud-commons-dependencies/pom.xml @@ -9,7 +9,7 @@ spring-cloud-commons-dependencies - 1.2.4.BUILD-SNAPSHOT + 1.2.5.BUILD-SNAPSHOT pom spring-cloud-commons-dependencies Spring Cloud Commons Dependencies diff --git a/spring-cloud-commons/pom.xml b/spring-cloud-commons/pom.xml index e6c6de0c..ebc286c8 100644 --- a/spring-cloud-commons/pom.xml +++ b/spring-cloud-commons/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.BUILD-SNAPSHOT + 1.2.5.BUILD-SNAPSHOT .. spring-cloud-commons diff --git a/spring-cloud-context/pom.xml b/spring-cloud-context/pom.xml index b5583cc5..fba5ee18 100644 --- a/spring-cloud-context/pom.xml +++ b/spring-cloud-context/pom.xml @@ -6,7 +6,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.BUILD-SNAPSHOT + 1.2.5.BUILD-SNAPSHOT .. spring-cloud-context diff --git a/spring-cloud-starter/pom.xml b/spring-cloud-starter/pom.xml index 3e2fe7a0..e84ff23c 100644 --- a/spring-cloud-starter/pom.xml +++ b/spring-cloud-starter/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-commons-parent - 1.2.4.BUILD-SNAPSHOT + 1.2.5.BUILD-SNAPSHOT spring-cloud-starter spring-cloud-starter From abe5374c792094826bc7c96d36d44473757f4c91 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Mon, 30 Oct 2017 18:56:01 -0400 Subject: [PATCH 5/6] Ensure parent context of bootstrap context is closed. fixes gh-267 --- .../cloud/context/refresh/ContextRefresher.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java index 2b7b59a3..a6ab3737 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java @@ -105,7 +105,20 @@ public class ContextRefresher { } finally { ConfigurableApplicationContext closeable = capture; - closeable.close(); + while (closeable != null) { + try { + closeable.close(); + } + catch (Exception e) { + // Ignore; + } + if (closeable.getParent() instanceof ConfigurableApplicationContext) { + closeable = (ConfigurableApplicationContext) closeable.getParent(); + } + else { + break; + } + } } } From 707787dc13fb012b091af5ff518547f24599664e Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Mon, 30 Oct 2017 19:13:36 -0400 Subject: [PATCH 6/6] Adds test for bootstrap context parent closing --- .../cloud/context/refresh/ContextRefresher.java | 6 +++--- .../context/refresh/ContextRefresherTests.java | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java index e4979db2..acdab8d8 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java @@ -33,7 +33,7 @@ public class ContextRefresher { private static final String REFRESH_ARGS_PROPERTY_SOURCE = "refreshArgs"; - private Set standardSources = new HashSet( + private Set standardSources = new HashSet<>( Arrays.asList(StandardEnvironment.SYSTEM_PROPERTIES_PROPERTY_SOURCE_NAME, StandardEnvironment.SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, StandardServletEnvironment.JNDI_PROPERTY_SOURCE_NAME, @@ -59,7 +59,7 @@ public class ContextRefresher { return keys; } - private void addConfigFilesToEnvironment() { + /* for testing */ ConfigurableApplicationContext addConfigFilesToEnvironment() { ConfigurableApplicationContext capture = null; try { StandardEnvironment environment = copyEnvironment( @@ -117,7 +117,7 @@ public class ContextRefresher { } } } - + return capture; } // Don't use ConfigurableEnvironment.merge() in case there are clashes with property diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java index 1c1a0251..96e7dd52 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherTests.java @@ -71,6 +71,22 @@ public class ContextRefresherTests { assertThat(names).first().isEqualTo("bootstrapProperties"); } + @Test + public void parentContextIsClosed() { + // Use spring.cloud.bootstrap.name to switch off the defaults (which would pick up + // a bootstrapProperties immediately + context = SpringApplication.run(ContextRefresherTests.class, + "--spring.main.webEnvironment=false", "--debug=false", + "--spring.main.bannerMode=OFF", "--spring.cloud.bootstrap.name=refresh"); + ContextRefresher refresher = new ContextRefresher(context, scope); + EnvironmentTestUtils.addEnvironment(context, + "spring.cloud.bootstrap.sources: org.springframework.cloud.context.refresh.ContextRefresherTests.PropertySourceConfiguration\n" + + ""); + ConfigurableApplicationContext refresherContext = refresher.addConfigFilesToEnvironment(); + assertThat(refresherContext.getParent()).isNotNull().isInstanceOf(ConfigurableApplicationContext.class); + ConfigurableApplicationContext parent = (ConfigurableApplicationContext) refresherContext.getParent(); + assertThat(parent.isActive()).isFalse(); + } private List names(MutablePropertySources propertySources) { List list = new ArrayList<>(); for (PropertySource p : propertySources) {