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

Commit be5bebaf authored by Li Li's avatar Li Li
Browse files

Use CloseGuard for cursor leak detection 



Protect Cursors with CloseGuard and report implicit cleanup.

By default, the leak detection code will print a one-line warning
message about failing to call the cleanup function. To report more
details like the original stacktrace and SQLite query statement, enable
the correspoinding VmPolicy flags as the example below, please.

/*
 * public void onCreate() {
 *     if (DEVELOPER_MODE) {
 *         StrictMode.setVmPolicy(new VmPolicy.Builder()
 *                 .detectLeakedSqlLiteObjects()  // for SqlLiteCursor
 *                 .detectLeakedClosableObjects() // for any Cursor
 *                 .penaltyLog()
 *                 .build());
 *     }
 *     super.onCreate();
 * }
 */

By enabling detectLeakedSqlLiteObjecs, the original SQLiteCursor query
statement is reported when close() or its equivalent cleanup function
is not called before finalize().

By enabling detectLeakedClosableObjects, the new CloseGuard report will
be provided, along with the original stack trace captured in open() or
its equivalent.

The former has better performance as it doesn't capture an original
stack trace during open() stage. Only enable the latter if performance
impact is not an issue. Both of them can be enabled at the same time.

Bug: 168639120
Test: manually test with an example SQLite Android application, with and
without calling cursor.close() after a db.rawQuery().

Change-Id: Ibe9fcdc8119c2e4651df1983e7ccd793f29e8e9d
Signed-off-by: default avatarLi Li <dualli@google.com>
parent 6cac0b0f

core/java/android/database/AbstractCursor.java

+8 −0
Original line number Diff line number Diff line
@@ -24,6 +24,8 @@ import android.os.Build; 
import android.os.Bundle;

import android.util.Log;



import dalvik.system.CloseGuard;



import java.lang.ref.WeakReference;

import java.util.Arrays;

import java.util.HashMap;
@@ -86,6 +88,9 @@ public abstract class AbstractCursor implements CrossProcessCursor { 
    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)

    private Bundle mExtras = Bundle.EMPTY;



    /** CloseGuard to detect leaked cursor **/

    private final CloseGuard mCloseGuard = CloseGuard.get();



    /* -------------------------------------------------------- */

    /* These need to be implemented by subclasses */

    @Override
@@ -179,6 +184,7 @@ public abstract class AbstractCursor implements CrossProcessCursor { 
        mClosed = true;

        mContentObservable.unregisterAll();

        onDeactivateOrClose();

        mCloseGuard.close();

    }



    /**
@@ -218,6 +224,7 @@ public abstract class AbstractCursor implements CrossProcessCursor { 
    /* Implementation */

    public AbstractCursor() {

        mPos = -1;

        mCloseGuard.open("close");

    }



    @Override
@@ -521,6 +528,7 @@ public abstract class AbstractCursor implements CrossProcessCursor { 
            mContentResolver.unregisterContentObserver(mSelfObserver);

        }

        try {

            if (mCloseGuard != null) mCloseGuard.warnIfOpen();

            if (!mClosed) close();

        } catch(Exception e) { }

    }

core/java/android/database/sqlite/SQLiteCursor.java

+7 −15
Original line number Diff line number Diff line
@@ -62,9 +62,6 @@ public class SQLiteCursor extends AbstractWindowedCursor { 
    /** A mapping of column names to column indices, to speed up lookups */

    private Map<String, Integer> mColumnNameMap;



    /** Used to find out where a cursor was allocated in case it never got released. */

    private final Throwable mStackTrace;



    /** Controls fetching of rows relative to requested position **/

    private boolean mFillWindowForwardOnly;
@@ -102,11 +99,6 @@ public class SQLiteCursor extends AbstractWindowedCursor { 
        if (query == null) {

            throw new IllegalArgumentException("query object cannot be null");

        }

        if (StrictMode.vmSqliteObjectLeaksEnabled()) {

            mStackTrace = new DatabaseObjectNotClosedException().fillInStackTrace();

        } else {

            mStackTrace = null;

        }

        mDriver = driver;

        mEditTable = editTable;

        mColumnNameMap = null;
@@ -283,17 +275,17 @@ public class SQLiteCursor extends AbstractWindowedCursor { 
        try {

            // if the cursor hasn't been closed yet, close it first

            if (mWindow != null) {

                if (mStackTrace != null) {

                // Report original sql statement

                if (StrictMode.vmSqliteObjectLeaksEnabled()) {

                    String sql = mQuery.getSql();

                    int len = sql.length();

                    StrictMode.onSqliteObjectLeaked(

                        "Finalizing a Cursor that has not been deactivated or closed. " +

                        "database = " + mQuery.getDatabase().getLabel() +

                        ", table = " + mEditTable +

                        ", query = " + sql.substring(0, (len > 1000) ? 1000 : len),

                        mStackTrace);

                            "Finalizing a Cursor that has not been deactivated or closed. "

                            + "database = " + mQuery.getDatabase().getLabel()

                            + ", table = " + mEditTable

                            + ", query = " + sql.substring(0, (len > 1000) ? 1000 : len),

                            null);

                }

                close();

            }

        } finally {

            super.finalize();