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

Attempt to fix memory leak in JarFile class

Browse Source 
Create a new `JarFileWrapper` class so that we can wrap and existing
`JarFile` and offer a version that can be safely closed.

Prior to this commit, we provided wrapper functionality in the `JarFile`
class itself. Unfortunately, because we override `close` and also create
a lot of wrappers this caused memory issues when running on Java 11.

With Java 11 `java.util.zip.ZipFile` class uses `FinalizableResource`
for any implementation that overrides `close()`. This means that any
wrapper classes will not be garbage collected until the JVM finalizer
thread runs.

Closes gh-22991
This commit is contained in:
Phillip Webb
2020-08-18 11:36:36 -07:00
parent 460fb3ccb8
commit aac367e9c5
8 changed files with 385 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/HandlerTests.java
View File

@@ -171,11 +171,12 @@ class HandlerTests {
TestJarCreator.createTestJar(testJar);
URL url = new URL(null, "jar:" + testJar.toURI().toURL() + "!/nested.jar!/3.dat", this.handler);
JarURLConnection connection = (JarURLConnection) url.openConnection();
JarFile jarFile = JarFileWrapper.unwrap(connection.getJarFile());
try {
assertThat(connection.getJarFile().getRootJarFile().getFile()).isEqualTo(testJar);
assertThat(jarFile.getRootJarFile().getFile()).isEqualTo(testJar);
}
finally {
connection.getJarFile().close();
jarFile.close();
}
}
@@ -185,11 +186,12 @@ class HandlerTests {
TestJarCreator.createTestJar(testJar);
URL url = new URL(null, "jar:" + testJar.toURI().toURL() + "!/nested.jar!/3.dat", this.handler);
JarURLConnection connection = (JarURLConnection) url.openConnection();
JarFile jarFile = JarFileWrapper.unwrap(connection.getJarFile());
try {
assertThat(connection.getJarFile().getRootJarFile().getFile()).isEqualTo(testJar);
assertThat(jarFile.getRootJarFile().getFile()).isEqualTo(testJar);
}
finally {
connection.getJarFile().close();
jarFile.close();
}
}

11
spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileTests.java
View File

