Close RandomAccessDataFile when direct JarFile is closed

Previously, the underlying RandomAccessDataFile was not closed when
the JarFile that was using it was closed. This causes a problem on
Windows as the open file handle prevents the file from being deleted.

This commit updates JarFile to close the underlying
RandomAccessDataFile when it is closed and has a JarFileType of
DIRECT.

Previously, when accessing the manifest of a jar file that maps to a
nested directory (BOOT-INF/classes) a new JarFile was created from the
root jar file, the manifest was retrieved, and the new JarFile was
closed. This could lead to the underlying RandomAccessDataFile being
closed while it was still in use.

This commit improves JarFile to retrieve the manifest from the
existing outer JarFile, thereby avoiding the need to create and close
a new JarFile.

Unfortunately, PropertiesLauncher creates a number of scenarios where
a JarFile with a type of direct is closed while it’s still being used.
To accommodate this behaviour, RandomAccessDataFile has been updated
so that it can re-open the underlying RandomAccessFile if it is used
after it has been closed.

Closes gh-12296

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/data/RandomAccessDataFile.java

@@ -30,9 +30,7 @@ import java.io.RandomAccessFile;
  */
 public class RandomAccessDataFile implements RandomAccessData {
 
-	private final File file;
+	private final FileAccess fileAccess;
-
-	private final RandomAccessFile randomAccessFile;
 
 	private final long offset;
 
@@ -47,31 +45,21 @@ public class RandomAccessDataFile implements RandomAccessData {
 		if (file == null) {
 			throw new IllegalArgumentException("File must not be null");
 		}
-		try {
+		this.fileAccess = new FileAccess(file);
-			this.randomAccessFile = new RandomAccessFile(file, "r");
-		}
-		catch (FileNotFoundException ex) {
-			throw new IllegalArgumentException(
-					String.format("File %s must exist", file.getAbsolutePath()));
-		}
-		this.file = file;
 		this.offset = 0L;
 		this.length = file.length();
 	}
 
 	/**
 	 * Private constructor used to create a {@link #getSubsection(long, long) subsection}.
-	 * @param file the underlying file
+	 * @param fileAccess provides access to the underlying file
-	 * @param randomAccessFile the random access file from which data is read
 	 * @param offset the offset of the section
 	 * @param length the length of the section
 	 */
-	private RandomAccessDataFile(File file, RandomAccessFile randomAccessFile,
+	private RandomAccessDataFile(FileAccess fileAccess, long offset, long length) {
-			long offset, long length) {
-		this.file = file;
 		this.offset = offset;
 		this.length = length;
-		this.randomAccessFile = randomAccessFile;
+		this.fileAccess = fileAccess;
 	}
 
 	/**
@@ -79,12 +67,12 @@ public class RandomAccessDataFile implements RandomAccessData {
 	 * @return the underlying file
 	 */
 	public File getFile() {
-		return this.file;
+		return this.fileAccess.file;
 	}
 
 	@Override
 	public InputStream getInputStream() throws IOException {
-		return new DataInputStream(this.randomAccessFile);
+		return new DataInputStream();
 	}
 
 	@Override
@@ -92,8 +80,7 @@ public class RandomAccessDataFile implements RandomAccessData {
 		if (offset < 0 || length < 0 || offset + length > this.length) {
 			throw new IndexOutOfBoundsException();
 		}
-		return new RandomAccessDataFile(this.file, this.randomAccessFile,
+		return new RandomAccessDataFile(this.fileAccess, this.offset + offset, length);
-				this.offset + offset, length);
 	}
 
 	@Override
@@ -104,20 +91,32 @@ public class RandomAccessDataFile implements RandomAccessData {
 	@Override
 	public byte[] read(long offset, long length) throws IOException {
 		byte[] bytes = new byte[(int) length];
-		synchronized (this.randomAccessFile) {
+		read(bytes, offset, 0, bytes.length);
-			this.randomAccessFile.seek(this.offset + offset);
-			this.randomAccessFile.read(bytes, 0, (int) length);
-		}
 		return bytes;
 	}
 
+	private int readByte(long position) throws IOException {
+		if (position >= this.length) {
+			return -1;
+		}
+		return this.fileAccess.readByte(this.offset + position);
+	}
+
+	private int read(byte[] bytes, long position, int offset, int length)
+			throws IOException {
+		if (position > this.length) {
+			return -1;
+		}
+		return this.fileAccess.read(bytes, this.offset + position, offset, length);
+	}
+
 	@Override
 	public long getSize() {
 		return this.length;
 	}
 
 	public void close() throws IOException {
-
+		this.fileAccess.close();
 	}
 
 	/**
@@ -126,17 +125,15 @@ public class RandomAccessDataFile implements RandomAccessData {
 	 */
 	private class DataInputStream extends InputStream {
 
-		private RandomAccessFile file;
-
 		private int position;
 
-		DataInputStream(RandomAccessFile file) throws IOException {
-			this.file = file;
-		}
-
 		@Override
 		public int read() throws IOException {
-			return doRead(null, 0, 1);
+			int read = RandomAccessDataFile.this.readByte(this.position);
+			if (read > -1) {
+				moveOn(1);
+			}
+			return read;
 		}
 
 		@Override
@@ -169,17 +166,8 @@ public class RandomAccessDataFile implements RandomAccessData {
 			if (cappedLen <= 0) {
 				return -1;
 			}
-			synchronized (this.file) {
+			return (int) moveOn(
-				this.file.seek(RandomAccessDataFile.this.offset + this.position);
+					RandomAccessDataFile.this.read(b, this.position, off, cappedLen));
-				if (b == null) {
-					int rtn = this.file.read();
-					moveOn(rtn == -1 ? 0 : 1);
-					return rtn;
-				}
-				else {
-					return (int) moveOn(this.file.read(b, off, cappedLen));
-				}
-			}
 		}
 
 		@Override
@@ -209,4 +197,57 @@ public class RandomAccessDataFile implements RandomAccessData {
 
 	}
 
+	private static final class FileAccess {
+
+		private final Object monitor = new Object();
+
+		private final File file;
+
+		private RandomAccessFile randomAccessFile;
+
+		private FileAccess(File file) {
+			this.file = file;
+			openIfNecessary();
+		}
+
+		private int read(byte[] bytes, long position, int offset, int length)
+				throws IOException {
+			synchronized (this.monitor) {
+				openIfNecessary();
+				this.randomAccessFile.seek(position);
+				return this.randomAccessFile.read(bytes, offset, length);
+			}
+		}
+
+		private void openIfNecessary() {
+			if (this.randomAccessFile == null) {
+				try {
+					this.randomAccessFile = new RandomAccessFile(this.file, "r");
+				}
+				catch (FileNotFoundException ex) {
+					throw new IllegalArgumentException(String.format("File %s must exist",
+							this.file.getAbsolutePath()));
+				}
+			}
+		}
+
+		private void close() throws IOException {
+			synchronized (this.monitor) {
+				if (this.randomAccessFile != null) {
+					this.randomAccessFile.close();
+					this.randomAccessFile = null;
+				}
+			}
+		}
+
+		private int readByte(long position) throws IOException {
+			synchronized (this.monitor) {
+				openIfNecessary();
+				this.randomAccessFile.seek(position);
+				return this.randomAccessFile.read();
+			}
+		}
+
+	}
+
 }

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

@@ -26,6 +26,7 @@ import java.net.URLStreamHandler;
 import java.net.URLStreamHandlerFactory;
 import java.util.Enumeration;
 import java.util.Iterator;
+import java.util.function.Supplier;
 import java.util.jar.JarInputStream;
 import java.util.jar.Manifest;
 import java.util.zip.ZipEntry;
@@ -70,6 +71,8 @@ public class JarFile extends java.util.jar.JarFile {
 
 	private JarFileEntries entries;
 
+	private Supplier<Manifest> manifestSupplier;
+
 	private SoftReference<Manifest> manifest;
 
 	private boolean signed;
@@ -103,12 +106,12 @@ public class JarFile extends java.util.jar.JarFile {
 	 */
 	private JarFile(RandomAccessDataFile rootFile, String pathFromRoot,
 			RandomAccessData data, JarFileType type) throws IOException {
-		this(rootFile, pathFromRoot, data, null, type);
+		this(rootFile, pathFromRoot, data, null, type, null);
 	}
 
 	private JarFile(RandomAccessDataFile rootFile, String pathFromRoot,
-			RandomAccessData data, JarEntryFilter filter, JarFileType type)
+			RandomAccessData data, JarEntryFilter filter, JarFileType type,
-			throws IOException {
+			Supplier<Manifest> manifestSupplier) throws IOException {
 		super(rootFile.getFile());
 		this.rootFile = rootFile;
 		this.pathFromRoot = pathFromRoot;
@@ -117,6 +120,17 @@ public class JarFile extends java.util.jar.JarFile {
 		parser.addVisitor(centralDirectoryVisitor());
 		this.data = parser.parse(data, filter == null);
 		this.type = type;
+		this.manifestSupplier = manifestSupplier != null ? manifestSupplier : () -> {
+			try (InputStream inputStream = getInputStream(MANIFEST_NAME)) {
+				if (inputStream == null) {
+					return null;
+				}
+				return new Manifest(inputStream);
+			}
+			catch (IOException ex) {
+				throw new RuntimeException(ex);
+			}
+		};
 	}
 
 	private CentralDirectoryVisitor centralDirectoryVisitor() {
@@ -156,18 +170,11 @@ public class JarFile extends java.util.jar.JarFile {
 	public Manifest getManifest() throws IOException {
 		Manifest manifest = (this.manifest == null ? null : this.manifest.get());
 		if (manifest == null) {
-			if (this.type == JarFileType.NESTED_DIRECTORY) {
+			try {
-				try (JarFile rootJarFile = new JarFile(this.getRootJarFile())) {
+				manifest = this.manifestSupplier.get();
-					manifest = rootJarFile.getManifest();
-				}
 			}
-			else {
+			catch (RuntimeException ex) {
-				try (InputStream inputStream = getInputStream(MANIFEST_NAME)) {
+				throw new IOException(ex);
-					if (inputStream == null) {
-						return null;
-					}
-					manifest = new Manifest(inputStream);
-				}
 			}
 			this.manifest = new SoftReference<>(manifest);
 		}
@@ -266,7 +273,7 @@ public class JarFile extends java.util.jar.JarFile {
 		return new JarFile(this.rootFile,
 				this.pathFromRoot + "!/"
 						+ entry.getName().substring(0, name.length() - 1),
-				this.data, filter, JarFileType.NESTED_DIRECTORY);
+				this.data, filter, JarFileType.NESTED_DIRECTORY, this.manifestSupplier);
 	}
 
 	private JarFile createJarFileFromFileEntry(JarEntry entry) throws IOException {
@@ -289,7 +296,9 @@ public class JarFile extends java.util.jar.JarFile {
 	@Override
 	public void close() throws IOException {
 		super.close();
-		this.rootFile.close();
+		if (this.type == JarFileType.DIRECT) {
+			this.rootFile.close();
+		}
 	}
 
 	/**