From 6310dfd04e0d3561d1ef58121046e6255eac8c87 Mon Sep 17 00:00:00 2001 From: linyuh Date: Thu, 28 Jun 2018 10:14:11 -0700 Subject: Also ignore IllegalStateException thrown by SQLiteClosable when coalescing fails. Test: Manual PiperOrigin-RevId: 202499434 Change-Id: Ie41eeb782072d82c5613b44be99649f43807498d --- .../android/dialer/calllog/database/Coalescer.java | 84 +++++++++++++++++----- .../dialer/calllog/ui/NewCallLogFragment.java | 19 +++-- 2 files changed, 73 insertions(+), 30 deletions(-) (limited to 'java') diff --git a/java/com/android/dialer/calllog/database/Coalescer.java b/java/com/android/dialer/calllog/database/Coalescer.java index 2ad9f9a97..fd751e767 100644 --- a/java/com/android/dialer/calllog/database/Coalescer.java +++ b/java/com/android/dialer/calllog/database/Coalescer.java @@ -16,6 +16,7 @@ package com.android.dialer.calllog.database; import android.database.Cursor; +import android.database.StaleDataException; import android.provider.CallLog.Calls; import android.support.annotation.NonNull; import android.support.annotation.WorkerThread; @@ -85,34 +86,72 @@ public class Coalescer { @WorkerThread @NonNull private ImmutableList coalesceInternal( - Cursor allAnnotatedCallLogRowsSortedByTimestampDesc) { + Cursor allAnnotatedCallLogRowsSortedByTimestampDesc) throws ExpectedCoalescerException { Assert.isWorkerThread(); - if (!allAnnotatedCallLogRowsSortedByTimestampDesc.moveToFirst()) { - return ImmutableList.of(); - } - ImmutableList.Builder coalescedRowListBuilder = new ImmutableList.Builder<>(); - RowCombiner rowCombiner = new RowCombiner(allAnnotatedCallLogRowsSortedByTimestampDesc); - rowCombiner.startNewGroup(); + try { + if (!allAnnotatedCallLogRowsSortedByTimestampDesc.moveToFirst()) { + return ImmutableList.of(); + } - long coalescedRowId = 0; - do { - boolean isRowMerged = rowCombiner.mergeRow(allAnnotatedCallLogRowsSortedByTimestampDesc); + RowCombiner rowCombiner = new RowCombiner(allAnnotatedCallLogRowsSortedByTimestampDesc); + rowCombiner.startNewGroup(); - if (isRowMerged) { - allAnnotatedCallLogRowsSortedByTimestampDesc.moveToNext(); - } + long coalescedRowId = 0; + do { + boolean isRowMerged = rowCombiner.mergeRow(allAnnotatedCallLogRowsSortedByTimestampDesc); + + if (isRowMerged) { + allAnnotatedCallLogRowsSortedByTimestampDesc.moveToNext(); + } + + if (!isRowMerged || allAnnotatedCallLogRowsSortedByTimestampDesc.isAfterLast()) { + coalescedRowListBuilder.add( + rowCombiner.combine().toBuilder().setId(coalescedRowId++).build()); + rowCombiner.startNewGroup(); + } + } while (!allAnnotatedCallLogRowsSortedByTimestampDesc.isAfterLast()); + + return coalescedRowListBuilder.build(); - if (!isRowMerged || allAnnotatedCallLogRowsSortedByTimestampDesc.isAfterLast()) { - coalescedRowListBuilder.add( - rowCombiner.combine().toBuilder().setId(coalescedRowId++).build()); - rowCombiner.startNewGroup(); + } catch (Exception exception) { + // Coalescing can fail if cursor "allAnnotatedCallLogRowsSortedByTimestampDesc" is closed by + // its loader while the work is still in progress. + // + // This can happen when the loader restarts and finishes loading data before the coalescing + // work is completed. + // + // This kind of failure doesn't have to crash the app as coalescing will be restarted on the + // latest data obtained by the loader. Therefore, we inspect the exception here and throw an + // ExpectedCoalescerException if it is the case described above. + // + // The type of expected exception depends on whether AbstractWindowedCursor#checkPosition() is + // called when the cursor is closed. + // (1) If it is called before the cursor is closed, we will get IllegalStateException thrown + // by SQLiteClosable when it attempts to acquire a reference to the database. + // (2) Otherwise, we will get StaleDataException thrown by AbstractWindowedCursor's + // checkPosition() method. + // + // Note that it would be more accurate to inspect the stack trace to locate the origin of the + // exception. However, according to the documentation on Throwable#getStackTrace, "some + // virtual machines may, under some circumstances, omit one or more stack frames from the + // stack trace". "In the extreme case, a virtual machine that has no stack trace information + // concerning this throwable is permitted to return a zero-length array from this method." + // Therefore, the best we can do is to inspect the message in the exception. + // TODO(linyuh): try to avoid the expected failure. + String message = exception.getMessage(); + if (message != null + && ((exception instanceof StaleDataException + && message.startsWith("Attempting to access a closed CursorWindow")) + || (exception instanceof IllegalStateException + && message.startsWith("attempt to re-open an already-closed object")))) { + throw new ExpectedCoalescerException(exception); } - } while (!allAnnotatedCallLogRowsSortedByTimestampDesc.isAfterLast()); - return coalescedRowListBuilder.build(); + throw exception; + } } /** Combines rows from {@link AnnotatedCallLog} into a {@link CoalescedRow}. */ @@ -337,4 +376,11 @@ public class Coalescer { return dialerPhoneNumberUtil.isMatch(groupPhoneNumber, rowPhoneNumber); } } + + /** A checked exception thrown when expected failure happens when coalescing is in progress. */ + public static final class ExpectedCoalescerException extends Exception { + ExpectedCoalescerException(Throwable throwable) { + super("Expected coalescing exception", throwable); + } + } } diff --git a/java/com/android/dialer/calllog/ui/NewCallLogFragment.java b/java/com/android/dialer/calllog/ui/NewCallLogFragment.java index 4141fe723..5e72a1af3 100644 --- a/java/com/android/dialer/calllog/ui/NewCallLogFragment.java +++ b/java/com/android/dialer/calllog/ui/NewCallLogFragment.java @@ -17,7 +17,6 @@ package com.android.dialer.calllog.ui; import android.app.Activity; import android.database.Cursor; -import android.database.StaleDataException; import android.os.Bundle; import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; @@ -34,6 +33,7 @@ import android.view.ViewGroup; import com.android.dialer.calllog.CallLogComponent; import com.android.dialer.calllog.RefreshAnnotatedCallLogReceiver; import com.android.dialer.calllog.database.CallLogDatabaseComponent; +import com.android.dialer.calllog.database.Coalescer; import com.android.dialer.calllog.model.CoalescedRow; import com.android.dialer.common.Assert; import com.android.dialer.common.LogUtil; @@ -337,16 +337,13 @@ public final class NewCallLogFragment extends Fragment implements LoaderCallback } }, throwable -> { - if (throwable instanceof StaleDataException) { - // Coalescing can fail if the cursor passed to Coalescer is closed by the loader while - // the work is still in progress. - // This can happen when the loader restarts and finishes loading data before the - // coalescing work is completed. - // This failure doesn't need to be thrown as coalescing will be restarted on the latest - // data obtained by the loader. - // TODO(linyuh): Also throw an exception if the failure above can be avoided. - LogUtil.e("NewCallLogFragment.onLoadFinished", "coalescing failed", throwable); - } else { + // Coalescing can fail if the cursor passed to Coalescer is closed by the loader while + // the work is still in progress. + // This can happen when the loader restarts and finishes loading data before the + // coalescing work is completed. + // This failure is identified by ExpectedCoalescerException and doesn't need to be + // thrown as coalescing will be restarted on the latest data obtained by the loader. + if (!(throwable instanceof Coalescer.ExpectedCoalescerException)) { throw new AssertionError(throwable); } }); -- cgit v1.2.3