From 2aa1e3b423cc56629a51f989e55bc6029ba77cca Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 1 Nov 2023 13:40:30 +0000 Subject: [PATCH] Refine fix Rather than clearing the interrupted flag, repairing the channel, and retrying the read, we now repair the channel and then rethrow the ClosedByInterruptException. This ensures that the channel can be read by other threads while also allowing the interruption of the current thread to continue. --- .../boot/loader/zip/FileChannelDataBlock.java | 21 +++++++++++-------- .../loader/zip/FileChannelDataBlockTests.java | 21 ++++++++++++++----- 2 files changed, 28 insertions(+), 14 deletions(-) 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/FileChannelDataBlock.java index 9c0cbcc0ea24..0346a87d4d0e 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/FileChannelDataBlock.java @@ -184,15 +184,8 @@ int read(ByteBuffer dst, long position) throws IOException { this.bufferSize = this.fileChannel.read(this.buffer, position); } catch (ClosedByInterruptException ex) { - Thread.interrupted(); - if (tracker != null) { - tracker.closedFileChannel(this.path, this.fileChannel); - } - this.fileChannel = FileChannel.open(this.path, StandardOpenOption.READ); - if (tracker != null) { - tracker.openedFileChannel(this.path, this.fileChannel); - } - this.bufferSize = this.fileChannel.read(this.buffer, position); + repairFileChannel(); + throw ex; } this.bufferPosition = position; } @@ -207,6 +200,16 @@ int read(ByteBuffer dst, long position) throws IOException { } } + private void repairFileChannel() throws IOException { + if (tracker != null) { + tracker.closedFileChannel(this.path, this.fileChannel); + } + this.fileChannel = FileChannel.open(this.path, StandardOpenOption.READ); + if (tracker != null) { + tracker.openedFileChannel(this.path, this.fileChannel); + } + } + void open() throws IOException { synchronized (this.lock) { if (this.referenceCount == 0) { 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/FileChannelDataBlockTests.java index b5abefc6aada..df015ff68d55 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/FileChannelDataBlockTests.java @@ -19,9 +19,11 @@ import java.io.File; import java.io.IOException; import java.nio.ByteBuffer; +import java.nio.channels.ClosedByInterruptException; import java.nio.channels.FileChannel; import java.nio.file.Files; import java.nio.file.Path; +import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -75,16 +77,25 @@ void readReadsFile() throws IOException { } @Test - void readReadsFileWhenThreadHasBeenInterrupted() throws IOException { + void readReadsFileWhenAnotherThreadHasBeenInterrupted() throws IOException, InterruptedException { try (FileChannelDataBlock block = createAndOpenBlock()) { ByteBuffer buffer = ByteBuffer.allocate(CONTENT.length); - Thread.currentThread().interrupt(); + AtomicReference failure = new AtomicReference<>(); + Thread thread = new Thread(() -> { + Thread.currentThread().interrupt(); + try { + block.read(ByteBuffer.allocate(CONTENT.length), 0); + } + catch (IOException ex) { + failure.set(ex); + } + }); + thread.start(); + thread.join(); + assertThat(failure.get()).isInstanceOf(ClosedByInterruptException.class); assertThat(block.read(buffer, 0)).isEqualTo(6); assertThat(buffer.array()).containsExactly(CONTENT); } - finally { - Thread.interrupted(); - } } @Test