Share 'cleaned' reference across slices, do not call close() in finalization

Author: Col-E

pom.xml

@@ -6,7 +6,7 @@
 
     <groupId>software.coley</groupId>
     <artifactId>lljzip</artifactId>
-    <version>1.3.2</version>
+    <version>1.3.3</version>
 
     <name>LL Java ZIP</name>
     <description>Lower level ZIP support for Java</description>

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());
 	}
 }

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

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