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

Commit 559d0645 authored by Jeff Brown's avatar Jeff Brown
Browse files

Refactor SQLiteOpenHelper.

Combine the code for opening readable and writable databases.
This improves the handling of the case where a database cannot
be opened because it cannot be upgraded.  Previously we would
open the database twice: first read-write, then read-only, each
time failing due to the version check.  Now only open it once.

Removed the goofy locking logic related to upgrading a read-only
database to read-write.  We now do it in place by reopening the
necessary connections in the connection pool.

Change-Id: I6deca3fb90e43f4ccb944d4715307fd6fc3e1383
parent 2437a4ab
Loading
Loading
Loading
Loading
+113 −73
Original line number Diff line number Diff line
@@ -92,13 +92,25 @@ public final class SQLiteConnectionPool implements Closeable {
            new ArrayList<SQLiteConnection>();
    private SQLiteConnection mAvailablePrimaryConnection;

    // Describes what should happen to an acquired connection when it is returned to the pool.
    enum AcquiredConnectionStatus {
        // The connection should be returned to the pool as usual.
        NORMAL,

        // The connection must be reconfigured before being returned.
        RECONFIGURE,

        // The connection must be closed and discarded.
        DISCARD,
    }

    // Weak references to all acquired connections.  The associated value
    // is a boolean that indicates whether the connection must be reconfigured
    // before being returned to the available connection list.
    // indicates whether the connection must be reconfigured before being
    // returned to the available connection list or discarded.
    // For example, the prepared statement cache size may have changed and
    // need to be updated.
    private final WeakHashMap<SQLiteConnection, Boolean> mAcquiredConnections =
            new WeakHashMap<SQLiteConnection, Boolean>();
    // need to be updated in preparation for the next client.
    private final WeakHashMap<SQLiteConnection, AcquiredConnectionStatus> mAcquiredConnections =
            new WeakHashMap<SQLiteConnection, AcquiredConnectionStatus>();

    /**
     * Connection flag: Read-only.
@@ -168,7 +180,7 @@ public final class SQLiteConnectionPool implements Closeable {
    private void open() {
        // Open the primary connection.
        // This might throw if the database is corrupt.
        mAvailablePrimaryConnection = openConnectionLocked(
        mAvailablePrimaryConnection = openConnectionLocked(mConfiguration,
                true /*primaryConnection*/); // might throw

        // Mark the pool as being open for business.
