Deprecate DatabaseUtils.InsertHelper. This class does not offer any advantages over SQLiteStatement and just makes code more complex and error-prone. Documented that the class is not thread-safe. Removed a potential deadlock in insert() and replace() caused by the insertInternal() method being synchronized in the case where the class was being used concurrently (woe to you!). Thread A would start a transaction. Thread B would call insertInternal() and acquire the object monitor, but block because it could not obtain the db connection because thread A is holding onto it. Thread A would call insertInternal() and block because Thread B was holding the object monitor. Deadlock. Changed this code to use a transaction instead of a lock, which provides the necessary mutual exclusion guarantee without the potential for a deadlock. Even so, the class really isn't thread safe. Bug: 6625094 Change-Id: I51d9a15567a6f2bad6f25e550b48f8f6ffcab2a7
diff --git a/api/16.txt b/api/16.txt index 70c4032..f01768d 100644 --- a/api/16.txt +++ b/api/16.txt
@@ -7321,7 +7321,6 @@ method public void prepareForInsert(); method public void prepareForReplace(); method public long replace(android.content.ContentValues); - field public static final int TABLE_INFO_PRAGMA_DEFAULT_INDEX = 4; // 0x4 } public final class DefaultDatabaseErrorHandler implements android.database.DatabaseErrorHandler {
diff --git a/api/current.txt b/api/current.txt index 89f265d..276a0af 100644 --- a/api/current.txt +++ b/api/current.txt
@@ -7316,7 +7316,7 @@ field public static final int STATEMENT_UPDATE = 2; // 0x2 } - public static class DatabaseUtils.InsertHelper { + public static deprecated class DatabaseUtils.InsertHelper { ctor public DatabaseUtils.InsertHelper(android.database.sqlite.SQLiteDatabase, java.lang.String); method public void bind(int, double); method public void bind(int, float); @@ -7333,7 +7333,6 @@ method public void prepareForInsert(); method public void prepareForReplace(); method public long replace(android.content.ContentValues); - field public static final int TABLE_INFO_PRAGMA_DEFAULT_INDEX = 4; // 0x4 } public final class DefaultDatabaseErrorHandler implements android.database.DatabaseErrorHandler {
diff --git a/core/java/android/database/DatabaseUtils.java b/core/java/android/database/DatabaseUtils.java index a6af5c2..1fc1226 100644 --- a/core/java/android/database/DatabaseUtils.java +++ b/core/java/android/database/DatabaseUtils.java
@@ -50,9 +50,6 @@ private static final String TAG = "DatabaseUtils"; private static final boolean DEBUG = false; - private static final boolean LOCAL_LOGV = false; - - private static final String[] countProjection = new String[]{"count(*)"}; /** One of the values returned by {@link #getSqlStatementType(String)}. */ public static final int STATEMENT_SELECT = 1; @@ -963,10 +960,15 @@ } /** - * This class allows users to do multiple inserts into a table but - * compile the SQL insert statement only once, which may increase - * performance. + * This class allows users to do multiple inserts into a table using + * the same statement. + * <p> + * This class is not thread-safe. + * </p> + * + * @deprecated Use {@link SQLiteStatement} instead. */ + @Deprecated public static class InsertHelper { private final SQLiteDatabase mDb; private final String mTableName; @@ -983,6 +985,13 @@ * table_info(...)" command that we depend on. */ public static final int TABLE_INFO_PRAGMA_COLUMNNAME_INDEX = 1; + + /** + * This field was accidentally exposed in earlier versions of the platform + * so we can hide it but we can't remove it. + * + * @hide + */ public static final int TABLE_INFO_PRAGMA_DEFAULT_INDEX = 4; /** @@ -1036,7 +1045,7 @@ sb.append(sbv); mInsertSQL = sb.toString(); - if (LOCAL_LOGV) Log.v(TAG, "insert statement is " + mInsertSQL); + if (DEBUG) Log.v(TAG, "insert statement is " + mInsertSQL); } private SQLiteStatement getStatement(boolean allowReplace) throws SQLException { @@ -1069,24 +1078,35 @@ * @return the row ID of the newly inserted row, or -1 if an * error occurred */ - private synchronized long insertInternal(ContentValues values, boolean allowReplace) { + private long insertInternal(ContentValues values, boolean allowReplace) { + // Start a transaction even though we don't really need one. + // This is to help maintain compatibility with applications that + // access InsertHelper from multiple threads even though they never should have. + // The original code used to lock the InsertHelper itself which was prone + // to deadlocks. Starting a transaction achieves the same mutual exclusion + // effect as grabbing a lock but without the potential for deadlocks. + mDb.beginTransactionNonExclusive(); try { SQLiteStatement stmt = getStatement(allowReplace); stmt.clearBindings(); - if (LOCAL_LOGV) Log.v(TAG, "--- inserting in table " + mTableName); + if (DEBUG) Log.v(TAG, "--- inserting in table " + mTableName); for (Map.Entry<String, Object> e: values.valueSet()) { final String key = e.getKey(); int i = getColumnIndex(key); DatabaseUtils.bindObjectToProgram(stmt, i, e.getValue()); - if (LOCAL_LOGV) { + if (DEBUG) { Log.v(TAG, "binding " + e.getValue() + " to column " + i + " (" + key + ")"); } } - return stmt.executeInsert(); + long result = stmt.executeInsert(); + mDb.setTransactionSuccessful(); + return result; } catch (SQLException e) { Log.e(TAG, "Error inserting " + values + " into table " + mTableName, e); return -1; + } finally { + mDb.endTransaction(); } } @@ -1223,7 +1243,7 @@ + "execute"); } try { - if (LOCAL_LOGV) Log.v(TAG, "--- doing insert or replace in table " + mTableName); + if (DEBUG) Log.v(TAG, "--- doing insert or replace in table " + mTableName); return mPreparedStatement.executeInsert(); } catch (SQLException e) { Log.e(TAG, "Error executing InsertHelper with table " + mTableName, e);