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

4 files changed, 236 insertions, 195 deletions

diff --git a/core/java/android/database/sqlite/SQLiteConnectionPool.java b/core/java/android/database/sqlite/SQLiteConnectionPool.java
index 236948e..3562e89 100644
--- a/core/java/android/database/sqlite/SQLiteConnectionPool.java
+++ b/core/java/android/database/sqlite/SQLiteConnectionPool.java
@@ -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();
 
-            reconfigureAllConnectionsLocked();
+                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()) {
-                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) {
+                if (recycleConnectionLocked(connection, status)) {
+                    assert mAvailablePrimaryConnection == null;
                     mAvailablePrimaryConnection = connection;
                 }
                 wakeConnectionWaitersLocked();
@@ -339,17 +338,7 @@ public final class SQLiteConnectionPool implements Closeable {
                     mConfiguration.maxConnectionPoolSize - 1) {
                 closeConnectionAndLogExceptionsLocked(connection);
             } else {
-                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;
-                }
-                if (connection != null) {
+                if (recycleConnectionLocked(connection, status)) {
                     mAvailableNonPrimaryConnections.add(connection);
                 }
                 wakeConnectionWaitersLocked();
@@ -357,6 +346,25 @@ public final class SQLiteConnectionPool implements Closeable {
         }
     }
 
+    // Can't throw.
+    private boolean recycleConnectionLocked(SQLiteConnection connection,
+            AcquiredConnectionStatus status) {
+        if (status == AcquiredConnectionStatus.RECONFIGURE) {
+            try {
+                connection.reconfigure(mConfiguration); // might throw
+            } catch (RuntimeException ex) {
+                Log.e(TAG, "Failed to reconfigure released connection, closing it: "
+                        + connection, ex);
+                status = AcquiredConnectionStatus.DISCARD;
+            }
+        }
+        if (status == AcquiredConnectionStatus.DISCARD) {
+            closeConnectionAndLogExceptionsLocked(connection);
+            return false;
+        }
+        return true;
+    }
+
     /**
      * Returns true if the session should yield the connection due to
      * contention over available database connections.
@@ -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
     }
 
@@ -443,6 +452,30 @@ public final class SQLiteConnectionPool implements Closeable {
     }
 
     // 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 {
             connection.close(); // might throw
@@ -453,8 +486,12 @@ 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>");
diff --git a/core/java/android/database/sqlite/SQLiteDatabase.java b/core/java/android/database/sqlite/SQLiteDatabase.java
index 505f83e..36f678d 100644
--- a/core/java/android/database/sqlite/SQLiteDatabase.java
+++ b/core/java/android/database/sqlite/SQLiteDatabase.java
@@ -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();
diff --git a/core/java/android/database/sqlite/SQLiteDatabaseConfiguration.java b/core/java/android/database/sqlite/SQLiteDatabaseConfiguration.java
index bc79ad3..02ef671 100644
--- a/core/java/android/database/sqlite/SQLiteDatabaseConfiguration.java
+++ b/core/java/android/database/sqlite/SQLiteDatabaseConfiguration.java
@@ -51,17 +51,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;
diff --git a/core/java/android/database/sqlite/SQLiteOpenHelper.java b/core/java/android/database/sqlite/SQLiteOpenHelper.java
index 46d9369..ffa4663 100644
--- a/core/java/android/database/sqlite/SQLiteOpenHelper.java
+++ b/core/java/android/database/sqlite/SQLiteOpenHelper.java
@@ -43,13 +43,21 @@ import android.util.Log;
 public abstract class SQLiteOpenHelper {
     private static final String TAG = SQLiteOpenHelper.class.getSimpleName();
 
+    // When true, getReadableDatabase returns a read-only database if it is just being opened.
+    // The database handle is reopened in read/write mode when getWritableDatabase is called.
+    // We leave this behavior disabled in production because it is inefficient and breaks
+    // many applications.  For debugging purposes it can be useful to turn on strict
+    // read-only semantics to catch applications that call getReadableDatabase when they really
+    // wanted getWritableDatabase.
+    private static final boolean DEBUG_STRICT_READONLY = false;
+
     private final Context mContext;
     private final String mName;
     private final CursorFactory mFactory;
     private final int mNewVersion;
 
-    private SQLiteDatabase mDatabase = null;
-    private boolean mIsInitializing = false;
+    private SQLiteDatabase mDatabase;
+    private boolean mIsInitializing;
     private final DatabaseErrorHandler mErrorHandler;
 
     /**
@@ -127,41 +135,89 @@ public abstract class SQLiteOpenHelper {
      * @throws SQLiteException if the database cannot be opened for writing
      * @return a read/write database object valid until {@link #close} is called
      */
-    public synchronized SQLiteDatabase getWritableDatabase() {
+    public SQLiteDatabase getWritableDatabase() {
+        synchronized (this) {
+            return getDatabaseLocked(true);
+        }
+    }
+
+    /**
+     * Create and/or open a database.  This will be the same object returned by
+     * {@link #getWritableDatabase} unless some problem, such as a full disk,
+     * requires the database to be opened read-only.  In that case, a read-only
+     * database object will be returned.  If the problem is fixed, a future call
+     * to {@link #getWritableDatabase} may succeed, in which case the read-only
+     * database object will be closed and the read/write object will be returned
+     * in the future.
+     *
+     * <p class="caution">Like {@link #getWritableDatabase}, this method may
+     * take a long time to return, so you should not call it from the
+     * application main thread, including from
+     * {@link android.content.ContentProvider#onCreate ContentProvider.onCreate()}.
+     *
+     * @throws SQLiteException if the database cannot be opened
+     * @return a database object valid until {@link #getWritableDatabase}
+     *     or {@link #close} is called.
+     */
+    public SQLiteDatabase getReadableDatabase() {
+        synchronized (this) {
+            return getDatabaseLocked(false);
+        }
+    }
+
+    private SQLiteDatabase getDatabaseLocked(boolean writable) {
         if (mDatabase != null) {
             if (!mDatabase.isOpen()) {
-                // darn! the user closed the database by calling mDatabase.close()
+                // Darn!  The user closed the database by calling mDatabase.close().
                 mDatabase = null;
-            } else if (!mDatabase.isReadOnly()) {
-                return mDatabase;  // The database is already open for business
+            } else if (!writable || !mDatabase.isReadOnly()) {
+                // The database is already open for business.
+                return mDatabase;
             }
         }
 
         if (mIsInitializing) {
-            throw new IllegalStateException("getWritableDatabase called recursively");
+            throw new IllegalStateException("getDatabase called recursively");
         }
 
-        // If we have a read-only database open, someone could be using it
-        // (though they shouldn't), which would cause a lock to be held on
-        // the file, and our attempts to open the database read-write would
-        // fail waiting for the file lock.  To prevent that, we acquire a lock
-        // on the read-only database, which shuts out other users.
-
-        boolean success = false;
-        SQLiteDatabase db = null;
-        if (mDatabase != null) {
-            mDatabase.lockPrimaryConnection();
-        }
+        SQLiteDatabase db = mDatabase;
         try {
             mIsInitializing = true;
-            if (mName == null) {
+
+            if (db != null) {
+                if (writable && db.isReadOnly()) {
+                    db.reopenReadWrite();
+                }
+            } else if (mName == null) {
                 db = SQLiteDatabase.create(null);
             } else {
-                db = mContext.openOrCreateDatabase(mName, 0, mFactory, mErrorHandler);
+                try {
+                    if (DEBUG_STRICT_READONLY && !writable) {
+                        final String path = mContext.getDatabasePath(mName).getPath();
+                        db = SQLiteDatabase.openDatabase(path, mFactory,
+                                SQLiteDatabase.OPEN_READONLY, mErrorHandler);
+                    } else {
+                        db = mContext.openOrCreateDatabase(mName, 0, mFactory, mErrorHandler);
+                    }
+                } catch (SQLiteException ex) {
+                    if (writable) {
+                        throw ex;
+                    }
+                    Log.e(TAG, "Couldn't open " + mName
+                            + " for writing (will try read-only):", ex);
+                    final String path = mContext.getDatabasePath(mName).getPath();
+                    db = SQLiteDatabase.openDatabase(path, mFactory,
+                            SQLiteDatabase.OPEN_READONLY, mErrorHandler);
+                }
             }
 
-            int version = db.getVersion();
+            final int version = db.getVersion();
             if (version != mNewVersion) {
+                if (db.isReadOnly()) {
+                    throw new SQLiteException("Can't upgrade read-only database from version " +
+                            db.getVersion() + " to " + mNewVersion + ": " + mName);
+                }
+
                 db.beginTransaction();
                 try {
                     if (version == 0) {
@@ -179,84 +235,19 @@ public abstract class SQLiteOpenHelper {
                     db.endTransaction();
                 }
             }
-
             onOpen(db);
-            success = true;
-            return db;
-        } finally {
-            mIsInitializing = false;
-            if (success) {
-                if (mDatabase != null) {
-                    try { mDatabase.close(); } catch (Exception e) { }
-                    mDatabase.unlockPrimaryConnection();
-                }
-                mDatabase = db;
-            } else {
-                if (mDatabase != null) {
-                    mDatabase.unlockPrimaryConnection();
-                }
-                if (db != null) db.close();
-            }
-        }
-    }
 
-    /**
-     * Create and/or open a database.  This will be the same object returned by
-     * {@link #getWritableDatabase} unless some problem, such as a full disk,
-     * requires the database to be opened read-only.  In that case, a read-only
-     * database object will be returned.  If the problem is fixed, a future call
-     * to {@link #getWritableDatabase} may succeed, in which case the read-only
-     * database object will be closed and the read/write object will be returned
-     * in the future.
-     *
-     * <p class="caution">Like {@link #getWritableDatabase}, this method may
-     * take a long time to return, so you should not call it from the
-     * application main thread, including from
-     * {@link android.content.ContentProvider#onCreate ContentProvider.onCreate()}.
-     *
-     * @throws SQLiteException if the database cannot be opened
-     * @return a database object valid until {@link #getWritableDatabase}
-     *     or {@link #close} is called.
-     */
-    public synchronized SQLiteDatabase getReadableDatabase() {
-        if (mDatabase != null) {
-            if (!mDatabase.isOpen()) {
-                // darn! the user closed the database by calling mDatabase.close()
-                mDatabase = null;
-            } else {
-                return mDatabase;  // The database is already open for business
-            }
-        }
-
-        if (mIsInitializing) {
-            throw new IllegalStateException("getReadableDatabase called recursively");
-        }
-
-        try {
-            return getWritableDatabase();
-        } catch (SQLiteException e) {
-            if (mName == null) throw e;  // Can't open a temp database read-only!
-            Log.e(TAG, "Couldn't open " + mName + " for writing (will try read-only):", e);
-        }
-
-        SQLiteDatabase db = null;
-        try {
-            mIsInitializing = true;
-            String path = mContext.getDatabasePath(mName).getPath();
-            db = SQLiteDatabase.openDatabase(path, mFactory, SQLiteDatabase.OPEN_READONLY,
-                    mErrorHandler);
-            if (db.getVersion() != mNewVersion) {
-                throw new SQLiteException("Can't upgrade read-only database from version " +
-                        db.getVersion() + " to " + mNewVersion + ": " + path);
+            if (db.isReadOnly()) {
+                Log.w(TAG, "Opened " + mName + " in read-only mode");
             }
 
-            onOpen(db);
-            Log.w(TAG, "Opened " + mName + " in read-only mode");
             mDatabase = db;
-            return mDatabase;
+            return db;
         } finally {
             mIsInitializing = false;
-            if (db != null && db != mDatabase) db.close();
+            if (db != null && db != mDatabase) {
+                db.close();
+            }
         }
     }