@@ -209,16 +221,7 @@ public final class SQLiteConnectionPool implements Closeable {

                mIsOpen = false;

                final int count = mAvailableNonPrimaryConnections.size();
                for (int i = 0; i < count; i++) {
                    closeConnectionAndLogExceptionsLocked(mAvailableNonPrimaryConnections.get(i));
                }
                mAvailableNonPrimaryConnections.clear();

                if (mAvailablePrimaryConnection != null) {
                    closeConnectionAndLogExceptionsLocked(mAvailablePrimaryConnection);
                    mAvailablePrimaryConnection = null;
                }
                closeAvailableConnectionsAndLogExceptionsLocked();

                final int pendingCount = mAcquiredConnections.size();
                if (pendingCount != 0) {
@@ -254,20 +257,26 @@ public final class SQLiteConnectionPool implements Closeable {
        synchronized (mLock) {
            throwIfClosedLocked();

            final boolean poolSizeChanged = mConfiguration.maxConnectionPoolSize
                    != configuration.maxConnectionPoolSize;
            mConfiguration.updateParametersFrom(configuration);
            if (mConfiguration.openFlags != configuration.openFlags) {
                // Try to reopen the primary connection using the new open flags then
                // close and discard all existing connections.
                // This might throw if the database is corrupt or cannot be opened in
                // the new mode in which case existing connections will remain untouched.
                SQLiteConnection newPrimaryConnection = openConnectionLocked(configuration,
                        true /*primaryConnection*/); // might throw

            if (poolSizeChanged) {
                int availableCount = mAvailableNonPrimaryConnections.size();
                while (availableCount-- > mConfiguration.maxConnectionPoolSize - 1) {
                    SQLiteConnection connection =
                            mAvailableNonPrimaryConnections.remove(availableCount);
                    closeConnectionAndLogExceptionsLocked(connection);
                }
            }
                closeAvailableConnectionsAndLogExceptionsLocked();
                discardAcquiredConnectionsLocked();

                mAvailablePrimaryConnection = newPrimaryConnection;
                mConfiguration.updateParametersFrom(configuration);
            } else {
                // Reconfigure the database connections in place.
                mConfiguration.updateParametersFrom(configuration);

                closeExcessConnectionsAndLogExceptionsLocked();
                reconfigureAllConnectionsLocked();
            }

            wakeConnectionWaitersLocked();
        }
@@ -310,8 +319,8 @@ public final class SQLiteConnectionPool implements Closeable {
     */
    public void releaseConnection(SQLiteConnection connection) {
        synchronized (mLock) {
            Boolean mustReconfigure = mAcquiredConnections.remove(connection);
            if (mustReconfigure == null) {
            AcquiredConnectionStatus status = mAcquiredConnections.remove(connection);
            if (status == null) {
                throw new IllegalStateException("Cannot perform this operation "
                        + "because the specified connection was not acquired "
                        + "from this pool or has already been released.");
@@ -320,18 +329,8 @@ public final class SQLiteConnectionPool implements Closeable {
            if (!mIsOpen) {
                closeConnectionAndLogExceptionsLocked(connection);
            } else if (connection.isPrimaryConnection()) {
                if (recycleConnectionLocked(connection, status)) {
                    assert mAvailablePrimaryConnection == null;
                try {
                    if (mustReconfigure == Boolean.TRUE) {
                        connection.reconfigure(mConfiguration); // might throw
                    }
                } catch (RuntimeException ex) {
                    Log.e(TAG, "Failed to reconfigure released primary connection, closing it: "
                            + connection, ex);
                    closeConnectionAndLogExceptionsLocked(connection);
                    connection = null;
                }
                if (connection != null) {
                    mAvailablePrimaryConnection = connection;
                }
                wakeConnectionWaitersLocked();
@@ -339,22 +338,31 @@ public final class SQLiteConnectionPool implements Closeable {
                    mConfiguration.maxConnectionPoolSize - 1) {
                closeConnectionAndLogExceptionsLocked(connection);
            } else {
                if (recycleConnectionLocked(connection, status)) {
                    mAvailableNonPrimaryConnections.add(connection);
                }
                wakeConnectionWaitersLocked();
            }
        }
    }

    // Can't throw.
    private boolean recycleConnectionLocked(SQLiteConnection connection,
            AcquiredConnectionStatus status) {
        if (status == AcquiredConnectionStatus.RECONFIGURE) {
            try {
                    if (mustReconfigure == Boolean.TRUE) {
                connection.reconfigure(mConfiguration); // might throw
                    }
            } catch (RuntimeException ex) {
                    Log.e(TAG, "Failed to reconfigure released non-primary connection, "
                            + "closing it: " + connection, ex);
                    closeConnectionAndLogExceptionsLocked(connection);
                    connection = null;
                Log.e(TAG, "Failed to reconfigure released connection, closing it: "
                        + connection, ex);
                status = AcquiredConnectionStatus.DISCARD;
            }
                if (connection != null) {
                    mAvailableNonPrimaryConnections.add(connection);
                }
                wakeConnectionWaitersLocked();
        }
        if (status == AcquiredConnectionStatus.DISCARD) {
            closeConnectionAndLogExceptionsLocked(connection);
            return false;
        }
        return true;
    }

    /**
@@ -407,9 +415,10 @@ public final class SQLiteConnectionPool implements Closeable {
    }

    // Might throw.
    private SQLiteConnection openConnectionLocked(boolean primaryConnection) {
    private SQLiteConnection openConnectionLocked(SQLiteDatabaseConfiguration configuration,
            boolean primaryConnection) {
        final int connectionId = mNextConnectionId++;
        return SQLiteConnection.open(this, mConfiguration,
        return SQLiteConnection.open(this, configuration,
                connectionId, primaryConnection); // might throw
    }

@@ -442,6 +451,30 @@ public final class SQLiteConnectionPool implements Closeable {
        mConnectionLeaked.set(true);
    }

    // Can't throw.
    private void closeAvailableConnectionsAndLogExceptionsLocked() {
        final int count = mAvailableNonPrimaryConnections.size();
        for (int i = 0; i < count; i++) {
            closeConnectionAndLogExceptionsLocked(mAvailableNonPrimaryConnections.get(i));
        }
        mAvailableNonPrimaryConnections.clear();

        if (mAvailablePrimaryConnection != null) {
            closeConnectionAndLogExceptionsLocked(mAvailablePrimaryConnection);
            mAvailablePrimaryConnection = null;
        }
    }

    // Can't throw.
    private void closeExcessConnectionsAndLogExceptionsLocked() {
        int availableCount = mAvailableNonPrimaryConnections.size();
        while (availableCount-- > mConfiguration.maxConnectionPoolSize - 1) {
            SQLiteConnection connection =
                    mAvailableNonPrimaryConnections.remove(availableCount);
            closeConnectionAndLogExceptionsLocked(connection);
        }
    }

    // Can't throw.
    private void closeConnectionAndLogExceptionsLocked(SQLiteConnection connection) {
        try {
@@ -452,9 +485,13 @@ public final class SQLiteConnectionPool implements Closeable {
        }
    }

    // Can't throw.
    private void discardAcquiredConnectionsLocked() {
        markAcquiredConnectionsLocked(AcquiredConnectionStatus.DISCARD);
    }

    // Can't throw.
    private void reconfigureAllConnectionsLocked() {
        boolean wake = false;
        if (mAvailablePrimaryConnection != null) {
            try {
                mAvailablePrimaryConnection.reconfigure(mConfiguration); // might throw
@@ -463,7 +500,6 @@ public final class SQLiteConnectionPool implements Closeable {
                        + mAvailablePrimaryConnection, ex);
                closeConnectionAndLogExceptionsLocked(mAvailablePrimaryConnection);
                mAvailablePrimaryConnection = null;
                wake = true;
            }
        }

@@ -478,27 +514,30 @@ public final class SQLiteConnectionPool implements Closeable {
                closeConnectionAndLogExceptionsLocked(connection);
                mAvailableNonPrimaryConnections.remove(i--);
                count -= 1;
                wake = true;
            }
        }

        markAcquiredConnectionsLocked(AcquiredConnectionStatus.RECONFIGURE);
    }

    // Can't throw.
    private void markAcquiredConnectionsLocked(AcquiredConnectionStatus status) {
        if (!mAcquiredConnections.isEmpty()) {
            ArrayList<SQLiteConnection> keysToUpdate = new ArrayList<SQLiteConnection>(
                    mAcquiredConnections.size());
            for (Map.Entry<SQLiteConnection, Boolean> entry : mAcquiredConnections.entrySet()) {
                if (entry.getValue() != Boolean.TRUE) {
            for (Map.Entry<SQLiteConnection, AcquiredConnectionStatus> entry
                    : mAcquiredConnections.entrySet()) {
                AcquiredConnectionStatus oldStatus = entry.getValue();
                if (status != oldStatus
                        && oldStatus != AcquiredConnectionStatus.DISCARD) {
                    keysToUpdate.add(entry.getKey());
                }
            }
            final int updateCount = keysToUpdate.size();
            for (int i = 0; i < updateCount; i++) {
                mAcquiredConnections.put(keysToUpdate.get(i), Boolean.TRUE);
                mAcquiredConnections.put(keysToUpdate.get(i), status);
            }
        }

        if (wake) {
            wakeConnectionWaitersLocked();
        }
    }

    // Might throw.
@@ -658,8 +697,7 @@ public final class SQLiteConnectionPool implements Closeable {
        int activeConnections = 0;
        int idleConnections = 0;
        if (!mAcquiredConnections.isEmpty()) {
            for (Map.Entry<SQLiteConnection, Boolean> entry : mAcquiredConnections.entrySet()) {
                final SQLiteConnection connection = entry.getKey();
            for (SQLiteConnection connection : mAcquiredConnections.keySet()) {
                String description = connection.describeCurrentOperationUnsafe();
                if (description != null) {
                    requests.add(description);
@@ -769,7 +807,8 @@ public final class SQLiteConnectionPool implements Closeable {

        // Uhoh.  No primary connection!  Either this is the first time we asked
        // for it, or maybe it leaked?
        connection = openConnectionLocked(true /*primaryConnection*/); // might throw
        connection = openConnectionLocked(mConfiguration,
                true /*primaryConnection*/); // might throw
        finishAcquireConnectionLocked(connection, connectionFlags); // might throw
        return connection;
    }
@@ -807,7 +846,8 @@ public final class SQLiteConnectionPool implements Closeable {
        if (openConnections >= mConfiguration.maxConnectionPoolSize) {
            return null;
        }
        connection = openConnectionLocked(false /*primaryConnection*/); // might throw
        connection = openConnectionLocked(mConfiguration,
                false /*primaryConnection*/); // might throw
        finishAcquireConnectionLocked(connection, connectionFlags); // might throw
        return connection;
    }
@@ -818,7 +858,7 @@ public final class SQLiteConnectionPool implements Closeable {
            final boolean readOnly = (connectionFlags & CONNECTION_FLAG_READ_ONLY) != 0;
            connection.setOnlyAllowReadOnlyOperations(readOnly);

            mAcquiredConnections.put(connection, Boolean.FALSE);
            mAcquiredConnections.put(connection, AcquiredConnectionStatus.NORMAL);
        } catch (RuntimeException ex) {
            Log.e(TAG, "Failed to prepare acquired connection for session, closing it: "
                    + connection +", connectionFlags=" + connectionFlags);
@@ -858,7 +898,7 @@ public final class SQLiteConnectionPool implements Closeable {
    private void throwIfClosedLocked() {
        if (!mIsOpen) {
            throw new IllegalStateException("Cannot perform this operation "
                    + "because the connection pool have been closed.");
                    + "because the connection pool has been closed.");
        }
    }

@@ -922,11 +962,11 @@ public final class SQLiteConnectionPool implements Closeable {

            printer.println("  Acquired connections:");
            if (!mAcquiredConnections.isEmpty()) {
                for (Map.Entry<SQLiteConnection, Boolean> entry :
                for (Map.Entry<SQLiteConnection, AcquiredConnectionStatus> entry :
                        mAcquiredConnections.entrySet()) {
                    final SQLiteConnection connection = entry.getKey();
                    connection.dumpUnsafe(indentedPrinter, verbose);
                    indentedPrinter.println("  Pending reconfiguration: " + entry.getValue());
                    indentedPrinter.println("  Status: " + entry.getValue());
                }
            } else {
                indentedPrinter.println("<none>");
+32 −22
Original line number Diff line number Diff line
@@ -677,6 +677,38 @@ public class SQLiteDatabase extends SQLiteClosable {
        return openDatabase(path, factory, CREATE_IF_NECESSARY, errorHandler);
    }

    /**
     * Reopens the database in read-write mode.
     * If the database is already read-write, does nothing.
     *
     * @throws SQLiteException if the database could not be reopened as requested, in which
     * case it remains open in read only mode.
     * @throws IllegalStateException if the database is not open.
     *
     * @see #isReadOnly()
     * @hide
     */
    public void reopenReadWrite() {
        synchronized (mLock) {
            throwIfNotOpenLocked();

            if (!isReadOnlyLocked()) {
                return; // nothing to do
            }

            // Reopen the database in read-write mode.
            final int oldOpenFlags = mConfigurationLocked.openFlags;
            mConfigurationLocked.openFlags = (mConfigurationLocked.openFlags & ~OPEN_READ_MASK)
                    | OPEN_READWRITE;
            try {
                mConnectionPoolLocked.reconfigure(mConfigurationLocked);
            } catch (RuntimeException ex) {
                mConfigurationLocked.openFlags = oldOpenFlags;
                throw ex;
            }
        }
    }

    private void open() {
        try {
            try {
@@ -1902,28 +1934,6 @@ public class SQLiteDatabase extends SQLiteClosable {
        return true;
    }

    /**
     * Prevent other threads from using the database's primary connection.
     *
     * This method is only used by {@link SQLiteOpenHelper} when transitioning from
     * a readable to a writable database.  It should not be used in any other way.
     *
     * @see #unlockPrimaryConnection()
     */
    void lockPrimaryConnection() {
        getThreadSession().beginTransaction(SQLiteSession.TRANSACTION_MODE_DEFERRED,
                null, SQLiteConnectionPool.CONNECTION_FLAG_PRIMARY_CONNECTION_AFFINITY, null);
    }

    /**
     * Allow other threads to use the database's primary connection.
     *
     * @see #lockPrimaryConnection()
     */
    void unlockPrimaryConnection() {
        getThreadSession().endTransaction(null);
    }

    @Override
    public String toString() {
        return "SQLiteDatabase: " + getPath();
+8 −8
Original line number Diff line number Diff line
@@ -50,17 +50,17 @@ public final class SQLiteDatabaseConfiguration {
     */
    public final String path;

    /**
     * The flags used to open the database.
     */
    public final int openFlags;

    /**
     * The label to use to describe the database when it appears in logs.
     * This is derived from the path but is stripped to remove PII.
     */
    public final String label;

    /**
     * The flags used to open the database.
     */
    public int openFlags;

    /**
     * The maximum number of connections to retain in the connection pool.
     * Must be at least 1.
@@ -103,8 +103,8 @@ public final class SQLiteDatabaseConfiguration {
        }

        this.path = path;
        this.openFlags = openFlags;
        label = stripPathForLogs(path);
        this.openFlags = openFlags;

        // Set default values for optional parameters.
        maxConnectionPoolSize = 1;
@@ -123,7 +123,6 @@ public final class SQLiteDatabaseConfiguration {
        }

        this.path = other.path;
        this.openFlags = other.openFlags;
        this.label = other.label;
        updateParametersFrom(other);
    }
@@ -138,11 +137,12 @@ public final class SQLiteDatabaseConfiguration {
        if (other == null) {
            throw new IllegalArgumentException("other must not be null.");
        }
        if (!path.equals(other.path) || openFlags != other.openFlags) {
        if (!path.equals(other.path)) {
            throw new IllegalArgumentException("other configuration must refer to "
                    + "the same database.");
        }

        openFlags = other.openFlags;
        maxConnectionPoolSize = other.maxConnectionPoolSize;
        maxSqlCacheSize = other.maxSqlCacheSize;
        locale = other.locale;
+83 −92

File changed.

Preview size limit exceeded, changes collapsed.