From 4203e1f2fa48e3e6839e2063a58bcaa6892a2281 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 16 Apr 2024 11:18:48 -0700 Subject: [PATCH 1/2] Rename FileChannelDataBlock to FileDataBlock Rename the internal `FileChannelDataBlock` to `FileDataBlock` since we want to fallback to a `RandomAccessFile` when a thread is interrupted. See gh-40096 --- ...annelDataBlock.java => FileDataBlock.java} | 35 ++++++------- .../boot/loader/zip/ZipContent.java | 25 +++++---- .../AssertFileChannelDataBlocksClosed.java | 6 +-- ...tFileChannelDataBlocksClosedExtension.java | 6 +-- ...ileChannelDataBlockManagedFileChannel.java | 6 +-- ...lockTests.java => FileDataBlockTests.java} | 51 ++++++++++--------- .../loader/zip/VirtualZipDataBlockTests.java | 4 +- 7 files changed, 67 insertions(+), 66 deletions(-) rename spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/{FileChannelDataBlock.java => FileDataBlock.java} (87%) rename spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/{FileChannelDataBlockTests.java => FileDataBlockTests.java} (83%) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/FileChannelDataBlock.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/FileDataBlock.java similarity index 87% rename from spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/FileChannelDataBlock.java rename to spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/FileDataBlock.java index 788841ea308..e94784a561d 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/FileChannelDataBlock.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/FileDataBlock.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -16,6 +16,7 @@ package org.springframework.boot.loader.zip; +import java.io.File; import java.io.IOException; import java.nio.ByteBuffer; import java.nio.channels.ClosedByInterruptException; @@ -29,14 +30,14 @@ import java.util.function.Supplier; import org.springframework.boot.loader.log.DebugLogger; /** - * Reference counted {@link DataBlock} implementation backed by a {@link FileChannel} with + * Reference counted {@link DataBlock} implementation backed by a {@link File} with * support for slicing. * * @author Phillip Webb */ -class FileChannelDataBlock implements CloseableDataBlock { +class FileDataBlock implements CloseableDataBlock { - private static final DebugLogger debug = DebugLogger.get(FileChannelDataBlock.class); + private static final DebugLogger debug = DebugLogger.get(FileDataBlock.class); static Tracker tracker; @@ -46,13 +47,13 @@ class FileChannelDataBlock implements CloseableDataBlock { private final long size; - FileChannelDataBlock(Path path) throws IOException { + FileDataBlock(Path path) throws IOException { this.channel = new ManagedFileChannel(path); this.offset = 0; this.size = Files.size(path); } - FileChannelDataBlock(ManagedFileChannel channel, long offset, long size) { + FileDataBlock(ManagedFileChannel channel, long offset, long size) { this.channel = channel; this.offset = offset; this.size = size; @@ -115,26 +116,26 @@ class FileChannelDataBlock implements CloseableDataBlock { } /** - * Return a new {@link FileChannelDataBlock} slice providing access to a subset of the - * data. The caller is responsible for calling {@link #open()} and {@link #close()} on - * the returned block. + * Return a new {@link FileDataBlock} slice providing access to a subset of the data. + * The caller is responsible for calling {@link #open()} and {@link #close()} on the + * returned block. * @param offset the start offset for the slice relative to this block - * @return a new {@link FileChannelDataBlock} instance + * @return a new {@link FileDataBlock} instance * @throws IOException on I/O error */ - FileChannelDataBlock slice(long offset) throws IOException { + FileDataBlock slice(long offset) throws IOException { return slice(offset, this.size - offset); } /** - * Return a new {@link FileChannelDataBlock} slice providing access to a subset of the - * data. The caller is responsible for calling {@link #open()} and {@link #close()} on - * the returned block. + * Return a new {@link FileDataBlock} slice providing access to a subset of the data. + * The caller is responsible for calling {@link #open()} and {@link #close()} on the + * returned block. * @param offset the start offset for the slice relative to this block * @param size the size of the new slice - * @return a new {@link FileChannelDataBlock} instance + * @return a new {@link FileDataBlock} instance */ - FileChannelDataBlock slice(long offset, long size) { + FileDataBlock slice(long offset, long size) { if (offset == 0 && size == this.size) { return this; } @@ -145,7 +146,7 @@ class FileChannelDataBlock implements CloseableDataBlock { throw new IllegalArgumentException("Size must not be negative and must be within bounds"); } debug.log("Slicing %s at %s with size %s", this.channel, offset, size); - return new FileChannelDataBlock(this.channel, this.offset + offset, size); + return new FileDataBlock(this.channel, this.offset + offset, size); } /** diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipContent.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipContent.java index b5afa6311ec..d9c5f1c689c 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipContent.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/ZipContent.java @@ -74,7 +74,7 @@ public final class ZipContent implements Closeable { private final Kind kind; - private final FileChannelDataBlock data; + private final FileDataBlock data; private final long centralDirectoryPos; @@ -96,7 +96,7 @@ public final class ZipContent implements Closeable { private SoftReference, Object>> info; - private ZipContent(Source source, Kind kind, FileChannelDataBlock data, long centralDirectoryPos, long commentPos, + private ZipContent(Source source, Kind kind, FileDataBlock data, long centralDirectoryPos, long commentPos, long commentLength, int[] lookupIndexes, int[] nameHashLookups, int[] relativeCentralDirectoryOffsetLookups, NameOffsetLookups nameOffsetLookups, boolean hasJarSignatureFile) { this.source = source; @@ -449,7 +449,7 @@ public final class ZipContent implements Closeable { private final Source source; - private final FileChannelDataBlock data; + private final FileDataBlock data; private final long centralDirectoryPos; @@ -463,8 +463,7 @@ public final class ZipContent implements Closeable { private int cursor; - private Loader(Source source, Entry directoryEntry, FileChannelDataBlock data, long centralDirectoryPos, - int maxSize) { + private Loader(Source source, Entry directoryEntry, FileDataBlock data, long centralDirectoryPos, int maxSize) { this.source = source; this.data = data; this.centralDirectoryPos = centralDirectoryPos; @@ -561,7 +560,7 @@ public final class ZipContent implements Closeable { private static ZipContent loadNonNested(Source source) throws IOException { debug.log("Loading non-nested zip '%s'", source.path()); - return openAndLoad(source, Kind.ZIP, new FileChannelDataBlock(source.path())); + return openAndLoad(source, Kind.ZIP, new FileDataBlock(source.path())); } private static ZipContent loadNestedZip(Source source, Entry entry) throws IOException { @@ -573,7 +572,7 @@ public final class ZipContent implements Closeable { return openAndLoad(source, Kind.NESTED_ZIP, entry.getContent()); } - private static ZipContent openAndLoad(Source source, Kind kind, FileChannelDataBlock data) throws IOException { + private static ZipContent openAndLoad(Source source, Kind kind, FileDataBlock data) throws IOException { try { data.open(); return loadContent(source, kind, data); @@ -584,7 +583,7 @@ public final class ZipContent implements Closeable { } } - private static ZipContent loadContent(Source source, Kind kind, FileChannelDataBlock data) throws IOException { + private static ZipContent loadContent(Source source, Kind kind, FileDataBlock data) throws IOException { ZipEndOfCentralDirectoryRecord.Located locatedEocd = ZipEndOfCentralDirectoryRecord.load(data); ZipEndOfCentralDirectoryRecord eocd = locatedEocd.endOfCentralDirectoryRecord(); long eocdPos = locatedEocd.pos(); @@ -634,7 +633,7 @@ public final class ZipContent implements Closeable { * @return the offset within the data where the archive begins * @throws IOException on I/O error */ - private static long getStartOfZipContent(FileChannelDataBlock data, ZipEndOfCentralDirectoryRecord eocd, + private static long getStartOfZipContent(FileDataBlock data, ZipEndOfCentralDirectoryRecord eocd, Zip64EndOfCentralDirectoryRecord zip64Eocd) throws IOException { long specifiedOffsetToStartOfCentralDirectory = (zip64Eocd != null) ? zip64Eocd.offsetToStartOfCentralDirectory() : eocd.offsetToStartOfCentralDirectory(); @@ -699,7 +698,7 @@ public final class ZipContent implements Closeable { private volatile String name; - private volatile FileChannelDataBlock content; + private volatile FileDataBlock content; /** * Create a new {@link Entry} instance. @@ -789,13 +788,13 @@ public final class ZipContent implements Closeable { * @throws IOException on I/O error */ public CloseableDataBlock openContent() throws IOException { - FileChannelDataBlock content = getContent(); + FileDataBlock content = getContent(); content.open(); return content; } - private FileChannelDataBlock getContent() throws IOException { - FileChannelDataBlock content = this.content; + private FileDataBlock getContent() throws IOException { + FileDataBlock content = this.content; if (content == null) { int pos = this.centralRecord.offsetToLocalHeader(); checkNotZip64Extended(pos); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosed.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosed.java index 75c208e5853..b7e4846f2e3 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosed.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosed.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -25,8 +25,8 @@ import java.lang.annotation.Target; import org.junit.jupiter.api.extension.ExtendWith; /** - * Annotation that can be added to tests to assert that {@link FileChannelDataBlock} files - * are not left open. + * Annotation that can be added to tests to assert that {@link FileDataBlock} files are + * not left open. * * @author Phillip Webb */ diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosedExtension.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosedExtension.java index 21ad19da62c..bb6b5366818 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosedExtension.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosedExtension.java @@ -31,7 +31,7 @@ import org.junit.jupiter.api.extension.BeforeEachCallback; import org.junit.jupiter.api.extension.ExtensionContext; import org.springframework.boot.loader.ref.DefaultCleanerTracking; -import org.springframework.boot.loader.zip.FileChannelDataBlock.Tracker; +import org.springframework.boot.loader.zip.FileDataBlock.Tracker; import static org.assertj.core.api.Assertions.assertThat; @@ -45,14 +45,14 @@ class AssertFileChannelDataBlocksClosedExtension implements BeforeEachCallback, @Override public void beforeEach(ExtensionContext context) throws Exception { tracker.clear(); - FileChannelDataBlock.tracker = tracker; + FileDataBlock.tracker = tracker; DefaultCleanerTracking.set(tracker::addedCleanable); } @Override public void afterEach(ExtensionContext context) throws Exception { tracker.assertAllClosed(); - FileChannelDataBlock.tracker = null; + FileDataBlock.tracker = null; } private static final class OpenFilesTracker implements Tracker { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileChannelDataBlockManagedFileChannel.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileChannelDataBlockManagedFileChannel.java index 6e945dbd43e..00c1f480dfc 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileChannelDataBlockManagedFileChannel.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileChannelDataBlockManagedFileChannel.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -16,7 +16,7 @@ package org.springframework.boot.loader.zip; -import org.springframework.boot.loader.zip.FileChannelDataBlock.ManagedFileChannel; +import org.springframework.boot.loader.zip.FileDataBlock.ManagedFileChannel; /** * Test access to {@link ManagedFileChannel} details. @@ -28,6 +28,6 @@ public final class FileChannelDataBlockManagedFileChannel { private FileChannelDataBlockManagedFileChannel() { } - public static int BUFFER_SIZE = FileChannelDataBlock.ManagedFileChannel.BUFFER_SIZE; + public static int BUFFER_SIZE = FileDataBlock.ManagedFileChannel.BUFFER_SIZE; } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileChannelDataBlockTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileDataBlockTests.java similarity index 83% rename from spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileChannelDataBlockTests.java rename to spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileDataBlockTests.java index b5abefc6aad..ac27a7c43a5 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileChannelDataBlockTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileDataBlockTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -28,17 +28,17 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import org.springframework.boot.loader.zip.FileChannelDataBlock.Tracker; +import org.springframework.boot.loader.zip.FileDataBlock.Tracker; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; /** - * Tests for {@link FileChannelDataBlock}. + * Tests for {@link FileDataBlock}. * * @author Phillip Webb */ -class FileChannelDataBlockTests { +class FileDataBlockTests { private static final byte[] CONTENT = new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 }; @@ -55,19 +55,19 @@ class FileChannelDataBlockTests { @AfterEach void resetTracker() { - FileChannelDataBlock.tracker = null; + FileDataBlock.tracker = null; } @Test void sizeReturnsFileSize() throws IOException { - try (FileChannelDataBlock block = createAndOpenBlock()) { + try (FileDataBlock block = createAndOpenBlock()) { assertThat(block.size()).isEqualTo(CONTENT.length); } } @Test void readReadsFile() throws IOException { - try (FileChannelDataBlock block = createAndOpenBlock()) { + try (FileDataBlock block = createAndOpenBlock()) { ByteBuffer buffer = ByteBuffer.allocate(CONTENT.length); assertThat(block.read(buffer, 0)).isEqualTo(6); assertThat(buffer.array()).containsExactly(CONTENT); @@ -76,7 +76,8 @@ class FileChannelDataBlockTests { @Test void readReadsFileWhenThreadHasBeenInterrupted() throws IOException { - try (FileChannelDataBlock block = createAndOpenBlock()) { + Files.write(this.tempFile.toPath(), CONTENT); + try (FileDataBlock block = createAndOpenBlock()) { ByteBuffer buffer = ByteBuffer.allocate(CONTENT.length); Thread.currentThread().interrupt(); assertThat(block.read(buffer, 0)).isEqualTo(6); @@ -89,7 +90,7 @@ class FileChannelDataBlockTests { @Test void readDoesNotReadPastEndOfFile() throws IOException { - try (FileChannelDataBlock block = createAndOpenBlock()) { + try (FileDataBlock block = createAndOpenBlock()) { ByteBuffer buffer = ByteBuffer.allocate(CONTENT.length); assertThat(block.read(buffer, 2)).isEqualTo(4); assertThat(buffer.array()).containsExactly(0x02, 0x03, 0x04, 0x05, 0x0, 0x0); @@ -98,7 +99,7 @@ class FileChannelDataBlockTests { @Test void readWhenPosAtSizeReturnsMinusOne() throws IOException { - try (FileChannelDataBlock block = createAndOpenBlock()) { + try (FileDataBlock block = createAndOpenBlock()) { ByteBuffer buffer = ByteBuffer.allocate(CONTENT.length); assertThat(block.read(buffer, 6)).isEqualTo(-1); } @@ -106,7 +107,7 @@ class FileChannelDataBlockTests { @Test void readWhenPosOverSizeReturnsMinusOne() throws IOException { - try (FileChannelDataBlock block = createAndOpenBlock()) { + try (FileDataBlock block = createAndOpenBlock()) { ByteBuffer buffer = ByteBuffer.allocate(CONTENT.length); assertThat(block.read(buffer, 7)).isEqualTo(-1); } @@ -114,7 +115,7 @@ class FileChannelDataBlockTests { @Test void readWhenPosIsNegativeThrowsException() throws IOException { - try (FileChannelDataBlock block = createAndOpenBlock()) { + try (FileDataBlock block = createAndOpenBlock()) { ByteBuffer buffer = ByteBuffer.allocate(CONTENT.length); assertThatIllegalArgumentException().isThrownBy(() -> block.read(buffer, -1)); } @@ -122,7 +123,7 @@ class FileChannelDataBlockTests { @Test void sliceWhenOffsetIsNegativeThrowsException() throws IOException { - try (FileChannelDataBlock block = createAndOpenBlock()) { + try (FileDataBlock block = createAndOpenBlock()) { assertThatIllegalArgumentException().isThrownBy(() -> block.slice(-1, 0)) .withMessage("Offset must not be negative"); } @@ -130,7 +131,7 @@ class FileChannelDataBlockTests { @Test void sliceWhenSizeIsNegativeThrowsException() throws IOException { - try (FileChannelDataBlock block = createAndOpenBlock()) { + try (FileDataBlock block = createAndOpenBlock()) { assertThatIllegalArgumentException().isThrownBy(() -> block.slice(0, -1)) .withMessage("Size must not be negative and must be within bounds"); } @@ -138,7 +139,7 @@ class FileChannelDataBlockTests { @Test void sliceWhenSizeIsOutOfBoundsThrowsException() throws IOException { - try (FileChannelDataBlock block = createAndOpenBlock()) { + try (FileDataBlock block = createAndOpenBlock()) { assertThatIllegalArgumentException().isThrownBy(() -> block.slice(2, 5)) .withMessage("Size must not be negative and must be within bounds"); } @@ -146,7 +147,7 @@ class FileChannelDataBlockTests { @Test void sliceReturnsSlice() throws IOException { - try (FileChannelDataBlock slice = createAndOpenBlock().slice(1, 4)) { + try (FileDataBlock slice = createAndOpenBlock().slice(1, 4)) { assertThat(slice.size()).isEqualTo(4); ByteBuffer buffer = ByteBuffer.allocate(4); assertThat(slice.read(buffer, 0)).isEqualTo(4); @@ -157,8 +158,8 @@ class FileChannelDataBlockTests { @Test void openAndCloseHandleReferenceCounting() throws IOException { TestTracker tracker = new TestTracker(); - FileChannelDataBlock.tracker = tracker; - FileChannelDataBlock block = createBlock(); + FileDataBlock.tracker = tracker; + FileDataBlock block = createBlock(); assertThat(block).extracting("channel.referenceCount").isEqualTo(0); tracker.assertOpenCloseCounts(0, 0); block.open(); @@ -184,9 +185,9 @@ class FileChannelDataBlockTests { @Test void openAndCloseSliceHandleReferenceCounting() throws IOException { TestTracker tracker = new TestTracker(); - FileChannelDataBlock.tracker = tracker; - FileChannelDataBlock block = createBlock(); - FileChannelDataBlock slice = block.slice(1, 4); + FileDataBlock.tracker = tracker; + FileDataBlock block = createBlock(); + FileDataBlock slice = block.slice(1, 4); assertThat(block).extracting("channel.referenceCount").isEqualTo(0); tracker.assertOpenCloseCounts(0, 0); block.open(); @@ -215,14 +216,14 @@ class FileChannelDataBlockTests { tracker.assertOpenCloseCounts(2, 2); } - private FileChannelDataBlock createAndOpenBlock() throws IOException { - FileChannelDataBlock block = createBlock(); + private FileDataBlock createAndOpenBlock() throws IOException { + FileDataBlock block = createBlock(); block.open(); return block; } - private FileChannelDataBlock createBlock() throws IOException { - return new FileChannelDataBlock(this.tempFile.toPath()); + private FileDataBlock createBlock() throws IOException { + return new FileDataBlock(this.tempFile.toPath()); } static class TestTracker implements Tracker { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/VirtualZipDataBlockTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/VirtualZipDataBlockTests.java index 33f93bfb0f0..46feb719002 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/VirtualZipDataBlockTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/VirtualZipDataBlockTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 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. @@ -58,7 +58,7 @@ class VirtualZipDataBlockTests { @Test void createContainsValidZipContent() throws IOException { - FileChannelDataBlock data = new FileChannelDataBlock(this.file.toPath()); + FileDataBlock data = new FileDataBlock(this.file.toPath()); data.open(); List centralRecords = new ArrayList<>(); List centralRecordPositions = new ArrayList<>(); From 9b0593efe35ffd5de057324400c2dd7f447b1b24 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 16 Apr 2024 14:29:32 -0700 Subject: [PATCH 2/2] Fallback to RandomAccessFile on ClosedByInterruptException Refine the fix for gh-38611 so that `ClosedByInterruptException` no longer retries in a loop. Our previous fix was flawed due to the fact that another interrupt could occur after we clear the first and whilst we are reading data. If this happens 10 times in a row, we raise an exception and end up causing NoClassDefFoundError errors. Our new approach retains the use of `FileChannel` and a direct buffer up to the point that a `ClosedByInterruptException` is raised or the thread is detected as interrupted. At that point, we temporarily switch to using a `RandomAccessFile` to access the data. This will block the thread until the data has been read. Fixes gh-40096 --- .../boot/loader/zip/FileDataBlock.java | 115 +++++++++++------- .../boot/loader/jar/NestedJarFileTests.java | 2 +- ...tFileChannelDataBlocksClosedExtension.java | 7 +- ...ileChannelDataBlockManagedFileChannel.java | 6 +- .../boot/loader/zip/FileDataBlockTests.java | 39 +++--- 5 files changed, 98 insertions(+), 71 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/FileDataBlock.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/FileDataBlock.java index e94784a561d..cd0a1da4321 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/FileDataBlock.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/zip/FileDataBlock.java @@ -18,6 +18,7 @@ package org.springframework.boot.loader.zip; import java.io.File; import java.io.IOException; +import java.io.RandomAccessFile; import java.nio.ByteBuffer; import java.nio.channels.ClosedByInterruptException; import java.nio.channels.ClosedChannelException; @@ -39,22 +40,22 @@ class FileDataBlock implements CloseableDataBlock { private static final DebugLogger debug = DebugLogger.get(FileDataBlock.class); - static Tracker tracker; + static Tracker tracker = Tracker.NONE; - private final ManagedFileChannel channel; + private final FileAccess fileAccess; private final long offset; private final long size; FileDataBlock(Path path) throws IOException { - this.channel = new ManagedFileChannel(path); + this.fileAccess = new FileAccess(path); this.offset = 0; this.size = Files.size(path); } - FileDataBlock(ManagedFileChannel channel, long offset, long size) { - this.channel = channel; + FileDataBlock(FileAccess fileAccess, long offset, long size) { + this.fileAccess = fileAccess; this.offset = offset; this.size = size; } @@ -79,7 +80,7 @@ class FileDataBlock implements CloseableDataBlock { originalDestinationLimit = dst.limit(); dst.limit(dst.position() + remaining); } - int result = this.channel.read(dst, this.offset + pos); + int result = this.fileAccess.read(dst, this.offset + pos); if (originalDestinationLimit != -1) { dst.limit(originalDestinationLimit); } @@ -92,7 +93,7 @@ class FileDataBlock implements CloseableDataBlock { * @throws IOException on I/O error */ void open() throws IOException { - this.channel.open(); + this.fileAccess.open(); } /** @@ -102,7 +103,7 @@ class FileDataBlock implements CloseableDataBlock { */ @Override public void close() throws IOException { - this.channel.close(); + this.fileAccess.close(); } /** @@ -112,7 +113,7 @@ class FileDataBlock implements CloseableDataBlock { * @throws E if the channel is closed */ void ensureOpen(Supplier exceptionSupplier) throws E { - this.channel.ensureOpen(exceptionSupplier); + this.fileAccess.ensureOpen(exceptionSupplier); } /** @@ -145,14 +146,14 @@ class FileDataBlock implements CloseableDataBlock { if (size < 0 || offset + size > this.size) { throw new IllegalArgumentException("Size must not be negative and must be within bounds"); } - debug.log("Slicing %s at %s with size %s", this.channel, offset, size); - return new FileDataBlock(this.channel, this.offset + offset, size); + debug.log("Slicing %s at %s with size %s", this.fileAccess, offset, size); + return new FileDataBlock(this.fileAccess, this.offset + offset, size); } /** * Manages access to underlying {@link FileChannel}. */ - static class ManagedFileChannel { + static class FileAccess { static final int BUFFER_SIZE = 1024 * 10; @@ -162,6 +163,10 @@ class FileDataBlock implements CloseableDataBlock { private FileChannel fileChannel; + private boolean fileChannelInterrupted; + + private RandomAccessFile randomAccessFile; + private ByteBuffer buffer; private long bufferPosition = -1; @@ -170,7 +175,7 @@ class FileDataBlock implements CloseableDataBlock { private final Object lock = new Object(); - ManagedFileChannel(Path path) { + FileAccess(Path path) { if (!Files.isRegularFile(path)) { throw new IllegalArgumentException(path + " must be a regular file"); } @@ -194,34 +199,45 @@ class FileDataBlock implements CloseableDataBlock { } private void fillBuffer(long position) throws IOException { - for (int i = 0; i < 10; i++) { - boolean interrupted = (i != 0) ? Thread.interrupted() : false; - try { - this.buffer.clear(); - this.bufferSize = this.fileChannel.read(this.buffer, position); - this.bufferPosition = position; - return; - } - catch (ClosedByInterruptException ex) { + if (Thread.currentThread().isInterrupted()) { + fillBufferUsingRandomAccessFile(position); + return; + } + try { + if (this.fileChannelInterrupted) { repairFileChannel(); + this.fileChannelInterrupted = false; } - finally { - if (interrupted) { - Thread.currentThread().interrupt(); - } - } + this.buffer.clear(); + this.bufferSize = this.fileChannel.read(this.buffer, position); + this.bufferPosition = position; + } + catch (ClosedByInterruptException ex) { + this.fileChannelInterrupted = true; + fillBufferUsingRandomAccessFile(position); } - throw new ClosedByInterruptException(); } - private void repairFileChannel() throws IOException { - if (tracker != null) { - tracker.closedFileChannel(this.path, this.fileChannel); + private void fillBufferUsingRandomAccessFile(long position) throws IOException { + if (this.randomAccessFile == null) { + this.randomAccessFile = new RandomAccessFile(this.path.toFile(), "r"); + tracker.openedFileChannel(this.path); } - this.fileChannel = FileChannel.open(this.path, StandardOpenOption.READ); - if (tracker != null) { - tracker.openedFileChannel(this.path, this.fileChannel); + byte[] bytes = new byte[BUFFER_SIZE]; + this.randomAccessFile.seek(position); + int len = this.randomAccessFile.read(bytes); + this.buffer.clear(); + if (len > 0) { + this.buffer.put(bytes, 0, len); } + this.bufferSize = len; + this.bufferPosition = position; + } + + private void repairFileChannel() throws IOException { + tracker.closedFileChannel(this.path); + this.fileChannel = FileChannel.open(this.path, StandardOpenOption.READ); + tracker.openedFileChannel(this.path); } void open() throws IOException { @@ -230,9 +246,7 @@ class FileDataBlock implements CloseableDataBlock { debug.log("Opening '%s'", this.path); this.fileChannel = FileChannel.open(this.path, StandardOpenOption.READ); this.buffer = ByteBuffer.allocateDirect(BUFFER_SIZE); - if (tracker != null) { - tracker.openedFileChannel(this.path, this.fileChannel); - } + tracker.openedFileChannel(this.path); } this.referenceCount++; debug.log("Reference count for '%s' incremented to %s", this.path, this.referenceCount); @@ -251,10 +265,13 @@ class FileDataBlock implements CloseableDataBlock { this.bufferPosition = -1; this.bufferSize = 0; this.fileChannel.close(); - if (tracker != null) { - tracker.closedFileChannel(this.path, this.fileChannel); - } + tracker.closedFileChannel(this.path); this.fileChannel = null; + if (this.randomAccessFile != null) { + this.randomAccessFile.close(); + tracker.closedFileChannel(this.path); + this.randomAccessFile = null; + } } debug.log("Reference count for '%s' decremented to %s", this.path, this.referenceCount); } @@ -262,7 +279,7 @@ class FileDataBlock implements CloseableDataBlock { void ensureOpen(Supplier exceptionSupplier) throws E { synchronized (this.lock) { - if (this.referenceCount == 0 || !this.fileChannel.isOpen()) { + if (this.referenceCount == 0) { throw exceptionSupplier.get(); } } @@ -280,9 +297,21 @@ class FileDataBlock implements CloseableDataBlock { */ interface Tracker { - void openedFileChannel(Path path, FileChannel fileChannel); + Tracker NONE = new Tracker() { + + @Override + public void openedFileChannel(Path path) { + } + + @Override + public void closedFileChannel(Path path) { + } + + }; + + void openedFileChannel(Path path); - void closedFileChannel(Path path, FileChannel fileChannel); + void closedFileChannel(Path path); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/NestedJarFileTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/NestedJarFileTests.java index 40a59c7baae..8dfc3e564c6 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/NestedJarFileTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/NestedJarFileTests.java @@ -303,7 +303,7 @@ class NestedJarFileTests { Cleanable cleanable = mock(Cleanable.class); given(cleaner.register(any(), action.capture())).willReturn(cleanable); try (NestedJarFile jar = new NestedJarFile(this.file, null, null, false, cleaner)) { - Object channel = Extractors.byName("resources.zipContent.data.channel").apply(jar); + Object channel = Extractors.byName("resources.zipContent.data.fileAccess").apply(jar); assertThat(channel).extracting("referenceCount").isEqualTo(1); action.getValue().run(); assertThat(channel).extracting("referenceCount").isEqualTo(0); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosedExtension.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosedExtension.java index bb6b5366818..d302dfcee07 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosedExtension.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/AssertFileChannelDataBlocksClosedExtension.java @@ -19,7 +19,6 @@ package org.springframework.boot.loader.zip; import java.io.Closeable; import java.io.IOException; import java.lang.ref.Cleaner.Cleanable; -import java.nio.channels.FileChannel; import java.nio.file.Path; import java.util.ArrayList; import java.util.LinkedHashSet; @@ -52,7 +51,7 @@ class AssertFileChannelDataBlocksClosedExtension implements BeforeEachCallback, @Override public void afterEach(ExtensionContext context) throws Exception { tracker.assertAllClosed(); - FileDataBlock.tracker = null; + FileDataBlock.tracker = Tracker.NONE; } private static final class OpenFilesTracker implements Tracker { @@ -64,12 +63,12 @@ class AssertFileChannelDataBlocksClosedExtension implements BeforeEachCallback, private final List close = new ArrayList<>(); @Override - public void openedFileChannel(Path path, FileChannel fileChannel) { + public void openedFileChannel(Path path) { this.paths.add(path); } @Override - public void closedFileChannel(Path path, FileChannel fileChannel) { + public void closedFileChannel(Path path) { this.paths.remove(path); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileChannelDataBlockManagedFileChannel.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileChannelDataBlockManagedFileChannel.java index 00c1f480dfc..293383b9acf 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileChannelDataBlockManagedFileChannel.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileChannelDataBlockManagedFileChannel.java @@ -16,10 +16,10 @@ package org.springframework.boot.loader.zip; -import org.springframework.boot.loader.zip.FileDataBlock.ManagedFileChannel; +import org.springframework.boot.loader.zip.FileDataBlock.FileAccess; /** - * Test access to {@link ManagedFileChannel} details. + * Test access to {@link FileAccess} details. * * @author Phillip Webb */ @@ -28,6 +28,6 @@ public final class FileChannelDataBlockManagedFileChannel { private FileChannelDataBlockManagedFileChannel() { } - public static int BUFFER_SIZE = FileDataBlock.ManagedFileChannel.BUFFER_SIZE; + public static int BUFFER_SIZE = FileDataBlock.FileAccess.BUFFER_SIZE; } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileDataBlockTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileDataBlockTests.java index ac27a7c43a5..09569a461fa 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileDataBlockTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/zip/FileDataBlockTests.java @@ -19,7 +19,6 @@ package org.springframework.boot.loader.zip; import java.io.File; import java.io.IOException; import java.nio.ByteBuffer; -import java.nio.channels.FileChannel; import java.nio.file.Files; import java.nio.file.Path; @@ -55,7 +54,7 @@ class FileDataBlockTests { @AfterEach void resetTracker() { - FileDataBlock.tracker = null; + FileDataBlock.tracker = Tracker.NONE; } @Test @@ -160,25 +159,25 @@ class FileDataBlockTests { TestTracker tracker = new TestTracker(); FileDataBlock.tracker = tracker; FileDataBlock block = createBlock(); - assertThat(block).extracting("channel.referenceCount").isEqualTo(0); + assertThat(block).extracting("fileAccess.referenceCount").isEqualTo(0); tracker.assertOpenCloseCounts(0, 0); block.open(); - assertThat(block).extracting("channel.referenceCount").isEqualTo(1); + assertThat(block).extracting("fileAccess.referenceCount").isEqualTo(1); tracker.assertOpenCloseCounts(1, 0); block.open(); - assertThat(block).extracting("channel.referenceCount").isEqualTo(2); + assertThat(block).extracting("fileAccess.referenceCount").isEqualTo(2); tracker.assertOpenCloseCounts(1, 0); block.close(); - assertThat(block).extracting("channel.referenceCount").isEqualTo(1); + assertThat(block).extracting("fileAccess.referenceCount").isEqualTo(1); tracker.assertOpenCloseCounts(1, 0); block.close(); - assertThat(block).extracting("channel.referenceCount").isEqualTo(0); + assertThat(block).extracting("fileAccess.referenceCount").isEqualTo(0); tracker.assertOpenCloseCounts(1, 1); block.open(); - assertThat(block).extracting("channel.referenceCount").isEqualTo(1); + assertThat(block).extracting("fileAccess.referenceCount").isEqualTo(1); tracker.assertOpenCloseCounts(2, 1); block.close(); - assertThat(block).extracting("channel.referenceCount").isEqualTo(0); + assertThat(block).extracting("fileAccess.referenceCount").isEqualTo(0); tracker.assertOpenCloseCounts(2, 2); } @@ -188,31 +187,31 @@ class FileDataBlockTests { FileDataBlock.tracker = tracker; FileDataBlock block = createBlock(); FileDataBlock slice = block.slice(1, 4); - assertThat(block).extracting("channel.referenceCount").isEqualTo(0); + assertThat(block).extracting("fileAccess.referenceCount").isEqualTo(0); tracker.assertOpenCloseCounts(0, 0); block.open(); - assertThat(block).extracting("channel.referenceCount").isEqualTo(1); + assertThat(block).extracting("fileAccess.referenceCount").isEqualTo(1); tracker.assertOpenCloseCounts(1, 0); slice.open(); - assertThat(slice).extracting("channel.referenceCount").isEqualTo(2); + assertThat(slice).extracting("fileAccess.referenceCount").isEqualTo(2); tracker.assertOpenCloseCounts(1, 0); slice.open(); - assertThat(slice).extracting("channel.referenceCount").isEqualTo(3); + assertThat(slice).extracting("fileAccess.referenceCount").isEqualTo(3); tracker.assertOpenCloseCounts(1, 0); slice.close(); - assertThat(slice).extracting("channel.referenceCount").isEqualTo(2); + assertThat(slice).extracting("fileAccess.referenceCount").isEqualTo(2); tracker.assertOpenCloseCounts(1, 0); slice.close(); - assertThat(slice).extracting("channel.referenceCount").isEqualTo(1); + assertThat(slice).extracting("fileAccess.referenceCount").isEqualTo(1); tracker.assertOpenCloseCounts(1, 0); block.close(); - assertThat(block).extracting("channel.referenceCount").isEqualTo(0); + assertThat(block).extracting("fileAccess.referenceCount").isEqualTo(0); tracker.assertOpenCloseCounts(1, 1); slice.open(); - assertThat(slice).extracting("channel.referenceCount").isEqualTo(1); + assertThat(slice).extracting("fileAccess.referenceCount").isEqualTo(1); tracker.assertOpenCloseCounts(2, 1); slice.close(); - assertThat(slice).extracting("channel.referenceCount").isEqualTo(0); + assertThat(slice).extracting("fileAccess.referenceCount").isEqualTo(0); tracker.assertOpenCloseCounts(2, 2); } @@ -233,12 +232,12 @@ class FileDataBlockTests { private int closeCount; @Override - public void openedFileChannel(Path path, FileChannel fileChannel) { + public void openedFileChannel(Path path) { this.openCount++; } @Override - public void closedFileChannel(Path path, FileChannel fileChannel) { + public void closedFileChannel(Path path) { this.closeCount++; }