Attempt to fix NestedJarFile file lock issues on Windows

Update `DefaultCleanerTracking` and `@AssertFileChannelDataBlocksClosed`
to capture and store the source object if it is a `Cleanable` so that
it can be released later.

Although the real cleaner cannot keep a reference to `obj`, it is safe
for us to do so in tests since we are in control of the object lifecycle
and we don't need it to be garbage collected.

This commit also updates the `UrlJarFile` to call the cleaner so that
it can be tracked.

See gh-37668
pull/37801/head
Phillip Webb 1 year ago
parent 5da31aca46
commit 9e4f160c17

@ -24,6 +24,8 @@ import java.util.jar.Manifest;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
import org.springframework.boot.loader.ref.Cleaner;
/**
* A {@link JarFile} subclass returned from a {@link JarUrlConnection}.
*
@ -37,6 +39,8 @@ class UrlJarFile extends JarFile {
UrlJarFile(File file, Runtime.Version version, Consumer<JarFile> closeAction) throws IOException {
super(file, true, ZipFile.OPEN_READ, version);
// Registered only for test cleanup since parent class is JarFile
Cleaner.instance.register(this, null);
this.manifest = new UrlJarManifest(super::getManifest);
this.closeAction = closeAction;
}
@ -53,7 +57,9 @@ class UrlJarFile extends JarFile {
@Override
public void close() throws IOException {
this.closeAction.accept(this);
if (this.closeAction != null) {
this.closeAction.accept(this);
}
super.close();
}

@ -56,7 +56,9 @@ class UrlNestedJarFile extends NestedJarFile {
@Override
public void close() throws IOException {
this.closeAction.accept(this);
if (this.closeAction != null) {
this.closeAction.accept(this);
}
super.close();
}

@ -17,7 +17,7 @@
package org.springframework.boot.loader.ref;
import java.lang.ref.Cleaner.Cleanable;
import java.util.function.Consumer;
import java.util.function.BiConsumer;
/**
* Default {@link Cleaner} implementation that delegates to {@link java.lang.ref.Cleaner}.
@ -28,15 +28,15 @@ class DefaultCleaner implements Cleaner {
static final DefaultCleaner instance = new DefaultCleaner();
static Consumer<Cleanable> tracker;
static BiConsumer<Object, Cleanable> tracker;
private final java.lang.ref.Cleaner cleaner = java.lang.ref.Cleaner.create();
@Override
public Cleanable register(Object obj, Runnable action) {
Cleanable cleanable = this.cleaner.register(obj, action);
Cleanable cleanable = (action != null) ? this.cleaner.register(obj, action) : null;
if (tracker != null) {
tracker.accept(cleanable);
tracker.accept(obj, cleanable);
}
return cleanable;
}

@ -26,6 +26,7 @@ import org.junit.jupiter.api.io.TempDir;
import org.springframework.boot.loader.launch.Archive.Entry;
import org.springframework.boot.loader.testsupport.TestJar;
import org.springframework.boot.loader.zip.AssertFileChannelDataBlocksClosed;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
@ -40,6 +41,7 @@ import static org.mockito.Mockito.withSettings;
*
* @author Phillip Webb
*/
@AssertFileChannelDataBlocksClosed
class ArchiveTests {
@TempDir
@ -75,8 +77,9 @@ class ArchiveTests {
CodeSource codeSource = mock(CodeSource.class);
given(protectionDomain.getCodeSource()).willReturn(codeSource);
given(codeSource.getLocation()).willReturn(jarFile.toURI().toURL());
Archive archive = Archive.create(protectionDomain);
assertThat(archive).isInstanceOf(JarFileArchive.class);
try (Archive archive = Archive.create(protectionDomain)) {
assertThat(archive).isInstanceOf(JarFileArchive.class);
}
}
@Test
@ -99,13 +102,17 @@ class ArchiveTests {
void createFromFileWhenJarFileReturnsJarFileArchive() throws Exception {
File target = new File(this.temp, "missing");
TestJar.create(target);
assertThat(Archive.create(target)).isInstanceOf(JarFileArchive.class);
try (Archive archive = Archive.create(target)) {
assertThat(archive).isInstanceOf(JarFileArchive.class);
}
}
@Test
void createFromFileWhenDirectoryReturnsExplodedFileArchive() throws Exception {
File target = this.temp;
assertThat(Archive.create(target)).isInstanceOf(ExplodedArchive.class);
try (Archive archive = Archive.create(target)) {
assertThat(archive).isInstanceOf(ExplodedArchive.class);
}
}
}

@ -34,6 +34,7 @@ import org.mockito.MockitoAnnotations;
import org.springframework.boot.loader.net.protocol.Handlers;
import org.springframework.boot.loader.testsupport.TestJar;
import org.springframework.boot.loader.zip.AssertFileChannelDataBlocksClosed;
import static org.assertj.core.api.Assertions.assertThat;
@ -42,6 +43,7 @@ import static org.assertj.core.api.Assertions.assertThat;
*
* @author Phillip Webb
*/
@AssertFileChannelDataBlocksClosed
class UrlJarFileFactoryTests {
@TempDir

@ -17,7 +17,7 @@
package org.springframework.boot.loader.ref;
import java.lang.ref.Cleaner.Cleanable;
import java.util.function.Consumer;
import java.util.function.BiConsumer;
/**
* Utility that allows tests to set a tracker on {@link DefaultCleaner}.
@ -29,7 +29,7 @@ public final class DefaultCleanerTracking {
private DefaultCleanerTracking() {
}
public static void set(Consumer<Cleanable> tracker) {
public static void set(BiConsumer<Object, Cleanable> tracker) {
DefaultCleaner.tracker = tracker;
}

@ -16,6 +16,8 @@
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;
@ -57,7 +59,9 @@ class AssertFileChannelDataBlocksClosedExtension implements BeforeEachCallback,
private final Set<Path> paths = new LinkedHashSet<>();
private final List<Cleanable> cleanup = new ArrayList<>();
private final List<Cleanable> clean = new ArrayList<>();
private final List<Closeable> close = new ArrayList<>();
@Override
public void openedFileChannel(Path path, FileChannel fileChannel) {
@ -71,16 +75,24 @@ class AssertFileChannelDataBlocksClosedExtension implements BeforeEachCallback,
void clear() {
this.paths.clear();
this.cleanup.clear();
this.clean.clear();
}
void assertAllClosed() {
this.cleanup.forEach(Cleanable::clean);
void assertAllClosed() throws IOException {
for (Closeable closeable : this.close) {
closeable.close();
}
this.clean.forEach(Cleanable::clean);
assertThat(this.paths).as("open paths").isEmpty();
}
private void addedCleanable(Cleanable cleanable) {
this.cleanup.add(cleanable);
private void addedCleanable(Object obj, Cleanable cleanable) {
if (cleanable != null) {
this.clean.add(cleanable);
}
if (obj instanceof Closeable closeable) {
this.close.add(closeable);
}
}
}

Loading…
Cancel
Save