Attempt to prevent JarFiles from being left open

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

Author: philwebb

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFile.java

@@ -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
@@ -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;
@@ -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();

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarURLConnection.java

@@ -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;
@@ -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
@@ -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 {
@@ -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) {
@@ -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);
 	}
 
 	/**
@@ -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;
-
-	}
-
 }

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

@@ -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;
@@ -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;
@@ -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() {