diff --git a/CHANGELOG.md b/CHANGELOG.md index c2b5b8f9c4..21b00f8ab8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.90.2 + +### Bug fixes + +* `OnSuccess.OnSuccess()` might not be called with the correct Realm version for async transaction (#1893). + ## 0.90.1 * Updated Realm Core to 0.100.2. diff --git a/realm/realm-library/src/androidTest/java/io/realm/RealmAsyncQueryTests.java b/realm/realm-library/src/androidTest/java/io/realm/RealmAsyncQueryTests.java index 6fe55d0990..ed37898a25 100644 --- a/realm/realm-library/src/androidTest/java/io/realm/RealmAsyncQueryTests.java +++ b/realm/realm-library/src/androidTest/java/io/realm/RealmAsyncQueryTests.java @@ -194,7 +194,7 @@ public void onSuccess() { @Override public void onError(Throwable error) { // Ensure we are giving developers quality messages in the logs. - assertEquals("Could not cancel transaction, not currently in a transaction.", testLogger.message); + assertEquals("Could not cancel transaction, not currently in a transaction.", testLogger.previousMessage); RealmLog.remove(testLogger); looperThread.testComplete(); } @@ -281,6 +281,88 @@ public void execute(Realm realm) { }, transactionCallback); } + // Test case for https://github.com/realm/realm-java/issues/1893 + // Ensure that onSuccess is called with the correct Realm version for async transaction. + @Test + @RunTestInLooperThread + public void executeTransactionAsync_asyncQuery() { + final Realm realm = looperThread.realm; + RealmResults results = realm.where(AllTypes.class).findAllAsync(); + assertEquals(0, results.size()); + + realm.executeTransactionAsync(new Realm.Transaction() { + @Override + public void execute(Realm realm) { + realm.createObject(AllTypes.class); + } + }, new Realm.Transaction.OnSuccess() { + @Override + public void onSuccess() { + assertEquals(realm.where(AllTypes.class).count(), 1); + looperThread.testComplete(); + } + }, new Realm.Transaction.OnError() { + @Override + public void onError(Throwable error) { + fail(); + } + }); + } + + // Test case for https://github.com/realm/realm-java/issues/1893 + // Ensure that Realm.close() logs a warning if you close a Realm with pending transactions. + @Test + @UiThreadTest + public void executeTransactionAsync_closeWithPendingTransaction() throws NoSuchFieldException, IllegalAccessException { + RealmConfiguration config = configFactory.createConfiguration(); + Realm realm = Realm.getInstance(config); + + final CountDownLatch closeLatch = new CountDownLatch(1); + final CountDownLatch waitAsyncTransaction = new CountDownLatch(1); + + realm.executeTransactionAsync(new Realm.Transaction() { + @Override + public void execute(Realm realm) { + TestHelper.awaitOrFail(closeLatch); + realm.close(); // It is for avoiding java.lang.IllegalStateException: It's not allowed to delete the file associated with an open Realm. + waitAsyncTransaction.countDown(); + } + }); + + final TestHelper.TestLogger testLogger = new TestHelper.TestLogger(); + RealmLog.add(testLogger); + realm.close(); + RealmLog.remove(testLogger); + String[] logs = testLogger.message.split("will "); + assertEquals("be closed with pending async transactions or callbacks.", logs[1]); + + closeLatch.countDown(); + TestHelper.awaitOrFail(waitAsyncTransaction); + } + + // Test case for https://github.com/realm/realm-java/issues/1893 + // Ensure that Realm.close() logs a warning if you close a Realm with pending callbacks. + @Test + @UiThreadTest + public void executeTransactionAsync_closeWithPendingTransactionCallback() { + RealmConfiguration config = configFactory.createConfiguration(); + Realm realm = Realm.getInstance(config); + + realm.asyncTransactionCallbacks.add(new Runnable() { + @Override + public void run() { + } + }); + + final TestHelper.TestLogger testLogger = new TestHelper.TestLogger(); + RealmLog.add(testLogger); + realm.close(); + RealmLog.remove(testLogger); + + String[] logs = testLogger.message.split("will "); + assertEquals("be closed with pending async transactions or callbacks.", logs[1]); + } + // ************************************ // *** promises based async queries *** // ************************************ @@ -1948,7 +2030,7 @@ public void onChange(RealmResults object) { // 3. The async query should now (hopefully) fail with a BadVersion result.load(); } - + // handlerController#emptyAsyncRealmObject is accessed from different threads // make sure that we iterate over it safely without any race condition (ConcurrentModification) @Test @@ -2004,7 +2086,7 @@ public void run() { @Test @UiThreadTest public void concurrentModificationRealmObjects() { - RealmConfiguration config = configFactory.createConfiguration(); + RealmConfiguration config = configFactory.createConfiguration(); final Realm realm = Realm.getInstance(config); Dog dog1 = new Dog(); dog1.setName("Dog 1"); @@ -2017,8 +2099,8 @@ public void concurrentModificationRealmObjects() { dog2 = realm.copyToRealm(dog2); realm.commitTransaction(); - final WeakReference weakReference1 = new WeakReference((RealmObjectProxy) dog1); - final WeakReference weakReference2 = new WeakReference((RealmObjectProxy) dog2); + final WeakReference weakReference1 = new WeakReference((RealmObjectProxy)dog1); + final WeakReference weakReference2 = new WeakReference((RealmObjectProxy)dog2); realm.handlerController.realmObjects.put(weakReference1, Boolean.TRUE); @@ -2045,36 +2127,6 @@ public void run() { realm.close(); } - @Test - @RunTestInLooperThread - public void testAsyncTransactionWorksWithAsyncQuery() { - RealmResults results = looperThread.realm.where(AllTypes.class).findAllAsync(); - results.addChangeListener(new RealmChangeListener() { - @Override - public void onChange() { - //assertEquals(looperThread.realm.where(AllTypes.class).count(), 1); - } - }); - looperThread.keepStrongReference.add(results); - looperThread.realm.executeTransactionAsync(new Realm.Transaction() { - @Override - public void execute(Realm realm) { - realm.createObject(AllTypes.class); - } - }, new Realm.Transaction.OnSuccess() { - @Override - public void onSuccess() { - assertEquals(looperThread.realm.where(AllTypes.class).count(), 1); - looperThread.testComplete(); - } - }, new Realm.Transaction.OnError() { - @Override - public void onError(Throwable error) { - fail(); - } - }); - } - // *** Helper methods *** private void populateTestRealm(final Realm testRealm, int objects) { diff --git a/realm/realm-library/src/androidTest/java/io/realm/RealmResultsTests.java b/realm/realm-library/src/androidTest/java/io/realm/RealmResultsTests.java index e8c09c7013..dc4b77a638 100644 --- a/realm/realm-library/src/androidTest/java/io/realm/RealmResultsTests.java +++ b/realm/realm-library/src/androidTest/java/io/realm/RealmResultsTests.java @@ -348,12 +348,12 @@ public void changeListener_syncIfNeeded_updatedFromOtherThread() { assertEquals(10, results.size()); // 1. Delete first object from another thread. - realm.executeTransaction(new Realm.Transaction() { + realm.executeTransactionAsync(new Realm.Transaction() { @Override public void execute(Realm realm) { realm.where(AllTypes.class).equalTo(AllTypes.FIELD_LONG, 0).findFirst().removeFromRealm(); } - }, new Realm.Transaction.Callback() { + }, new Realm.Transaction.OnSuccess() { @Override public void onSuccess() { // 2. RealmResults are refreshed before onSuccess is called diff --git a/realm/realm-library/src/androidTest/java/io/realm/TestHelper.java b/realm/realm-library/src/androidTest/java/io/realm/TestHelper.java index e6865dc211..1b5833f22c 100644 --- a/realm/realm-library/src/androidTest/java/io/realm/TestHelper.java +++ b/realm/realm-library/src/androidTest/java/io/realm/TestHelper.java @@ -246,60 +246,77 @@ public static String getRandomString(int length) { */ public static class TestLogger implements Logger { + public String previousMessage; public String message; public Throwable throwable; + private void backupMessage() { + if (this.message != null) { + previousMessage = this.message; + } + } + @Override public void v(String message) { + backupMessage(); this.message = message; } @Override public void v(String message, Throwable t) { + backupMessage(); this.message = message; this.throwable = t; } @Override public void d(String message) { + backupMessage(); this.message = message; } @Override public void d(String message, Throwable t) { + backupMessage(); this.message = message; this.throwable = t; } @Override public void i(String message) { + backupMessage(); this.message = message; } @Override public void i(String message, Throwable t) { + backupMessage(); this.message = message; this.throwable = t; } @Override public void w(String message) { + backupMessage(); this.message = message; } @Override public void w(String message, Throwable t) { + backupMessage(); this.message = message; this.throwable = t; } @Override public void e(String message) { + backupMessage(); this.message = message; } @Override public void e(String message, Throwable t) { + backupMessage(); this.message = message; this.throwable = t; } diff --git a/realm/realm-library/src/main/java/io/realm/BaseRealm.java b/realm/realm-library/src/main/java/io/realm/BaseRealm.java index 074bc81e53..dba4dc2bf8 100644 --- a/realm/realm-library/src/main/java/io/realm/BaseRealm.java +++ b/realm/realm-library/src/main/java/io/realm/BaseRealm.java @@ -22,6 +22,7 @@ import java.io.Closeable; import java.io.File; import java.io.FileNotFoundException; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -66,6 +67,7 @@ abstract class BaseRealm implements Closeable { RealmSchema schema; Handler handler; HandlerController handlerController; + List asyncTransactionCallbacks = new ArrayList(); static { RealmLog.add(BuildConfig.DEBUG ? new DebugAndroidLogger() : new ReleaseAndroidLogger()); @@ -458,6 +460,11 @@ public void close() { * Closes the Realm instances and all its resources without checking the {@link RealmCache}. */ void doClose() { + if (asyncTaskExecutor.hasPendingTransactions() || !asyncTransactionCallbacks.isEmpty()) { + String canonicalPath = getPath(); + RealmLog.w("Realm " + canonicalPath + " will be closed with pending async transactions or callbacks."); + } + if (sharedGroupManager != null) { sharedGroupManager.close(); sharedGroupManager = null; diff --git a/realm/realm-library/src/main/java/io/realm/HandlerController.java b/realm/realm-library/src/main/java/io/realm/HandlerController.java index 6ef77f81dd..22aeb59ea3 100644 --- a/realm/realm-library/src/main/java/io/realm/HandlerController.java +++ b/realm/realm-library/src/main/java/io/realm/HandlerController.java @@ -515,6 +515,17 @@ private void completedAsyncQueriesUpdate(QueryUpdateTask.Result result) { updateAsyncQueriesTask = null; } + + executeAsyncTransactionCallbacks(); + } + + void executeAsyncTransactionCallbacks() { + if (!realm.asyncTransactionCallbacks.isEmpty()) { + for (Runnable transactionCallback : realm.asyncTransactionCallbacks) { + realm.handler.post(transactionCallback); + } + realm.asyncTransactionCallbacks.clear(); + } } private void completedAsyncRealmObject(QueryUpdateTask.Result result) { @@ -584,7 +595,7 @@ private void completedAsyncRealmObject(QueryUpdateTask.Result result) { * @return {@code true} if there is at least one (non GC'ed) instance of {@link RealmResults} {@code false} * otherwise. */ - private boolean threadContainsAsyncQueries() { + boolean threadContainsAsyncQueries() { boolean isEmpty = true; Iterator>, RealmQuery>> iterator = asyncRealmResults.entrySet().iterator(); while (iterator.hasNext()) { diff --git a/realm/realm-library/src/main/java/io/realm/Realm.java b/realm/realm-library/src/main/java/io/realm/Realm.java index c8926b45f9..e4bac47f7d 100644 --- a/realm/realm-library/src/main/java/io/realm/Realm.java +++ b/realm/realm-library/src/main/java/io/realm/Realm.java @@ -1112,87 +1112,19 @@ public void executeTransaction(Transaction transaction) { */ @Deprecated public RealmAsyncTask executeTransaction(final Transaction transaction, final Transaction.Callback callback) { - checkIfValid(); - if (transaction == null) { - throw new IllegalArgumentException("Transaction should not be null"); - } - - // If the user provided a Callback then we make sure, the current Realm has a Handler - // we can use to deliver the result - if (callback != null && handler == null) { - throw new IllegalStateException("Your Realm is opened from a thread without a Looper" + - " and you provided a callback, we need a Handler to invoke your callback"); - } - - // We need to use the same configuration to open a background SharedGroup (i.e Realm) - // to perform the transaction - final RealmConfiguration realmConfiguration = getConfiguration(); - - final Future pendingTransaction = asyncTaskExecutor.submit(new Runnable() { + return executeTransactionAsync(transaction, new Transaction.OnSuccess() { @Override - public void run() { - if (Thread.currentThread().isInterrupted()) { - return; - } - - boolean transactionCommitted = false; - final Exception[] exception = new Exception[1]; - final Realm bgRealm = Realm.getInstance(realmConfiguration); - bgRealm.beginTransaction(); - try { - transaction.execute(bgRealm); - - if (!Thread.currentThread().isInterrupted()) { - bgRealm.commitTransaction(false, new Runnable() { - @Override - public void run() { - // The bgRealm needs to be closed before post event to caller's handler to avoid - // concurrency problem. eg.: User wants to delete Realm in the callbacks. - // This will close Realm before sending REALM_CHANGED. - bgRealm.close(); - } - }); - transactionCommitted = true; - } - } catch (final Exception e) { - exception[0] = e; - } finally { - if (!bgRealm.isClosed()) { - if (bgRealm.isInTransaction()) { - bgRealm.cancelTransaction(); - } else if (exception[0] != null) { - RealmLog.w("Could not cancel transaction, not currently in a transaction."); - } - bgRealm.close(); - } - - // Send response as the final step to ensure the bg thread quit before others get the response! - if (callback != null - && handler != null - && !Thread.currentThread().isInterrupted() - && handler.getLooper().getThread().isAlive()) { - if (transactionCommitted) { - handler.post(new Runnable() { - @Override - public void run() { - callback.onSuccess(); - } - }); - } else if (exception[0] != null) { - // transaction has not been canceled by there is a exception during transaction. - handler.post(new Runnable() { - @Override - public void run() { - callback.onError(exception[0]); - } - }); - } - } + public void onSuccess() { + callback.onSuccess(); + } + }, new Transaction.OnError() { + @Override + public void onError(Throwable error) { + if (error instanceof Exception) { + callback.onError((Exception) error); } } }); - - return new RealmAsyncTask(pendingTransaction); } /** @@ -1269,7 +1201,7 @@ public RealmAsyncTask executeTransactionAsync(final Transaction transaction, fin // to perform the transaction final RealmConfiguration realmConfiguration = getConfiguration(); - final Future pendingTransaction= asyncTaskExecutor.submit(new Runnable() { + final Future pendingTransaction = asyncTaskExecutor.submitTransaction(new Runnable() { @Override public void run() { if (Thread.currentThread().isInterrupted()) { @@ -1313,7 +1245,7 @@ public void run() { && !Thread.currentThread().isInterrupted() && handler.getLooper().getThread().isAlive()) { if (onSuccess != null && transactionCommitted) { - handler.post(new Runnable() { + asyncTransactionCallbacks.add(new Runnable() { @Override public void run() { onSuccess.onSuccess(); @@ -1323,14 +1255,14 @@ public void run() { if (backgroundException != null) { if (onError != null) { - handler.post(new Runnable() { + asyncTransactionCallbacks.add(new Runnable() { @Override public void run() { onError.onError(backgroundException); } }); } else { - handler.post(new Runnable() { + asyncTransactionCallbacks.add(new Runnable() { @Override public void run() { if (backgroundException instanceof RuntimeException) { @@ -1344,6 +1276,10 @@ public void run() { }); } } + + if (!handlerController.threadContainsAsyncQueries()) { + handlerController.executeAsyncTransactionCallbacks(); + } } else { // Throw exception in the worker thread if the caller thread terminated if (backgroundException != null) { @@ -1366,7 +1302,6 @@ public void run() { return new RealmAsyncTask(pendingTransaction); } - /** * Removes all objects of the specified class. * diff --git a/realm/realm-library/src/main/java/io/realm/internal/async/RealmThreadPoolExecutor.java b/realm/realm-library/src/main/java/io/realm/internal/async/RealmThreadPoolExecutor.java index 24246f1526..a1fe48d3e5 100644 --- a/realm/realm-library/src/main/java/io/realm/internal/async/RealmThreadPoolExecutor.java +++ b/realm/realm-library/src/main/java/io/realm/internal/async/RealmThreadPoolExecutor.java @@ -16,6 +16,10 @@ package io.realm.internal.async; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.Callable; import java.util.concurrent.Future; @@ -38,6 +42,7 @@ public class RealmThreadPoolExecutor extends ThreadPoolExecutor { private boolean isPaused; private ReentrantLock pauseLock = new ReentrantLock(); private Condition unpaused = pauseLock.newCondition(); + private List> transactions = Collections.synchronizedList(new ArrayList>()); /** * Creates a default RealmThreadPool that is bounded by the number of available cores. @@ -59,6 +64,12 @@ private RealmThreadPoolExecutor(int corePoolSize, int maxPoolSize) { new ArrayBlockingQueue(QUEUE_SIZE)); } + public Future submitTransaction(Runnable task) { + Future future = this.submit(task); + transactions.add(future); + return future; + } + @Override public Future submit(Runnable task) { return super.submit(new BgPriorityRunnable(task)); @@ -71,7 +82,7 @@ public Future submit(Callable task) { @Override protected void beforeExecute(Thread t, Runnable r) { - super.beforeExecute(t, r); + super.beforeExecute(t, r); pauseLock.lock(); try { while (isPaused) unpaused.await(); @@ -82,6 +93,27 @@ protected void beforeExecute(Thread t, Runnable r) { } } + @Override + protected void afterExecute(Runnable r, Throwable t) { + super.afterExecute(r, t); + tidyTransactions(); + } + + public boolean hasPendingTransactions() { + tidyTransactions(); + return !transactions.isEmpty(); + } + + private void tidyTransactions() { + Iterator> iterator = transactions.iterator(); + if (iterator.hasNext()) { + Future future = iterator.next(); + if (future.isCancelled() || future.isDone()) { + iterator.remove(); + } + } + } + /** * Pauses the executor. Pausing means the executor will stop starting new tasks (but complete current ones). */