@@ -218,10 +218,10 @@ class JarFileTests {
URL url = this.jarFile.getUrl();
assertThat(url.toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/");
JarURLConnection jarURLConnection = (JarURLConnection) url.openConnection();
assertThat(jarURLConnection.getJarFile().getParent()).isSameAs(this.jarFile);
assertThat(JarFileWrapper.unwrap(jarURLConnection.getJarFile())).isSameAs(this.jarFile);
assertThat(jarURLConnection.getJarEntry()).isNull();
assertThat(jarURLConnection.getContentLength()).isGreaterThan(1);
assertThat(((JarFile) jarURLConnection.getContent()).getParent()).isSameAs(this.jarFile);
assertThat(JarFileWrapper.unwrap((java.util.jar.JarFile) jarURLConnection.getContent())).isSameAs(this.jarFile);
assertThat(jarURLConnection.getContentType()).isEqualTo("x-java/jar");
assertThat(jarURLConnection.getJarFileURL().toURI()).isEqualTo(this.rootJarFile.toURI());
}
@@ -231,7 +231,7 @@ class JarFileTests {
URL url = new URL(this.jarFile.getUrl(), "1.dat");
assertThat(url.toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/1.dat");
JarURLConnection jarURLConnection = (JarURLConnection) url.openConnection();
assertThat(jarURLConnection.getJarFile().getParent()).isSameAs(this.jarFile);
assertThat(JarFileWrapper.unwrap(jarURLConnection.getJarFile())).isSameAs(this.jarFile);
assertThat(jarURLConnection.getJarEntry()).isSameAs(this.jarFile.getJarEntry("1.dat"));
assertThat(jarURLConnection.getContentLength()).isEqualTo(1);
assertThat(jarURLConnection.getContent()).isInstanceOf(InputStream.class);
@@ -285,7 +285,7 @@ class JarFileTests {
URL url = nestedJarFile.getUrl();
assertThat(url.toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/nested.jar!/");
JarURLConnection conn = (JarURLConnection) url.openConnection();
assertThat(conn.getJarFile().getParent()).isSameAs(nestedJarFile);
assertThat(JarFileWrapper.unwrap(conn.getJarFile())).isSameAs(nestedJarFile);
assertThat(conn.getJarFileURL().toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/nested.jar");
assertThat(conn.getInputStream()).isNotNull();
JarInputStream jarInputStream = new JarInputStream(conn.getInputStream());
@@ -314,7 +314,8 @@ class JarFileTests {
URL url = nestedJarFile.getUrl();
assertThat(url.toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/d!/");
assertThat(((JarURLConnection) url.openConnection()).getJarFile().getParent()).isSameAs(nestedJarFile);
JarURLConnection connection = (JarURLConnection) url.openConnection();
assertThat(JarFileWrapper.unwrap(connection.getJarFile())).isSameAs(nestedJarFile);
}
}

140
spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileWrapperTests.java Normal file
View File

@@ -0,0 +1,140 @@
/*
* Copyright 2012-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.boot.loader.jar;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.net.MalformedURLException;
import java.util.jar.JarOutputStream;
import java.util.zip.ZipEntry;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
/**
* Tests for {@link JarFileWrapper}.
*
* @author Phillip Webb
*/
class JarFileWrapperTests {
private JarFile parent;
private JarFileWrapper wrapper;
@BeforeEach
void setup(@TempDir File temp) throws IOException {
this.parent = spy(new JarFile(createTempJar(temp)));
this.wrapper = new JarFileWrapper(this.parent);
}
private File createTempJar(File temp) throws IOException {
File file = new File(temp, "temp.jar");
new JarOutputStream(new FileOutputStream(file)).close();
return file;
}
@Test
void getUrlDelegatesToParent() throws MalformedURLException {
this.wrapper.getUrl();
verify(this.parent).getUrl();
}
@Test
void getTypeDelegatesToParent() {
this.wrapper.getType();
verify(this.parent).getType();
}
@Test
void getPermissionDelegatesToParent() {
this.wrapper.getPermission();
verify(this.parent).getPermission();
}
@Test
void getManifestDelegatesToParent() throws IOException {
this.wrapper.getManifest();
verify(this.parent).getManifest();
}
@Test
void entriesDelegatesToParent() {
this.wrapper.entries();
verify(this.parent).entries();
}
@Test
void getJarEntryDelegatesToParent() {
this.wrapper.getJarEntry("test");
verify(this.parent).getJarEntry("test");
}
@Test
void getEntryDelegatesToParent() {
this.wrapper.getEntry("test");
verify(this.parent).getEntry("test");
}
@Test
void getInputStreamDelegatesToParent() throws IOException {
this.wrapper.getInputStream();
verify(this.parent).getInputStream();
}
@Test
void getEntryInputStreamDelegatesToParent() throws IOException {
ZipEntry entry = new ZipEntry("test");
this.wrapper.getInputStream(entry);
verify(this.parent).getInputStream(entry);
}
@Test
void getCommentDelegatesToParent() {
this.wrapper.getComment();
verify(this.parent).getComment();
}
@Test
void sizeDelegatesToParent() {
this.wrapper.size();
verify(this.parent).size();
}
@Test
void toStringDelegatesToParent() {
assertThat(this.wrapper.toString()).endsWith("/temp.jar");
}
@Test // gh-22991
void wrapperMustNotImplementClose() {
// If the wrapper overrides close then on Java 11 a FinalizableResource
// instance will be used to perform cleanup. This can result in a lot
// of additional memory being used since cleanup only occurs when the
// finalizer thread runs. See gh-22991
assertThatExceptionOfType(NoSuchMethodException.class)
.isThrownBy(() -> JarFileWrapper.class.getDeclaredMethod("close"));
}
}

6
spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarURLConnectionTests.java
View File

@@ -62,14 +62,14 @@ class JarURLConnectionTests {
void connectionToRootUsingAbsoluteUrl() throws Exception {
URL url = new URL("jar:" + this.rootJarFile.toURI().toURL() + "!/");
Object content = JarURLConnection.get(url, this.jarFile).getContent();
assertThat(((JarFile) content).getParent()).isSameAs(this.jarFile);
assertThat(JarFileWrapper.unwrap((java.util.jar.JarFile) content)).isSameAs(this.jarFile);
}
@Test
void connectionToRootUsingRelativeUrl() throws Exception {
URL url = new URL("jar:file:" + getRelativePath() + "!/");
Object content = JarURLConnection.get(url, this.jarFile).getContent();
assertThat(((JarFile) content).getParent()).isSameAs(this.jarFile);
assertThat(JarFileWrapper.unwrap((java.util.jar.JarFile) content)).isSameAs(this.jarFile);
}
@Test
@@ -226,7 +226,7 @@ class JarURLConnectionTests {
void openConnectionCanBeClosedWithoutClosingSourceJar() throws Exception {
URL url = new URL("jar:" + this.rootJarFile.toURI().toURL() + "!/");
JarURLConnection connection = JarURLConnection.get(url, this.jarFile);
JarFile connectionJarFile = connection.getJarFile();
java.util.jar.JarFile connectionJarFile = connection.getJarFile();
connectionJarFile.close();
assertThat(this.jarFile.isClosed()).isFalse();
}