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
This commit is contained in:
Phillip Webb
2024-04-16 14:29:32 -07:00
parent 4203e1f2fa
commit 9b0593efe3
5 changed files with 100 additions and 73 deletions

View File

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

View File

@@ -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<Closeable> 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);
}

View File

@@ -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;
}

View File

@@ -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++;
}