Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 223ba7e1 authored by Lee Shombert's avatar Lee Shombert
Browse files

Report source of db closure 

If a database is closed directly, record the caller as a stack.
Include the caller in the exception that is thrown when an operation
is attempted on a closed database.  If the exception percolates up to
the framework, the framework will log the exception and its cause.

This covers the common case of a database being closed because
corruption was detected.  It makes it easier for triage to understand
why the database was closed unexpectedly.

Test: atest
 * FrameworksCoreTests:android.database
 * CtsDatabaseTestCases

Flag: EXEMPT bugfix
Bug: 338297108
Change-Id: Id9bf31177881644922f31d2a353aafd1aab2823e
parent 353572f3

core/java/android/database/sqlite/SQLiteClosable.java

+21 −1
Original line number Diff line number Diff line
@@ -30,6 +30,20 @@ public abstract class SQLiteClosable implements Closeable { 
    @UnsupportedAppUsage

    private int mReferenceCount = 1;



    /**

     * True if the instance should record when it was closed.  Tracking closure can be expensive,

     * so it is best reserved for subclasses that have long lifetimes.

     * @hide

     */

    protected boolean mTrackClosure = false;



    /**

     * The caller that finally released this instance.  If this is not null, it is supplied as the

     * cause to the IllegalStateException that is thrown when the object is reopened.  Subclasses

     * are responsible for populating this field, if they wish to use it.

     */

    private Throwable mClosedBy = null;



    /**

     * Called when the last reference to the object was released by

     * a call to {@link #releaseReference()} or {@link #close()}.
@@ -57,7 +71,7 @@ public abstract class SQLiteClosable implements Closeable { 
        synchronized(this) {

            if (mReferenceCount <= 0) {

                throw new IllegalStateException(

                        "attempt to re-open an already-closed object: " + this);

                    "attempt to re-open an already-closed object: " + this, mClosedBy);

            }

            mReferenceCount++;

        }
@@ -108,5 +122,11 @@ public abstract class SQLiteClosable implements Closeable { 
     */

    public void close() {

        releaseReference();

        synchronized (this) {

            if (mTrackClosure && (mClosedBy == null)) {

                String name = getClass().getName();

                mClosedBy = new Exception("closed by " + name + ".close()").fillInStackTrace();

            }

        }

    }

}

core/java/android/database/sqlite/SQLiteConnectionPool.java

+6 −1
Original line number Diff line number Diff line
@@ -96,6 +96,10 @@ public final class SQLiteConnectionPool implements Closeable { 
    private boolean mIsOpen;

    private int mNextConnectionId;



    // Record the caller that explicitly closed the database.

    @GuardedBy("mLock")

    private Throwable mClosedBy;



    private ConnectionWaiter mConnectionWaiterPool;

    private ConnectionWaiter mConnectionWaiterQueue;
@@ -265,6 +269,7 @@ public final class SQLiteConnectionPool implements Closeable { 
                throwIfClosedLocked();



                mIsOpen = false;

                mClosedBy = new Exception("SQLiteConnectionPool.close()").fillInStackTrace();



                closeAvailableConnectionsAndLogExceptionsLocked();
@@ -1101,7 +1106,7 @@ public final class SQLiteConnectionPool implements Closeable { 
    private void throwIfClosedLocked() {

        if (!mIsOpen) {

            throw new IllegalStateException("Cannot perform this operation "

                    + "because the connection pool has been closed.");

                    + "because the connection pool has been closed.", mClosedBy);

        }

    }

core/java/android/database/sqlite/SQLiteDatabase.java

+1 −0
Original line number Diff line number Diff line
@@ -488,6 +488,7 @@ public final class SQLiteDatabase extends SQLiteClosable { 
            @Nullable CursorFactory cursorFactory, @Nullable DatabaseErrorHandler errorHandler,

            int lookasideSlotSize, int lookasideSlotCount, long idleConnectionTimeoutMs,

            @Nullable String journalMode, @Nullable String syncMode) {

        mTrackClosure = true;

        mCursorFactory = cursorFactory;

        mErrorHandler = errorHandler != null ? errorHandler : new DefaultDatabaseErrorHandler();

        mConfigurationLocked = new SQLiteDatabaseConfiguration(path, openFlags);

core/tests/coretests/src/android/database/sqlite/SQLiteDatabaseTest.java

+67 −0
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import static org.junit.Assert.fail; 
import android.content.Context;

import android.database.Cursor;

import android.database.DatabaseUtils;

import android.database.DefaultDatabaseErrorHandler;

import android.platform.test.annotations.RequiresFlagsEnabled;

import android.platform.test.flag.junit.CheckFlagsRule;

import android.platform.test.flag.junit.DeviceFlagsValueProvider;
@@ -592,4 +593,70 @@ public class SQLiteDatabaseTest { 
        }

        closeAndDeleteDatabase();

    }



    @Test

    public void testCloseCorruptionReport() throws Exception {

        mDatabase.beginTransaction();

        try {

            mDatabase.execSQL("CREATE TABLE t2 (i int, j int);");

            mDatabase.execSQL("INSERT INTO t2 (i, j) VALUES (2, 20)");

            mDatabase.execSQL("INSERT INTO t2 (i, j) VALUES (3, 30)");

            mDatabase.setTransactionSuccessful();

        } finally {

            mDatabase.endTransaction();

        }



        // Start a transaction and announce that the DB is corrupted.

        DefaultDatabaseErrorHandler errorHandler = new DefaultDatabaseErrorHandler();



        // Do not bother with endTransaction; the database will have been closed in the corruption

        // handler.

        mDatabase.beginTransaction();

        try {

            errorHandler.onCorruption(mDatabase);

            mDatabase.execSQL("INSERT INTO t2 (i, j) VALUES (4, 40)");

            fail("expected an exception");

        } catch (IllegalStateException e) {

            final Throwable cause = e.getCause();

            assertNotNull(cause);

            boolean found = false;

            for (StackTraceElement s : cause.getStackTrace()) {

                if (s.getMethodName().contains("onCorruption")) {

                    found = true;

                }

            }

            assertTrue(found);

        }

    }



    @Test

    public void testCloseReport() throws Exception {

        mDatabase.beginTransaction();

        try {

            mDatabase.execSQL("CREATE TABLE t2 (i int, j int);");

            mDatabase.execSQL("INSERT INTO t2 (i, j) VALUES (2, 20)");

            mDatabase.execSQL("INSERT INTO t2 (i, j) VALUES (3, 30)");

            mDatabase.setTransactionSuccessful();

        } finally {

            mDatabase.endTransaction();

        }



        mDatabase.close();

        try {

            // Do not bother with endTransaction; the database has already been close.

            mDatabase.beginTransaction();

            fail("expected an exception");

        } catch (IllegalStateException e) {

            assertTrue(e.toString().contains("attempt to re-open an already-closed object"));

            final Throwable cause = e.getCause();

            assertNotNull(cause);

            boolean found = false;

            for (StackTraceElement s : cause.getStackTrace()) {

                if (s.getMethodName().contains("testCloseReport")) {

                    found = true;

                }

            }

            assertTrue(found);

        }

    }

}