Skip to content

Commit

Permalink
Attempt to prevent JarFiles from being left open
Browse files Browse the repository at this point in the history 
Update `JarFile` so that `super.close()` is called early so that the
file is not left open. Since we re-implement `JarFile` methods to work
directly on the underlying `RandomAccessDataFile`, it should be safe
to close immediately.

See gh-21126
  • Loading branch information
@philwebb
philwebb committed Apr 25, 2020
1 parent 5ed27dd commit 7c6e912
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 50 deletions.
12 changes: 11 additions & 1 deletion ...t-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFile.java
View file
Expand Up @@ -84,6 +84,8 @@ public class JarFile extends java.util.jar.JarFile {

private String comment;

private volatile boolean closed;

/**
* Create a new {@link JarFile} backed by the specified file.
* @param file the root jar file
Expand Down Expand Up @@ -140,6 +142,7 @@ private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccess
private JarFile(JarFile parent, RandomAccessDataFile rootFile, String pathFromRoot, RandomAccessData data,
JarEntryFilter filter, JarFileType type, Supplier<Manifest> manifestSupplier) throws IOException {
super(rootFile.getFile());
super.close();
this.parent = parent;
this.rootFile = rootFile;
this.pathFromRoot = pathFromRoot;
Expand Down Expand Up @@ -321,12 +324,19 @@ public int size() {

@Override
public void close() throws IOException {
super.close();
if (this.closed) {
return;
}
this.closed = true;
if (this.type == JarFileType.DIRECT && this.parent == null) {
this.rootFile.close();
}
}

boolean isClosed() {
return this.closed;
}

String getUrlString() throws MalformedURLException {
if (this.urlString == null) {
this.urlString = getUrl().toString();
Expand Down
56 changes: 10 additions & 46 deletions ...pring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarURLConnection.java
View file
Expand Up @@ -19,7 +19,6 @@
import java.io.ByteArrayOutputStream;
import java.io.FileNotFoundException;
import java.io.FilePermission;
import java.io.FilterInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.UnsupportedEncodingException;
Expand Down Expand Up @@ -81,18 +80,14 @@ protected URLConnection openConnection(URL u) throws IOException {

private final JarEntryName jarEntryName;

private final CloseAction closeAction;

private JarEntry jarEntry;

private JarURLConnection(URL url, JarFile jarFile, JarEntryName jarEntryName, CloseAction closeAction)
throws IOException {
private JarURLConnection(URL url, JarFile jarFile, JarEntryName jarEntryName) throws IOException {
// What we pass to super is ultimately ignored
super(EMPTY_JAR_URL);
this.url = url;
this.jarFile = jarFile;
this.jarEntryName = jarEntryName;
this.closeAction = closeAction;
}

@Override
Expand Down Expand Up @@ -173,17 +168,7 @@ public InputStream getInputStream() throws IOException {
if (inputStream == null) {
throwFileNotFound(this.jarEntryName, this.jarFile);
}
return new FilterInputStream(inputStream) {

@Override
public void close() throws IOException {
super.close();
if (JarURLConnection.this.closeAction != null) {
JarURLConnection.this.closeAction.perform();
}
}

};
return inputStream;
}

private void throwFileNotFound(Object entry, JarFile jarFile) throws FileNotFoundException {
Expand Down Expand Up @@ -264,30 +249,24 @@ static JarURLConnection get(URL url, JarFile jarFile) throws IOException {
int index = indexOfRootSpec(spec, jarFile.getPathFromRoot());
if (index == -1) {
return (Boolean.TRUE.equals(useFastExceptions.get()) ? NOT_FOUND_CONNECTION
: new JarURLConnection(url, null, EMPTY_JAR_ENTRY_NAME, null));
: new JarURLConnection(url, null, EMPTY_JAR_ENTRY_NAME));
}
int separator;
JarFile connectionJarFile = jarFile;
while ((separator = spec.indexOf(SEPARATOR, index)) > 0) {
JarEntryName entryName = JarEntryName.get(spec.subSequence(index, separator));
JarEntry jarEntry = jarFile.getJarEntry(entryName.toCharSequence());
if (jarEntry == null) {
return JarURLConnection.notFound(connectionJarFile, entryName,
(connectionJarFile != jarFile) ? connectionJarFile::close : null);
return JarURLConnection.notFound(jarFile, entryName);
}
connectionJarFile = connectionJarFile.getNestedJarFile(jarEntry);
jarFile = jarFile.getNestedJarFile(jarEntry);
index = separator + SEPARATOR.length();
}
JarEntryName jarEntryName = JarEntryName.get(spec, index);
if (Boolean.TRUE.equals(useFastExceptions.get()) && !jarEntryName.isEmpty()
&& !connectionJarFile.containsEntry(jarEntryName.toString())) {
if (connectionJarFile != jarFile) {
connectionJarFile.close();
}
&& !jarFile.containsEntry(jarEntryName.toString())) {
return NOT_FOUND_CONNECTION;
}
return new JarURLConnection(url, new JarFile(connectionJarFile), jarEntryName,
(connectionJarFile != jarFile) ? connectionJarFile::close : null);
return new JarURLConnection(url, new JarFile(jarFile), jarEntryName);
}

private static int indexOfRootSpec(StringSequence file, String pathFromRoot) {
Expand All @@ -300,22 +279,18 @@ private static int indexOfRootSpec(StringSequence file, String pathFromRoot) {

private static JarURLConnection notFound() {
try {
return notFound(null, null, null);
return notFound(null, null);
}
catch (IOException ex) {
throw new IllegalStateException(ex);
}
}

private static JarURLConnection notFound(JarFile jarFile, JarEntryName jarEntryName, CloseAction closeAction)
throws IOException {
private static JarURLConnection notFound(JarFile jarFile, JarEntryName jarEntryName) throws IOException {
if (Boolean.TRUE.equals(useFastExceptions.get())) {
if (closeAction != null) {
closeAction.perform();
}
return NOT_FOUND_CONNECTION;
}
return new JarURLConnection(null, jarFile, jarEntryName, closeAction);
return new JarURLConnection(null, jarFile, jarEntryName);
}

/**
Expand Down Expand Up @@ -418,15 +393,4 @@ static JarEntryName get(StringSequence spec, int beginIndex) {

}

/**
* An action to be taken when the connection is being "closed" and its underlying
* resources are no longer needed.
*/
@FunctionalInterface
private interface CloseAction {

void perform() throws IOException;

}

}
4 changes: 1 addition & 3 deletions ...-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarURLConnectionTests.java
View file
Expand Up @@ -21,7 +21,6 @@
import java.io.FileNotFoundException;
import java.io.InputStream;
import java.net.URL;
import java.util.zip.ZipFile;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -30,7 +29,6 @@

import org.springframework.boot.loader.TestJarCreator;
import org.springframework.boot.loader.jar.JarURLConnection.JarEntryName;
import org.springframework.test.util.ReflectionTestUtils;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
Expand Down Expand Up @@ -230,7 +228,7 @@ void openConnectionCanBeClosedWithoutClosingSourceJar() throws Exception {
JarURLConnection connection = JarURLConnection.get(url, this.jarFile);
JarFile connectionJarFile = connection.getJarFile();
connectionJarFile.close();
assertThat((Boolean) ReflectionTestUtils.getField(this.jarFile, ZipFile.class, "closeRequested")).isFalse();
assertThat(this.jarFile.isClosed()).isFalse();
}

private String getRelativePath() {
Expand Down

0 comments on commit 7c6e912

Please sign in to comment.