From 171a7f34eacbb6faad6b1851730cfb41249e7606 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Mon, 11 Dec 2017 14:54:22 -0800 Subject: [PATCH 1/4] Replace Disposable with AutoCloseable. Update documentation of close (formerly dispose). Some other small cleanup items. --- .../main/com/apple/foundationdb/Cluster.java | 12 +++---- .../main/com/apple/foundationdb/Database.java | 18 +++++++--- .../com/apple/foundationdb/Disposable.java | 35 ------------------ .../main/com/apple/foundationdb/FDB.java | 10 +++--- .../com/apple/foundationdb/FDBDatabase.java | 14 ++++---- .../apple/foundationdb/FDBTransaction.java | 16 ++++----- .../com/apple/foundationdb/FutureResults.java | 2 +- .../com/apple/foundationdb/LocalityUtil.java | 35 ++++++++---------- .../com/apple/foundationdb/NativeFuture.java | 16 +++++---- ...ableImpl.java => NativeObjectWrapper.java} | 36 +++++++++---------- .../com/apple/foundationdb/RangeQuery.java | 4 +-- .../com/apple/foundationdb/Transaction.java | 13 +++++-- .../apple/foundationdb/async/AsyncUtil.java | 26 +++++++------- ...rator.java => CloseableAsyncIterator.java} | 27 ++++++++++---- .../foundationdb/test/AsyncStackTester.java | 4 +-- .../com/apple/foundationdb/test/Context.java | 11 +++--- .../foundationdb/test/LocalityTests.java | 6 ++-- .../apple/foundationdb/test/RangeTest.java | 8 ++--- .../apple/foundationdb/test/StackTester.java | 11 +++--- 19 files changed, 149 insertions(+), 155 deletions(-) delete mode 100644 bindings/java/src-completable/main/com/apple/foundationdb/Disposable.java rename bindings/java/src-completable/main/com/apple/foundationdb/{DefaultDisposableImpl.java => NativeObjectWrapper.java} (73%) rename bindings/java/src-completable/main/com/apple/foundationdb/async/{DisposableAsyncIterator.java => CloseableAsyncIterator.java} (54%) diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/Cluster.java b/bindings/java/src-completable/main/com/apple/foundationdb/Cluster.java index ca97b895c2..cf3664a7bd 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/Cluster.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/Cluster.java @@ -27,10 +27,10 @@ import java.util.concurrent.Executor; * The {@code Cluster} represents a connection to a physical set of cooperating machines * running FoundationDB. A {@code Cluster} is opened with a reference to a cluster file.
*
- * Note: {@code Cluster} objects must be disposed when no longer in use in order - * to free associated native memory. + * Note: {@code Cluster} objects must be {@link #close closed} when no longer in use + * in order to free any associated resources. */ -public class Cluster extends DefaultDisposableImpl implements Disposable { +public class Cluster extends NativeObjectWrapper { private ClusterOptions options; private final Executor executor; @@ -60,8 +60,8 @@ public class Cluster extends DefaultDisposableImpl implements Disposable { @Override protected void finalize() throws Throwable { try { - checkUndisposed("Cluster"); - dispose(); + checkUnclosed("Cluster"); + close(); } finally { super.finalize(); @@ -96,7 +96,7 @@ public class Cluster extends DefaultDisposableImpl implements Disposable { } @Override - protected void disposeInternal(long cPtr) { + protected void closeInternal(long cPtr) { Cluster_dispose(cPtr); } diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/Database.java b/bindings/java/src-completable/main/com/apple/foundationdb/Database.java index 3fe7f16803..81127463f1 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/Database.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/Database.java @@ -37,10 +37,10 @@ import java.util.function.Function; * executed. These methods will not return successfully until {@code commit()} has * returned successfully.
*
- * Note: {@code Database} objects must be disposed when no longer in use in order - * to free associated native memory. + * Note: {@code Database} objects must be {@link #close closed} when no longer + * in use in order to free any associated resources. */ -public interface Database extends Disposable, TransactionContext { +public interface Database extends AutoCloseable, TransactionContext { /** * Creates a {@link Transaction} that operates on this {@code Database}.
*
@@ -94,7 +94,7 @@ public interface Database extends Disposable, TransactionContext { * @param retryable the block of logic to execute in a {@link Transaction} against * this database * @param e the {@link Executor} to use for asynchronous callbacks - * + * * @see #read(Function) */ T read(Function retryable, Executor e); @@ -128,7 +128,7 @@ public interface Database extends Disposable, TransactionContext { * @param retryable the block of logic to execute in a {@link ReadTransaction} against * this database * @param e the {@link Executor} to use for asynchronous callbacks - * + * * @see #readAsync(Function) */ CompletableFuture readAsync( @@ -211,4 +211,12 @@ public interface Database extends Disposable, TransactionContext { */ CompletableFuture runAsync( Function> retryable, Executor e); + + /** + * Close the {@code Database} object and release any associated resources. This must be called at + * least once after the {@code Database} object is no longer in use. This can be called multiple + * times, but care should be taken that it is not in use in another thread at the time of the call. + */ + @Override + void close(); } diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/Disposable.java b/bindings/java/src-completable/main/com/apple/foundationdb/Disposable.java deleted file mode 100644 index 3af021a0c7..0000000000 --- a/bindings/java/src-completable/main/com/apple/foundationdb/Disposable.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Disposable.java - * - * This source file is part of the FoundationDB open source project - * - * Copyright 2013-2018 Apple Inc. and the FoundationDB project authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.apple.foundationdb; - -/** - * An object with native FoundationDB resources that must be freed. Failure to dispose of - * {@code Disposable} objects will result in memory leaks. - */ -public interface Disposable { - /** - * Dispose of the object. This must be called once the object is no longer in use to - * free any native resources associated with the object. This can be called multiple times, - * but care should be taken that an object is not in use in another thread at the time of - * the call. - */ - void dispose(); -} \ No newline at end of file diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/FDB.java b/bindings/java/src-completable/main/com/apple/foundationdb/FDB.java index 42ac857eab..1226764e6d 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/FDB.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/FDB.java @@ -81,7 +81,7 @@ public class FDB { final int apiVersion; private volatile boolean netStarted = false; private volatile boolean netStopped = false; - volatile boolean warnOnUndisposed = true; + volatile boolean warnOnUnclosed = true; final private Semaphore netRunning = new Semaphore(1); private final NetworkOptions options; @@ -165,14 +165,14 @@ public class FDB { FDB fdb = new FDB(version); if(version < 510) { - fdb.warnOnUndisposed = false; + fdb.warnOnUnclosed = false; } return singleton = fdb; } - public void setUndisposedWarning(boolean warnOnUndisposed) { - this.warnOnUndisposed = warnOnUndisposed; + public void setUnclosedWarning(boolean warnOnUnclosed) { + this.warnOnUnclosed = warnOnUnclosed; } // Singleton is initialized to null and only set once by a call to selectAPIVersion @@ -299,7 +299,7 @@ public class FDB { } Cluster c = f.join(); Database db = c.openDatabase(e); - c.dispose(); + c.close(); return db; } diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/FDBDatabase.java b/bindings/java/src-completable/main/com/apple/foundationdb/FDBDatabase.java index a1b65c51d3..5ed51efd4a 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/FDBDatabase.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/FDBDatabase.java @@ -28,7 +28,7 @@ import java.util.function.Function; import com.apple.foundationdb.async.AsyncUtil; -class FDBDatabase extends DefaultDisposableImpl implements Database, Disposable, OptionConsumer { +class FDBDatabase extends NativeObjectWrapper implements Database, OptionConsumer { private DatabaseOptions options; private final Executor executor; @@ -57,7 +57,7 @@ class FDBDatabase extends DefaultDisposableImpl implements Database, Disposable, } } } finally { - t.dispose(); + t.close(); } } @@ -90,7 +90,7 @@ class FDBDatabase extends DefaultDisposableImpl implements Database, Disposable, }, e).thenCompose(x -> x); }, e) .thenApply(o -> returnValue.get()) - .whenComplete((v, t) -> trRef.get().dispose()); + .whenComplete((v, t) -> trRef.get().close()); } @Override @@ -102,8 +102,8 @@ class FDBDatabase extends DefaultDisposableImpl implements Database, Disposable, @Override protected void finalize() throws Throwable { try { - checkUndisposed("Database"); - dispose(); + checkUnclosed("Database"); + close(); } finally { super.finalize(); @@ -120,7 +120,7 @@ class FDBDatabase extends DefaultDisposableImpl implements Database, Disposable, return tr; } catch(RuntimeException err) { if(tr != null) { - tr.dispose(); + tr.close(); } throw err; @@ -145,7 +145,7 @@ class FDBDatabase extends DefaultDisposableImpl implements Database, Disposable, } @Override - protected void disposeInternal(long cPtr) { + protected void closeInternal(long cPtr) { Database_dispose(cPtr); } diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/FDBTransaction.java b/bindings/java/src-completable/main/com/apple/foundationdb/FDBTransaction.java index 495915ffd9..02c3f03f4e 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/FDBTransaction.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/FDBTransaction.java @@ -29,7 +29,7 @@ import java.util.function.Function; import com.apple.foundationdb.async.*; import com.apple.foundationdb.tuple.ByteArrayUtil; -class FDBTransaction extends DefaultDisposableImpl implements Disposable, Transaction, OptionConsumer { +class FDBTransaction extends NativeObjectWrapper implements Transaction, OptionConsumer { private final Database database; private final Executor executor; private final TransactionOptions options; @@ -300,7 +300,7 @@ class FDBTransaction extends DefaultDisposableImpl implements Disposable, Transa return database; } - // Users of this function must dispose of the returned FutureResults when finished + // Users of this function must close the returned FutureResults when finished protected FutureResults getRange_internal( KeySelector begin, KeySelector end, int rowLimit, int targetBytes, int streamingMode, @@ -496,13 +496,13 @@ class FDBTransaction extends DefaultDisposableImpl implements Disposable, Transa return f.thenApply(v -> tr) .whenComplete((v, t) -> { if(t != null) { - tr.dispose(); + tr.close(); } }); } finally { pointerReadLock.unlock(); if(!transactionOwner) { - dispose(); + close(); } } } @@ -537,7 +537,7 @@ class FDBTransaction extends DefaultDisposableImpl implements Disposable, Transa } catch(RuntimeException err) { if(tr != null) { - tr.dispose(); + tr.close(); } throw err; @@ -557,8 +557,8 @@ class FDBTransaction extends DefaultDisposableImpl implements Disposable, Transa @Override protected void finalize() throws Throwable { try { - checkUndisposed("Transaction"); - dispose(); + checkUnclosed("Transaction"); + close(); } finally { super.finalize(); @@ -566,7 +566,7 @@ class FDBTransaction extends DefaultDisposableImpl implements Disposable, Transa } @Override - protected void disposeInternal(long cPtr) { + protected void closeInternal(long cPtr) { if(transactionOwner) { Transaction_dispose(cPtr); } diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/FutureResults.java b/bindings/java/src-completable/main/com/apple/foundationdb/FutureResults.java index 2ebff91dbc..d1a9660bef 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/FutureResults.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/FutureResults.java @@ -30,7 +30,7 @@ class FutureResults extends NativeFuture { @Override protected void postMarshal() { - // We can't dispose because this class actually marshals on-demand + // We can't close because this class actually marshals on-demand } @Override diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/LocalityUtil.java b/bindings/java/src-completable/main/com/apple/foundationdb/LocalityUtil.java index 6e2784eb32..9a3cd7fbfb 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/LocalityUtil.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/LocalityUtil.java @@ -28,7 +28,7 @@ import java.util.function.BiFunction; import com.apple.foundationdb.async.AsyncIterable; import com.apple.foundationdb.async.AsyncIterator; -import com.apple.foundationdb.async.DisposableAsyncIterator; +import com.apple.foundationdb.async.CloseableAsyncIterator; import com.apple.foundationdb.tuple.ByteArrayUtil; /** @@ -40,7 +40,7 @@ import com.apple.foundationdb.tuple.ByteArrayUtil; */ public class LocalityUtil { /** - * Returns a {@code DisposableAsyncIterator} of keys {@code k} such that + * Returns a {@code CloseableAsyncIterator} of keys {@code k} such that * {@code begin <= k < end} and {@code k} is located at the start of a * contiguous range stored on a single server.
*
@@ -53,12 +53,12 @@ public class LocalityUtil { * * @return an sequence of keys denoting the start of single-server ranges */ - public static DisposableAsyncIterator getBoundaryKeys(Database db, byte[] begin, byte[] end) { + public static CloseableAsyncIterator getBoundaryKeys(Database db, byte[] begin, byte[] end) { return getBoundaryKeys_internal(db.createTransaction(), begin, end); } /** - * Returns a {@code DisposableAsyncIterator} of keys {@code k} such that + * Returns a {@code CloseableAsyncIterator} of keys {@code k} such that * {@code begin <= k < end} and {@code k} is located at the start of a * contiguous range stored on a single server.
*
@@ -79,7 +79,7 @@ public class LocalityUtil { * * @return an sequence of keys denoting the start of single-server ranges */ - public static DisposableAsyncIterator getBoundaryKeys(Transaction tr, byte[] begin, byte[] end) { + public static CloseableAsyncIterator getBoundaryKeys(Transaction tr, byte[] begin, byte[] end) { Transaction local = tr.getDatabase().createTransaction(); CompletableFuture readVersion = tr.getReadVersion(); if(readVersion.isDone() && !readVersion.isCompletedExceptionally()) { @@ -110,11 +110,11 @@ public class LocalityUtil { return ((FDBTransaction)tr).getAddressesForKey(key); } - private static DisposableAsyncIterator getBoundaryKeys_internal(Transaction tr, byte[] begin, byte[] end) { + private static CloseableAsyncIterator getBoundaryKeys_internal(Transaction tr, byte[] begin, byte[] end) { return new BoundaryIterator(tr, begin, end); } - static class BoundaryIterator implements DisposableAsyncIterator { + static class BoundaryIterator implements CloseableAsyncIterator { Transaction tr; byte[] begin; byte[] lastBegin; @@ -123,7 +123,7 @@ public class LocalityUtil { AsyncIterator block; private CompletableFuture nextFuture; - private boolean disposed; + private boolean closed; BoundaryIterator(Transaction tr, byte[] begin, byte[] end) { this.tr = tr; @@ -139,7 +139,7 @@ public class LocalityUtil { block = firstGet.iterator(); nextFuture = block.onHasNext().handleAsync(handler, tr.getExecutor()).thenCompose(x -> x); - disposed = false; + closed = false; } @Override @@ -174,7 +174,7 @@ public class LocalityUtil { if(o instanceof FDBException) { FDBException err = (FDBException) o; if(err.getCode() == 1007 && !Arrays.equals(begin, lastBegin)) { - BoundaryIterator.this.tr.dispose(); + BoundaryIterator.this.tr.close(); BoundaryIterator.this.tr = BoundaryIterator.this.tr.getDatabase().createTransaction(); return restartGet(); } @@ -210,21 +210,16 @@ public class LocalityUtil { } @Override - public void cancel() { - BoundaryIterator.this.tr.dispose(); - disposed = true; - } - - @Override - public void dispose() { - cancel(); + public void close() { + BoundaryIterator.this.tr.close(); + closed = true; } @Override protected void finalize() throws Throwable { try { - if(FDB.getInstance().warnOnUndisposed && !disposed) { - System.err.println("DisposableAsyncIterator not disposed (getBoundaryKeys)"); + if(FDB.getInstance().warnOnUnclosed && !closed) { + System.err.println("CloseableAsyncIterator not closed (getBoundaryKeys)"); } } finally { diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/NativeFuture.java b/bindings/java/src-completable/main/com/apple/foundationdb/NativeFuture.java index 3a340eb04d..ecb4704333 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/NativeFuture.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/NativeFuture.java @@ -25,7 +25,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantReadWriteLock; -abstract class NativeFuture extends CompletableFuture implements Disposable { +abstract class NativeFuture extends CompletableFuture implements AutoCloseable { private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock(); protected final Lock pointerReadLock = rwl.readLock(); @@ -42,7 +42,7 @@ abstract class NativeFuture extends CompletableFuture implements Disposabl // lead to a race where the marshalWhenDone tries to run on an // unconstructed subclass. // - // Since this must be called from a constructor, we assume that dispose + // Since this must be called from a constructor, we assume that close // cannot be called concurrently. protected void registerMarshalCallback(Executor executor) { if(cPtr != 0) { @@ -81,13 +81,13 @@ abstract class NativeFuture extends CompletableFuture implements Disposabl } protected void postMarshal() { - dispose(); + close(); } abstract protected T getIfDone_internal(long cPtr) throws FDBException; @Override - public void dispose() { + public void close() { long ptr = 0; rwl.writeLock().lock(); @@ -100,7 +100,7 @@ abstract class NativeFuture extends CompletableFuture implements Disposabl if(ptr != 0) { Future_dispose(ptr); if(!isDone()) { - completeExceptionally(new IllegalStateException("Future has been disposed")); + completeExceptionally(new IllegalStateException("Future has been closed")); } } } @@ -127,11 +127,15 @@ abstract class NativeFuture extends CompletableFuture implements Disposabl assert( rwl.getReadHoldCount() > 0 ); if(cPtr == 0) - throw new IllegalStateException("Cannot access disposed object"); + throw new IllegalStateException("Cannot access closed object"); return cPtr; } + boolean isCleanedUp() { + return isDone() && cPtr == 0; + } + private native void Future_registerCallback(long cPtr, Runnable callback); private native void Future_blockUntilReady(long cPtr); private native boolean Future_isReady(long cPtr); diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/DefaultDisposableImpl.java b/bindings/java/src-completable/main/com/apple/foundationdb/NativeObjectWrapper.java similarity index 73% rename from bindings/java/src-completable/main/com/apple/foundationdb/DefaultDisposableImpl.java rename to bindings/java/src-completable/main/com/apple/foundationdb/NativeObjectWrapper.java index 5fed513c98..0d13052535 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/DefaultDisposableImpl.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/NativeObjectWrapper.java @@ -1,5 +1,5 @@ /* - * DefaultDisposableImpl.java + * NativeObjectWrapper.java * * This source file is part of the FoundationDB open source project * @@ -23,56 +23,56 @@ package com.apple.foundationdb; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantReadWriteLock; -abstract class DefaultDisposableImpl implements Disposable { +abstract class NativeObjectWrapper implements AutoCloseable { private final ReentrantReadWriteLock rwl = new ReentrantReadWriteLock(); protected final Lock pointerReadLock = rwl.readLock(); - private boolean disposed = false; + private boolean closed = false; private long cPtr; - public DefaultDisposableImpl() { + public NativeObjectWrapper() { } - public DefaultDisposableImpl(long cPtr) { + public NativeObjectWrapper(long cPtr) { this.cPtr = cPtr; if(this.cPtr == 0) - this.disposed = true; + this.closed = true; } - public boolean isDisposed() { + public boolean isClosed() { // we must have a read lock for this function to make sense, however it // does not make sense to take the lock here, since the code that uses // the result must inherently have the read lock itself. assert( rwl.getReadHoldCount() > 0 ); - return disposed; + return closed; } - public void checkUndisposed(String context) { + public void checkUnclosed(String context) { try { - if(FDB.getInstance().warnOnUndisposed && !disposed) { - System.err.println(context + " not disposed"); + if(FDB.getInstance().warnOnUnclosed && !closed) { + System.err.println(context + " not closed"); } } catch(Exception e) {} } @Override - public void dispose() { + public void close() { rwl.writeLock().lock(); long ptr = 0; try { - if(disposed) + if(closed) return; ptr = cPtr; this.cPtr = 0; - disposed = true; + closed = true; } finally { rwl.writeLock().unlock(); } - disposeInternal(ptr); + closeInternal(ptr); } protected long getPtr() { @@ -81,11 +81,11 @@ abstract class DefaultDisposableImpl implements Disposable { // the result must inherently have the read lock itself. assert( rwl.getReadHoldCount() > 0 ); - if(this.disposed) - throw new IllegalStateException("Cannot access disposed object"); + if(this.closed) + throw new IllegalStateException("Cannot access closed object"); return this.cPtr; } - protected abstract void disposeInternal(long cPtr); + protected abstract void closeInternal(long cPtr); } diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/RangeQuery.java b/bindings/java/src-completable/main/com/apple/foundationdb/RangeQuery.java index addf385afe..ad78ca8320 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/RangeQuery.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/RangeQuery.java @@ -85,7 +85,7 @@ class RangeQuery implements AsyncIterable, Iterable { this.begin, this.end, this.rowLimit, 0, StreamingMode.EXACT.code(), 1, this.snapshot, this.reverse); return range.thenApply(result -> result.get().values) - .whenComplete((result, e) -> range.dispose()); + .whenComplete((result, e) -> range.close()); } // If the streaming mode is not EXACT, simply collect the results of an iteration into a list @@ -197,7 +197,7 @@ class RangeQuery implements AsyncIterable, Iterable { promise.complete(Boolean.TRUE); } finally { - fetchingChunk.dispose(); + fetchingChunk.close(); } } } diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/Transaction.java b/bindings/java/src-completable/main/com/apple/foundationdb/Transaction.java index de41c3aad4..b2590cff74 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/Transaction.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/Transaction.java @@ -70,10 +70,10 @@ import com.apple.foundationdb.tuple.Tuple; * option. This is because the Java bindings disallow use of {@code Transaction} objects after {@link #onError} * is called.
*
- * Note: {@code Transaction} objects must be disposed when no longer in use in order - * to free associated native memory. + * Note: {@code Transaction} objects must be {@link #close closed} when no longer + * in use in order to free any associated resources. */ -public interface Transaction extends Disposable, ReadTransaction, TransactionContext { +public interface Transaction extends AutoCloseable, ReadTransaction, TransactionContext { /** * Return special-purpose, read-only view of the database. Reads done through this interface are known as "snapshot reads". @@ -369,4 +369,11 @@ public interface Transaction extends Disposable, ReadTransaction, TransactionCon CompletableFuture runAsync( Function> retryable); + /** + * Close the {@code Transaction object and release any associated resources. This must be called at + * least once after the {@code Transaction} object is no longer in use. This can be called multiple + * times, but care should be taken that it is not in use in another thread at the time of the call. + */ + @Override + void close(); } diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/async/AsyncUtil.java b/bindings/java/src-completable/main/com/apple/foundationdb/async/AsyncUtil.java index 27da073c66..fb082fe44e 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/async/AsyncUtil.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/async/AsyncUtil.java @@ -185,16 +185,16 @@ public class AsyncUtil { } /** - * Map a {@code DisposableAsyncIterator} into a {@code DisposableAsyncIterator} of another type or with + * Map a {@code CloseableAsyncIterator} into a {@code CloseableAsyncIterator} of another type or with * each element modified in some fashion. * * @param iterator input * @param func mapping function applied to each element * @return a new iterator with each element mapped to a different value */ - public static DisposableAsyncIterator mapIterator(final DisposableAsyncIterator iterator, + public static CloseableAsyncIterator mapIterator(final CloseableAsyncIterator iterator, final Function func) { - return new DisposableAsyncIterator() { + return new CloseableAsyncIterator() { @Override public void remove() { iterator.remove(); @@ -221,8 +221,8 @@ public class AsyncUtil { } @Override - public void dispose() { - iterator.dispose(); + public void close() { + iterator.close(); } }; } @@ -283,7 +283,7 @@ public class AsyncUtil { * * @param body the asynchronous operation over which to loop * - * @return a {@code PartialFuture} which will be set at completion of the loop. + * @return a {@link CompletableFuture} which will be set at completion of the loop. * @deprecated Since version 5.1.0. Use the version of {@link #whileTrue(Supplier) whileTrue} that takes a * {@link Supplier} instead. */ @@ -298,7 +298,7 @@ public class AsyncUtil { * @param body the asynchronous operation over which to loop * @param executor the {@link Executor} to use for asynchronous operations * - * @return a {@code PartialFuture} which will be set at completion of the loop. + * @return a {@link CompletableFuture} which will be set at completion of the loop. * @deprecated Since version 5.1.0. Use the version of {@link #whileTrue(Supplier, Executor) whileTrue} that takes a * {@link Supplier} instead. */ @@ -312,7 +312,7 @@ public class AsyncUtil { * * @param body the asynchronous operation over which to loop * - * @return a {@code PartialFuture} which will be set at completion of the loop. + * @return a {@link CompletableFuture} which will be set at completion of the loop. */ public static CompletableFuture whileTrue(Supplier> body) { return whileTrue(body, DEFAULT_EXECUTOR); @@ -324,7 +324,7 @@ public class AsyncUtil { * @param body the asynchronous operation over which to loop * @param executor the {@link Executor} to use for asynchronous operations * - * @return a {@code PartialFuture} which will be set at completion of the loop. + * @return a {@link CompletableFuture} which will be set at completion of the loop. */ public static CompletableFuture whileTrue(Supplier> body, Executor executor) { return new LoopPartial(body, executor).run(); @@ -402,10 +402,10 @@ public class AsyncUtil { } /** - * Return a {@code CompletableFuture} that will be set when any of the {@code PartialFuture} + * Return a {@code CompletableFuture} that will be set when any of the {@link CompletableFuture} * inputs are done. A {@code CompletableFuture} is done both on success and failure. * - * @param input the list of {@code PartialFuture}s to monitor. This list + * @param input the list of {@link CompletableFuture}s to monitor. This list * must not be modified during the execution of this call. * * @return a signal that will be set when any of the {@code CompletableFuture}s are done @@ -418,10 +418,10 @@ public class AsyncUtil { } /** - * Return a {@code CompletableFuture} that will be set when all the {@code PartialFuture} + * Return a {@code CompletableFuture} that will be set when all the {@link CompletableFuture} * inputs are done. A {@code CompletableFuture} is done both on success and failure. * - * @param input the list of {@code PartialFuture}s to monitor. This list + * @param input the list of {@link CompletableFuture}s to monitor. This list * must not be modified during the execution of this call. * * @return a signal that will be set when all of the {@code CompletableFuture}s are done diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/async/DisposableAsyncIterator.java b/bindings/java/src-completable/main/com/apple/foundationdb/async/CloseableAsyncIterator.java similarity index 54% rename from bindings/java/src-completable/main/com/apple/foundationdb/async/DisposableAsyncIterator.java rename to bindings/java/src-completable/main/com/apple/foundationdb/async/CloseableAsyncIterator.java index 79920722e1..cd9eaa4898 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/async/DisposableAsyncIterator.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/async/CloseableAsyncIterator.java @@ -1,5 +1,5 @@ /* - * DisposableAsyncIterator.java + * CloseableAsyncIterator.java * * This source file is part of the FoundationDB open source project * @@ -20,12 +20,27 @@ package com.apple.foundationdb.async; -import com.apple.foundationdb.Disposable; - /** - * A version of {@link AsyncIterator} that holds native FDB resources and - * must be disposed once it is no longer in use. + * A version of {@link AsyncIterator} that must be closed once no longer in use in order to free + * any associated resources. * * @param the type of object yielded by {@code next()} */ -public interface DisposableAsyncIterator extends Disposable, AsyncIterator {} +public interface CloseableAsyncIterator extends AutoCloseable, AsyncIterator { + /** + * Cancels any outstanding asynchronous work, closes the iterator, and frees any associated + * resources. This must be called at least once after the object is no longer in use. This + * can be called multiple times, but care should be taken that an object is not in use + * in another thread at the time of the call. + */ + @Override + void close(); + + /** + * Alias for {@link #close} + */ + @Override + default void cancel() { + close(); + } +} diff --git a/bindings/java/src-completable/test/com/apple/foundationdb/test/AsyncStackTester.java b/bindings/java/src-completable/test/com/apple/foundationdb/test/AsyncStackTester.java index f2157f57b1..6de31ced6e 100644 --- a/bindings/java/src-completable/test/com/apple/foundationdb/test/AsyncStackTester.java +++ b/bindings/java/src-completable/test/com/apple/foundationdb/test/AsyncStackTester.java @@ -792,7 +792,7 @@ public class AsyncStackTester { Transaction tr = db.createTransaction(); return tr.getRange(nextKey, endKey, 1000).asList() - .whenComplete((x, t) -> tr.dispose()) + .whenComplete((x, t) -> tr.close()) .thenComposeAsync(new Function, CompletableFuture>() { @Override public CompletableFuture apply(List next) { @@ -886,7 +886,7 @@ public class AsyncStackTester { byte[] bs = db.createTransaction().get(key).get(); System.out.println("output of " + ByteArrayUtil.printable(key) + " as: " + ByteArrayUtil.printable(bs));*/ - db.dispose(); + db.close(); System.gc(); /*fdb.stopNetwork(); diff --git a/bindings/java/src-completable/test/com/apple/foundationdb/test/Context.java b/bindings/java/src-completable/test/com/apple/foundationdb/test/Context.java index 29d316b47a..fb0f80f187 100644 --- a/bindings/java/src-completable/test/com/apple/foundationdb/test/Context.java +++ b/bindings/java/src-completable/test/com/apple/foundationdb/test/Context.java @@ -37,7 +37,7 @@ import com.apple.foundationdb.Transaction; import com.apple.foundationdb.tuple.ByteArrayUtil; import com.apple.foundationdb.tuple.Tuple; -abstract class Context implements Runnable { +abstract class Context implements Runnable, AutoCloseable { final Stack stack = new Stack(); final Database db; final String preStr; @@ -104,7 +104,7 @@ abstract class Context implements Runnable { if(count.decrementAndGet() == 0) { assert !transactionMap.containsValue(tr); transactionRefCounts.remove(tr); - tr.dispose(); + tr.close(); } } } @@ -140,7 +140,7 @@ abstract class Context implements Runnable { public void newTransaction(Transaction oldTr) { Transaction newTr = db.createTransaction(); if(!updateCurrentTransaction(oldTr, newTr)) { - newTr.dispose(); + newTr.close(); } } @@ -212,9 +212,10 @@ abstract class Context implements Runnable { return done.thenApplyAsync((x) -> params); } - void dispose() { + @Override + public void close() { for(Transaction tr : transactionMap.values()) { - tr.dispose(); + tr.close(); } } } diff --git a/bindings/java/src-completable/test/com/apple/foundationdb/test/LocalityTests.java b/bindings/java/src-completable/test/com/apple/foundationdb/test/LocalityTests.java index aec323ed02..5d3e049e36 100644 --- a/bindings/java/src-completable/test/com/apple/foundationdb/test/LocalityTests.java +++ b/bindings/java/src-completable/test/com/apple/foundationdb/test/LocalityTests.java @@ -27,7 +27,7 @@ import com.apple.foundationdb.Database; import com.apple.foundationdb.FDB; import com.apple.foundationdb.LocalityUtil; import com.apple.foundationdb.Transaction; -import com.apple.foundationdb.async.DisposableAsyncIterator; +import com.apple.foundationdb.async.CloseableAsyncIterator; import com.apple.foundationdb.async.AsyncUtil; import com.apple.foundationdb.tuple.ByteArrayUtil; @@ -46,13 +46,13 @@ public class LocalityTests { long start = System.currentTimeMillis(); - DisposableAsyncIterator keys = LocalityUtil.getBoundaryKeys(database, new byte[0], new byte[] { (byte)255 } ); + CloseableAsyncIterator keys = LocalityUtil.getBoundaryKeys(database, new byte[0], new byte[] { (byte)255 } ); CompletableFuture> collection = AsyncUtil.collect(keys); List list = collection.join(); System.out.println("Took " + (System.currentTimeMillis() - start) + "ms to get " + list.size() + " items"); - keys.dispose(); + keys.close(); int i = 0; for(byte[] key : collection.join()) { diff --git a/bindings/java/src-completable/test/com/apple/foundationdb/test/RangeTest.java b/bindings/java/src-completable/test/com/apple/foundationdb/test/RangeTest.java index 6a8e2f4ce5..9426c6cd5d 100644 --- a/bindings/java/src-completable/test/com/apple/foundationdb/test/RangeTest.java +++ b/bindings/java/src-completable/test/com/apple/foundationdb/test/RangeTest.java @@ -114,8 +114,8 @@ public class RangeTest { e.printStackTrace(); return; } - //db.dispose(); - //cluster.dispose(); + //db.close(); + //cluster.close(); tr = db.createTransaction(); checkRange(tr); @@ -154,8 +154,8 @@ public class RangeTest { System.out.println("range comparisons okay"); } - db.dispose(); - //cluster.dispose(); + db.close(); + //cluster.close(); //fdb.stopNetwork(); System.out.println("Done with test program"); } diff --git a/bindings/java/src-completable/test/com/apple/foundationdb/test/StackTester.java b/bindings/java/src-completable/test/com/apple/foundationdb/test/StackTester.java index 6a237c6506..8fabc80aa8 100644 --- a/bindings/java/src-completable/test/com/apple/foundationdb/test/StackTester.java +++ b/bindings/java/src-completable/test/com/apple/foundationdb/test/StackTester.java @@ -37,12 +37,11 @@ import com.apple.foundationdb.KeyValue; import com.apple.foundationdb.LocalityUtil; import com.apple.foundationdb.MutationType; import com.apple.foundationdb.Range; -import com.apple.foundationdb.ReadTransaction; import com.apple.foundationdb.StreamingMode; import com.apple.foundationdb.Transaction; import com.apple.foundationdb.async.AsyncIterable; +import com.apple.foundationdb.async.CloseableAsyncIterator; import com.apple.foundationdb.tuple.ByteArrayUtil; -import com.apple.foundationdb.async.DisposableAsyncIterator; import com.apple.foundationdb.tuple.Tuple; import com.apple.foundationdb.async.AsyncUtil; @@ -520,7 +519,7 @@ public class StackTester { while(true) { Transaction t = db.createTransaction(); List keyValues = t.getRange(begin, endKey/*, 1000*/).asList().join(); - t.dispose(); + t.close(); if(keyValues.size() == 0) { break; } @@ -677,7 +676,7 @@ public class StackTester { tr.options().setTimeout(60*1000); tr.options().setReadSystemKeys(); tr.getReadVersion().join(); - DisposableAsyncIterator boundaryKeys = LocalityUtil.getBoundaryKeys( + CloseableAsyncIterator boundaryKeys = LocalityUtil.getBoundaryKeys( tr, new byte[0], new byte[]{(byte) 255, (byte) 255}); try { List keys = AsyncUtil.collect(boundaryKeys).join(); @@ -696,7 +695,7 @@ public class StackTester { return null; } finally { - boundaryKeys.dispose(); + boundaryKeys.close(); } }); } @@ -722,7 +721,7 @@ public class StackTester { //System.out.println("Starting test..."); c.run(); //System.out.println("Done with test."); - db.dispose(); + db.close(); System.gc(); } } From f76e6729fcadd1f58ba54e0ce1b79030d0606b33 Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Mon, 11 Dec 2017 15:01:28 -0800 Subject: [PATCH 2/4] Revert whitespace change --- .../src-completable/main/com/apple/foundationdb/Database.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/Database.java b/bindings/java/src-completable/main/com/apple/foundationdb/Database.java index 81127463f1..9906b72f93 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/Database.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/Database.java @@ -94,7 +94,7 @@ public interface Database extends AutoCloseable, TransactionContext { * @param retryable the block of logic to execute in a {@link Transaction} against * this database * @param e the {@link Executor} to use for asynchronous callbacks - * + * * @see #read(Function) */ T read(Function retryable, Executor e); @@ -128,7 +128,7 @@ public interface Database extends AutoCloseable, TransactionContext { * @param retryable the block of logic to execute in a {@link ReadTransaction} against * this database * @param e the {@link Executor} to use for asynchronous callbacks - * + * * @see #readAsync(Function) */ CompletableFuture readAsync( From db4c3cf74047fbb92c8ca742266cdd9edd6d065c Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Mon, 11 Dec 2017 18:28:17 -0800 Subject: [PATCH 3/4] Minor documentation and import edits --- .../com/apple/foundationdb/async/CloseableAsyncIterator.java | 2 +- .../test/com/apple/foundationdb/test/StackTester.java | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/async/CloseableAsyncIterator.java b/bindings/java/src-completable/main/com/apple/foundationdb/async/CloseableAsyncIterator.java index cd9eaa4898..ebad0155b6 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/async/CloseableAsyncIterator.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/async/CloseableAsyncIterator.java @@ -37,7 +37,7 @@ public interface CloseableAsyncIterator extends AutoCloseable, AsyncIterator< void close(); /** - * Alias for {@link #close} + * Alias for {@link #close}. */ @Override default void cancel() { diff --git a/bindings/java/src-completable/test/com/apple/foundationdb/test/StackTester.java b/bindings/java/src-completable/test/com/apple/foundationdb/test/StackTester.java index 8fabc80aa8..343a2f6bc1 100644 --- a/bindings/java/src-completable/test/com/apple/foundationdb/test/StackTester.java +++ b/bindings/java/src-completable/test/com/apple/foundationdb/test/StackTester.java @@ -40,12 +40,11 @@ import com.apple.foundationdb.Range; import com.apple.foundationdb.StreamingMode; import com.apple.foundationdb.Transaction; import com.apple.foundationdb.async.AsyncIterable; +import com.apple.foundationdb.async.AsyncUtil; import com.apple.foundationdb.async.CloseableAsyncIterator; import com.apple.foundationdb.tuple.ByteArrayUtil; import com.apple.foundationdb.tuple.Tuple; -import com.apple.foundationdb.async.AsyncUtil; - /** * Implements a cross-binding test of the FoundationDB API. * From b354c7fc00845914143e69977c3ba9071bd36cad Mon Sep 17 00:00:00 2001 From: "A.J. Beamon" Date: Tue, 12 Dec 2017 09:24:34 -0800 Subject: [PATCH 4/4] Remove unused constructor, add close() to BoundaryIterator finalizer for now, remove unused debug method from NativeFuture --- .../main/com/apple/foundationdb/LocalityUtil.java | 1 + .../main/com/apple/foundationdb/NativeFuture.java | 10 ++-------- .../com/apple/foundationdb/NativeObjectWrapper.java | 3 --- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/LocalityUtil.java b/bindings/java/src-completable/main/com/apple/foundationdb/LocalityUtil.java index 9a3cd7fbfb..a6bc704ea8 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/LocalityUtil.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/LocalityUtil.java @@ -221,6 +221,7 @@ public class LocalityUtil { if(FDB.getInstance().warnOnUnclosed && !closed) { System.err.println("CloseableAsyncIterator not closed (getBoundaryKeys)"); } + close(); } finally { super.finalize(); diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/NativeFuture.java b/bindings/java/src-completable/main/com/apple/foundationdb/NativeFuture.java index ecb4704333..b63c115b13 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/NativeFuture.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/NativeFuture.java @@ -69,10 +69,8 @@ abstract class NativeFuture extends CompletableFuture implements AutoClose complete(val); } } catch(FDBException t) { - assert(t.getCode() != 2015); // future_not_set not possible - if(t.getCode() != 1102) { // future_released - completeExceptionally(t); - } + assert(t.getCode() != 1102 && t.getCode() != 2015); // future_released, future_not_set not possible + completeExceptionally(t); } catch(Throwable t) { completeExceptionally(t); } finally { @@ -132,10 +130,6 @@ abstract class NativeFuture extends CompletableFuture implements AutoClose return cPtr; } - boolean isCleanedUp() { - return isDone() && cPtr == 0; - } - private native void Future_registerCallback(long cPtr, Runnable callback); private native void Future_blockUntilReady(long cPtr); private native boolean Future_isReady(long cPtr); diff --git a/bindings/java/src-completable/main/com/apple/foundationdb/NativeObjectWrapper.java b/bindings/java/src-completable/main/com/apple/foundationdb/NativeObjectWrapper.java index 0d13052535..b2e89dbfbf 100644 --- a/bindings/java/src-completable/main/com/apple/foundationdb/NativeObjectWrapper.java +++ b/bindings/java/src-completable/main/com/apple/foundationdb/NativeObjectWrapper.java @@ -30,9 +30,6 @@ abstract class NativeObjectWrapper implements AutoCloseable { private boolean closed = false; private long cPtr; - public NativeObjectWrapper() { - } - public NativeObjectWrapper(long cPtr) { this.cPtr = cPtr; if(this.cPtr == 0)