From 834db304d35fcd0a84500be387d5c1eb877ff93b Mon Sep 17 00:00:00 2001 From: Col-E Date: Fri, 9 Jun 2023 16:29:15 -0400 Subject: [PATCH] Share 'cleaned' reference across slices, do not call close() in finalization --- pom.xml | 2 +- .../software/coley/llzip/util/BufferData.java | 32 +++++++------------ .../coley/llzip/util/FileMapUtil.java | 3 +- .../coley/llzip/util/UnsafeMappedFile.java | 29 +++++++---------- 4 files changed, 26 insertions(+), 40 deletions(-) diff --git a/pom.xml b/pom.xml index bb459bb..fcf20da 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ software.coley lljzip - 1.3.2 + 1.3.3 LL Java ZIP Lower level ZIP support for Java diff --git a/src/main/java/software/coley/llzip/util/BufferData.java b/src/main/java/software/coley/llzip/util/BufferData.java index e6fff1e..db7dc90 100644 --- a/src/main/java/software/coley/llzip/util/BufferData.java +++ b/src/main/java/software/coley/llzip/util/BufferData.java @@ -5,6 +5,7 @@ import java.nio.Buffer; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.util.concurrent.atomic.AtomicBoolean; /** * File that is backed by a byte buffer. @@ -13,10 +14,11 @@ */ public final class BufferData implements ByteData { private final ByteBuffer buffer; - private volatile boolean cleaned; + private final AtomicBoolean cleaned; - private BufferData(ByteBuffer buffer) { + private BufferData(ByteBuffer buffer, AtomicBoolean cleaned) { this.buffer = buffer; + this.cleaned = cleaned; } @Override @@ -43,8 +45,7 @@ public void get(long position, byte[] b, int off, int len) { ensureOpen(); // Left intentionally as unchained calls due to API differences across Java versions // and how the compiler changes output. - ByteBuffer buffer = this.buffer; - buffer.slice(); + ByteBuffer buffer = this.buffer.slice(); buffer.order(buffer.order()); ((Buffer) buffer).position(validate(position)); buffer.get(b, off, len); @@ -73,7 +74,7 @@ public void transferTo(OutputStream out, byte[] buf) throws IOException { @Override public ByteData slice(long startIndex, long endIndex) { ensureOpen(); - return new BufferData(ByteDataUtil.sliceExact(buffer, validate(startIndex), validate(endIndex))); + return new BufferData(ByteDataUtil.sliceExact(buffer, validate(startIndex), validate(endIndex)), cleaned); } @Override @@ -97,11 +98,11 @@ public int hashCode() { @Override public void close() { - if (!cleaned) { + if (!cleaned.get()) { synchronized (this) { - if (cleaned) + if (cleaned.get()) return; - cleaned = true; + cleaned.set(true); ByteBuffer buffer = this.buffer; if (buffer.isDirect()) { CleanerUtil.invokeCleaner(buffer); @@ -110,17 +111,8 @@ public void close() { } } - @Override - protected void finalize() throws Throwable { - try { - close(); - } finally { - super.finalize(); - } - } - private void ensureOpen() { - if (cleaned) + if (cleaned.get()) throw new IllegalStateException("Cannot access data after close"); } @@ -138,7 +130,7 @@ private static int validate(long v) { * @return Buffer data. */ public static BufferData wrap(ByteBuffer buffer) { - return new BufferData(buffer); + return new BufferData(buffer, new AtomicBoolean()); } /** @@ -148,6 +140,6 @@ public static BufferData wrap(ByteBuffer buffer) { * @return Buffer data. */ public static BufferData wrap(byte[] array) { - return new BufferData(ByteBuffer.wrap(array).order(ByteOrder.LITTLE_ENDIAN)); + return new BufferData(ByteBuffer.wrap(array).order(ByteOrder.LITTLE_ENDIAN), new AtomicBoolean()); } } diff --git a/src/main/java/software/coley/llzip/util/FileMapUtil.java b/src/main/java/software/coley/llzip/util/FileMapUtil.java index 8bc5cb2..f9139a5 100644 --- a/src/main/java/software/coley/llzip/util/FileMapUtil.java +++ b/src/main/java/software/coley/llzip/util/FileMapUtil.java @@ -9,6 +9,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; +import java.util.concurrent.atomic.AtomicBoolean; /** * Maps large files into memory. @@ -65,7 +66,7 @@ public static ByteData map(Path path) throws IOException { } catch (IllegalAccessException | InvocationTargetException ex) { throw new InternalError(ex); } - }); + }, new AtomicBoolean()); return mappedFile; } } diff --git a/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java b/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java index 400ceba..07038e8 100644 --- a/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java +++ b/src/main/java/software/coley/llzip/util/UnsafeMappedFile.java @@ -5,6 +5,7 @@ import java.io.IOException; import java.io.OutputStream; import java.nio.ByteOrder; +import java.util.concurrent.atomic.AtomicBoolean; /** * Mapped file backed by address & length. @@ -15,24 +16,26 @@ final class UnsafeMappedFile implements ByteData { private static final boolean SWAP = ByteOrder.nativeOrder() == ByteOrder.BIG_ENDIAN; private static final Unsafe UNSAFE = UnsafeUtil.get(); - private volatile boolean cleaned; + private final AtomicBoolean cleaned; private final long address; private final long end; private final Runnable deallocator; @SuppressWarnings("unused") private final Object attachment; - private UnsafeMappedFile(Object attachment, long address, long end) { + private UnsafeMappedFile(Object attachment, long address, long end, AtomicBoolean cleaned) { this.attachment = attachment; this.address = address; this.end = end; + this.cleaned = cleaned; deallocator = null; } - UnsafeMappedFile(long address, long length, Runnable deallocator) { + UnsafeMappedFile(long address, long length, Runnable deallocator, AtomicBoolean cleaned) { this.address = address; this.end = address + length; this.deallocator = deallocator; + this.cleaned = cleaned; attachment = null; } @@ -83,7 +86,7 @@ public ByteData slice(long startIndex, long endIndex) { ensureOpen(); if (startIndex > endIndex) throw new IllegalArgumentException(); - return new UnsafeMappedFile(this, validate(startIndex), validate(endIndex)); + return new UnsafeMappedFile(this, validate(startIndex), validate(endIndex), cleaned); } @Override @@ -113,11 +116,11 @@ public int hashCode() { @Override public void close() { - if (!cleaned) { + if (!cleaned.get()) { synchronized (this) { - if (cleaned) + if (cleaned.get()) return; - cleaned = true; + cleaned.set(true); Runnable deallocator = this.deallocator; if (deallocator != null) deallocator.run(); @@ -126,20 +129,10 @@ public void close() { } private void ensureOpen() { - if (cleaned) + if (cleaned.get()) throw new IllegalStateException("Cannot access data after close"); } - @SuppressWarnings("deprecation") - @Override - protected void finalize() throws Throwable { - try { - close(); - } finally { - super.finalize(); - } - } - private long validate(long position) { if (position < 0L) { throw new IllegalArgumentException();