Merge changes from topic "mar31" into rvc-dev
* changes:
Introduce database schema locking.
Progress towards database schema locking.
diff --git a/legacy/src/com/android/providers/media/LegacyMediaProvider.java b/legacy/src/com/android/providers/media/LegacyMediaProvider.java
index 086f88c..a505ee1 100644
--- a/legacy/src/com/android/providers/media/LegacyMediaProvider.java
+++ b/legacy/src/com/android/providers/media/LegacyMediaProvider.java
@@ -94,8 +94,10 @@
public Cursor query(Uri uri, String[] projection, String selection, String[] selectionArgs,
String sortOrder) {
final DatabaseHelper helper = getDatabaseForUri(uri);
- return helper.getReadableDatabase().query("files", projection, selection, selectionArgs,
- null, null, sortOrder);
+ return helper.runWithoutTransaction((db) -> {
+ return db.query("files", projection, selection, selectionArgs,
+ null, null, sortOrder);
+ });
}
@Override
@@ -114,7 +116,9 @@
}
final DatabaseHelper helper = getDatabaseForUri(uri);
- final long id = helper.getWritableDatabase().insert("files", null, values);
+ final long id = helper.runWithTransaction((db) -> {
+ return db.insert("files", null, values);
+ });
return ContentUris.withAppendedId(uri, id);
}
diff --git a/src/com/android/providers/media/DatabaseHelper.java b/src/com/android/providers/media/DatabaseHelper.java
index e49052d..3913065 100644
--- a/src/com/android/providers/media/DatabaseHelper.java
+++ b/src/com/android/providers/media/DatabaseHelper.java
@@ -35,7 +35,6 @@
import android.net.Uri;
import android.os.Bundle;
import android.os.Environment;
-import android.os.Looper;
import android.os.SystemClock;
import android.os.Trace;
import android.provider.MediaStore;
@@ -75,7 +74,8 @@
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
-import java.util.function.LongSupplier;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Function;
import java.util.function.UnaryOperator;
import java.util.regex.Matcher;
@@ -110,8 +110,19 @@
final Set<String> mFilterVolumeNames = new ArraySet<>();
long mScanStartTime;
long mScanStopTime;
- /** Flag indicating if a schema change is in progress */
- boolean mSchemaChanging;
+
+ /**
+ * Lock used to guard against deadlocks in SQLite; the write lock is used to
+ * guard any schema changes, and the read lock is used for all other
+ * database operations.
+ * <p>
+ * As a concrete example: consider the case where the primary database
+ * connection is performing a schema change inside a transaction, while a
+ * secondary connection is waiting to begin a transaction. When the primary
+ * database connection changes the schema, it attempts to close all other
+ * database connections, which then deadlocks.
+ */
+ private final ReentrantReadWriteLock mSchemaLock = new ReentrantReadWriteLock();
public interface OnSchemaChangeListener {
public void onSchemaChange(@NonNull String volumeName, int versionFrom, int versionTo,
@@ -195,26 +206,33 @@
mFilterVolumeNames.addAll(filterVolumeNames);
}
- // We might be tempted to open a transaction and recreate views here,
- // but that would result in an obscure deadlock; instead we simply close
- // the entire database, letting the views be recreated the next time
- // it's opened.
- close();
+ // Recreate all views to apply this filter
+ final SQLiteDatabase db = super.getWritableDatabase();
+ mSchemaLock.writeLock().lock();
+ try {
+ db.beginTransaction();
+ createLatestViews(db, mInternal);
+ db.setTransactionSuccessful();
+ } finally {
+ db.endTransaction();
+ mSchemaLock.writeLock().unlock();
+ }
}
@Override
public SQLiteDatabase getReadableDatabase() {
- if (Looper.myLooper() == Looper.getMainLooper()) {
- Log.wtf(TAG, "Database operations must not happen on main thread", new Throwable());
- }
- return super.getReadableDatabase();
+ throw new UnsupportedOperationException("All database operations must be routed through"
+ + " runWithTransaction() or runWithoutTransaction() to avoid deadlocks");
}
@Override
public SQLiteDatabase getWritableDatabase() {
- if (Looper.myLooper() == Looper.getMainLooper()) {
- Log.wtf(TAG, "Database operations must not happen on main thread", new Throwable());
- }
+ throw new UnsupportedOperationException("All database operations must be routed through"
+ + " runWithTransaction() or runWithoutTransaction() to avoid deadlocks");
+ }
+
+ @VisibleForTesting
+ SQLiteDatabase getWritableDatabaseForTest() {
return super.getWritableDatabase();
}
@@ -222,7 +240,8 @@
public void onConfigure(SQLiteDatabase db) {
Log.v(TAG, "onConfigure() for " + mName);
db.setCustomScalarFunction("_INSERT", (arg) -> {
- if (arg != null && mFilesListener != null && !mSchemaChanging) {
+ if (arg != null && mFilesListener != null
+ && !mSchemaLock.isWriteLockedByCurrentThread()) {
final String[] split = arg.split(":");
final String volumeName = split[0];
final long id = Long.parseLong(split[1]);
@@ -240,7 +259,8 @@
return null;
});
db.setCustomScalarFunction("_UPDATE", (arg) -> {
- if (arg != null && mFilesListener != null && !mSchemaChanging) {
+ if (arg != null && mFilesListener != null
+ && !mSchemaLock.isWriteLockedByCurrentThread()) {
final String[] split = arg.split(":");
final String volumeName = split[0];
final long oldId = Long.parseLong(split[1]);
@@ -265,7 +285,8 @@
return null;
});
db.setCustomScalarFunction("_DELETE", (arg) -> {
- if (arg != null && mFilesListener != null && !mSchemaChanging) {
+ if (arg != null && mFilesListener != null
+ && !mSchemaLock.isWriteLockedByCurrentThread()) {
final String[] split = arg.split(":");
final String volumeName = split[0];
final long id = Long.parseLong(split[1]);
@@ -286,7 +307,7 @@
return null;
});
db.setCustomScalarFunction("_GET_ID", (arg) -> {
- if (mIdGenerator != null && !mSchemaChanging) {
+ if (mIdGenerator != null && !mSchemaLock.isWriteLockedByCurrentThread()) {
Trace.beginSection("_GET_ID");
try {
return mIdGenerator.apply(arg);
@@ -299,43 +320,35 @@
}
@Override
- public void onOpen(SQLiteDatabase db) {
- // Always recreate latest views and triggers during open; they're
- // cheap and it's an easy way to ensure they're defined consistently
- createLatestViews(db, mInternal);
- createLatestTriggers(db, mInternal);
- }
-
- @Override
public void onCreate(final SQLiteDatabase db) {
Log.v(TAG, "onCreate() for " + mName);
- mSchemaChanging = true;
+ mSchemaLock.writeLock().lock();
try {
updateDatabase(db, 0, mVersion);
} finally {
- mSchemaChanging = false;
+ mSchemaLock.writeLock().unlock();
}
}
@Override
public void onUpgrade(final SQLiteDatabase db, final int oldV, final int newV) {
Log.v(TAG, "onUpgrade() for " + mName + " from " + oldV + " to " + newV);
- mSchemaChanging = true;
+ mSchemaLock.writeLock().lock();
try {
updateDatabase(db, oldV, newV);
} finally {
- mSchemaChanging = false;
+ mSchemaLock.writeLock().unlock();
}
}
@Override
public void onDowngrade(final SQLiteDatabase db, final int oldV, final int newV) {
Log.v(TAG, "onDowngrade() for " + mName + " from " + oldV + " to " + newV);
- mSchemaChanging = true;
+ mSchemaLock.writeLock().lock();
try {
downgradeDatabase(db, oldV, newV);
} finally {
- mSchemaChanging = false;
+ mSchemaLock.writeLock().unlock();
}
}
@@ -411,7 +424,8 @@
}
mTransactionState.set(new TransactionState());
- final SQLiteDatabase db = getWritableDatabase();
+ final SQLiteDatabase db = super.getWritableDatabase();
+ mSchemaLock.readLock().lock();
db.beginTransaction();
db.execSQL("UPDATE local_metadata SET generation=generation+1;");
}
@@ -423,7 +437,7 @@
}
state.successful = true;
- final SQLiteDatabase db = getWritableDatabase();
+ final SQLiteDatabase db = super.getWritableDatabase();
db.setTransactionSuccessful();
}
@@ -434,8 +448,9 @@
}
mTransactionState.remove();
- final SQLiteDatabase db = getWritableDatabase();
+ final SQLiteDatabase db = super.getWritableDatabase();
db.endTransaction();
+ mSchemaLock.readLock().unlock();
if (state.successful) {
// We carefully "phase" our two sets of work here to ensure that we
@@ -458,19 +473,23 @@
}
/**
- * Execute the given runnable inside a transaction. If the calling thread is
- * not already in an active transaction, this method will wrap the given
+ * Execute the given operation inside a transaction. If the calling thread
+ * is not already in an active transaction, this method will wrap the given
* runnable inside a new transaction.
*/
- public long runWithTransaction(@NonNull LongSupplier s) {
+ public @NonNull <T> T runWithTransaction(@NonNull Function<SQLiteDatabase, T> op) {
+ // We carefully acquire the database here so that any schema changes can
+ // be applied before acquiring the read lock below
+ final SQLiteDatabase db = super.getWritableDatabase();
+
if (mTransactionState.get() != null) {
// Already inside a transaction, so we can run directly
- return s.getAsLong();
+ return op.apply(db);
} else {
// Not inside a transaction, so we need to make one
beginTransaction();
try {
- final long res = s.getAsLong();
+ final T res = op.apply(db);
setTransactionSuccessful();
return res;
} finally {
@@ -479,6 +498,29 @@
}
}
+ /**
+ * Execute the given operation regardless of the calling thread being in an
+ * active transaction or not.
+ */
+ public @NonNull <T> T runWithoutTransaction(@NonNull Function<SQLiteDatabase, T> op) {
+ // We carefully acquire the database here so that any schema changes can
+ // be applied before acquiring the read lock below
+ final SQLiteDatabase db = super.getWritableDatabase();
+
+ if (mTransactionState.get() != null) {
+ // Already inside a transaction, so we can run directly
+ return op.apply(db);
+ } else {
+ // We still need to acquire a schema read lock
+ mSchemaLock.readLock().lock();
+ try {
+ return op.apply(db);
+ } finally {
+ mSchemaLock.readLock().unlock();
+ }
+ }
+ }
+
public void notifyInsert(@NonNull Uri uri) {
notifyChange(uri, ContentResolver.NOTIFY_INSERT);
}
@@ -1331,6 +1373,11 @@
}
}
+ // Always recreate latest views and triggers during upgrade; they're
+ // cheap and it's an easy way to ensure they're defined consistently
+ createLatestViews(db, internal);
+ createLatestTriggers(db, internal);
+
getOrCreateUuid(db);
final long elapsedMillis = (SystemClock.elapsedRealtime() - startTime);
@@ -1383,8 +1430,8 @@
* {@link MediaColumns#GENERATION_ADDED} or
* {@link MediaColumns#GENERATION_MODIFIED}.
*/
- public long getGeneration() {
- return android.database.DatabaseUtils.longForQuery(getReadableDatabase(),
+ public static long getGeneration(@NonNull SQLiteDatabase db) {
+ return android.database.DatabaseUtils.longForQuery(db,
CURRENT_GENERATION_CLAUSE + ";", null);
}
@@ -1392,15 +1439,7 @@
* Return total number of items tracked inside this database. This includes
* only real media items, and does not include directories.
*/
- public long getItemCount() {
- return getItemCount(getReadableDatabase());
- }
-
- /**
- * Return total number of items tracked inside this database. This includes
- * only real media items, and does not include directories.
- */
- private long getItemCount(SQLiteDatabase db) {
+ public static long getItemCount(@NonNull SQLiteDatabase db) {
return android.database.DatabaseUtils.longForQuery(db,
"SELECT COUNT(_id) FROM files WHERE " + FileColumns.MIME_TYPE + " IS NOT NULL",
null);
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 77497f4..04826c9 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -125,7 +125,6 @@
import android.os.Trace;
import android.os.UserHandle;
import android.os.storage.StorageManager;
-import android.os.storage.StorageManager.StorageVolumeCallback;
import android.os.storage.StorageVolume;
import android.preference.PreferenceManager;
import android.provider.BaseColumns;
@@ -686,7 +685,7 @@
* devices. We only do this once per volume so we don't annoy the user if
* deleted manually.
*/
- private void ensureDefaultFolders(String volumeName, DatabaseHelper helper) {
+ private void ensureDefaultFolders(@NonNull String volumeName, @NonNull SQLiteDatabase db) {
try {
final File path = getVolumePath(volumeName);
final StorageVolume vol = mStorageManager.getStorageVolume(path);
@@ -709,7 +708,7 @@
final File folder = new File(vol.getDirectory(), folderName);
if (!folder.exists()) {
folder.mkdirs();
- insertDirectory(helper, folder.getAbsolutePath());
+ insertDirectory(db, folder.getAbsolutePath());
}
}
@@ -728,9 +727,8 @@
* {@link DatabaseHelper#getOrCreateUuid} doesn't match the UUID found on
* disk, then all thumbnails will be considered stable and will be deleted.
*/
- private void ensureThumbnailsValid(String volumeName, DatabaseHelper helper) {
- final String uuidFromDatabase = DatabaseHelper
- .getOrCreateUuid(helper.getReadableDatabase());
+ private void ensureThumbnailsValid(@NonNull String volumeName, @NonNull SQLiteDatabase db) {
+ final String uuidFromDatabase = DatabaseHelper.getOrCreateUuid(db);
try {
for (File dir : getThumbnailDirectories(volumeName)) {
if (!dir.exists()) {
@@ -891,9 +889,6 @@
// Trim any stale log files before we emit new events below
Logging.trimPersistent();
- final DatabaseHelper helper = mExternalDatabase;
- final SQLiteDatabase db = helper.getReadableDatabase();
-
// Scan all volumes to resolve any staleness
for (String volumeName : getExternalVolumeNames()) {
// Possibly bail before digging into each volume
@@ -906,81 +901,100 @@
}
// Ensure that our thumbnails are valid
- ensureThumbnailsValid(volumeName, helper);
+ mExternalDatabase.runWithTransaction((db) -> {
+ ensureThumbnailsValid(volumeName, db);
+ return null;
+ });
}
// Delete any stale thumbnails
- final int staleThumbnails = pruneThumbnails(signal);
+ final int staleThumbnails = mExternalDatabase.runWithTransaction((db) -> {
+ return pruneThumbnails(db, signal);
+ });
Log.d(TAG, "Pruned " + staleThumbnails + " unknown thumbnails");
// Finished orphaning any content whose package no longer exists
- final ArraySet<String> unknownPackages = new ArraySet<>();
- try (Cursor c = db.query(true, "files", new String[] { "owner_package_name" },
- null, null, null, null, null, null, signal)) {
- while (c.moveToNext()) {
- final String packageName = c.getString(0);
- if (TextUtils.isEmpty(packageName)) continue;
+ final int stalePackages = mExternalDatabase.runWithTransaction((db) -> {
+ final ArraySet<String> unknownPackages = new ArraySet<>();
+ try (Cursor c = db.query(true, "files", new String[] { "owner_package_name" },
+ null, null, null, null, null, null, signal)) {
+ while (c.moveToNext()) {
+ final String packageName = c.getString(0);
+ if (TextUtils.isEmpty(packageName)) continue;
- if (!isPackageKnown(packageName)) {
- unknownPackages.add(packageName);
+ if (!isPackageKnown(packageName)) {
+ unknownPackages.add(packageName);
+ }
}
}
- }
-
- for (String packageName : unknownPackages) {
- onPackageOrphaned(packageName);
- }
- final int stalePackages = unknownPackages.size();
+ for (String packageName : unknownPackages) {
+ onPackageOrphaned(db, packageName);
+ }
+ return unknownPackages.size();
+ });
Log.d(TAG, "Pruned " + stalePackages + " unknown packages");
// Delete any expired content; we're paranoid about wildly changing
// clocks, so only delete items within the last week
final long from = ((System.currentTimeMillis() - DateUtils.WEEK_IN_MILLIS) / 1000);
final long to = (System.currentTimeMillis() / 1000);
- final int expiredMedia;
- try (Cursor c = db.query(true, "files", new String[] { "volume_name", "_id" },
- FileColumns.DATE_EXPIRES + " BETWEEN " + from + " AND " + to, null,
- null, null, null, null, signal)) {
- while (c.moveToNext()) {
- final String volumeName = c.getString(0);
- final long id = c.getLong(1);
- delete(Files.getContentUri(volumeName, id), null, null);
+ final int expiredMedia = mExternalDatabase.runWithTransaction((db) -> {
+ try (Cursor c = db.query(true, "files", new String[] { "volume_name", "_id" },
+ FileColumns.DATE_EXPIRES + " BETWEEN " + from + " AND " + to, null,
+ null, null, null, null, signal)) {
+ while (c.moveToNext()) {
+ final String volumeName = c.getString(0);
+ final long id = c.getLong(1);
+ delete(Files.getContentUri(volumeName, id), null, null);
+ }
+ return c.getCount();
}
- expiredMedia = c.getCount();
- Log.d(TAG, "Deleted " + expiredMedia + " expired items on " + helper.mName);
- }
+ });
+ Log.d(TAG, "Deleted " + expiredMedia + " expired items");
// Forget any stale volumes
- final Set<String> recentVolumeNames = MediaStore.getRecentExternalVolumeNames(getContext());
- final Set<String> knownVolumeNames = new ArraySet<>();
- try (Cursor c = db.query(true, "files", new String[] { MediaColumns.VOLUME_NAME },
- null, null, null, null, null, null, signal)) {
- while (c.moveToNext()) {
- knownVolumeNames.add(c.getString(0));
+ mExternalDatabase.runWithTransaction((db) -> {
+ final Set<String> recentVolumeNames = MediaStore
+ .getRecentExternalVolumeNames(getContext());
+ final Set<String> knownVolumeNames = new ArraySet<>();
+ try (Cursor c = db.query(true, "files", new String[] { MediaColumns.VOLUME_NAME },
+ null, null, null, null, null, null, signal)) {
+ while (c.moveToNext()) {
+ knownVolumeNames.add(c.getString(0));
+ }
}
- }
- final Set<String> staleVolumeNames = new ArraySet<>();
- staleVolumeNames.addAll(knownVolumeNames);
- staleVolumeNames.removeAll(recentVolumeNames);
- for (String staleVolumeName : staleVolumeNames) {
- final int num = db.delete("files", FileColumns.VOLUME_NAME + "=?",
- new String[] { staleVolumeName });
- Log.d(TAG, "Forgot " + num + " stale items from " + staleVolumeName);
- }
+ final Set<String> staleVolumeNames = new ArraySet<>();
+ staleVolumeNames.addAll(knownVolumeNames);
+ staleVolumeNames.removeAll(recentVolumeNames);
+ for (String staleVolumeName : staleVolumeNames) {
+ final int num = db.delete("files", FileColumns.VOLUME_NAME + "=?",
+ new String[] { staleVolumeName });
+ Log.d(TAG, "Forgot " + num + " stale items from " + staleVolumeName);
+ }
+ return null;
+ });
synchronized (mDirectoryCache) {
mDirectoryCache.clear();
}
+ final long itemCount = mExternalDatabase.runWithTransaction((db) -> {
+ return DatabaseHelper.getItemCount(db);
+ });
+
final long durationMillis = (SystemClock.elapsedRealtime() - startTime);
- Metrics.logIdleMaintenance(MediaStore.VOLUME_EXTERNAL, helper.getItemCount(),
+ Metrics.logIdleMaintenance(MediaStore.VOLUME_EXTERNAL, itemCount,
durationMillis, staleThumbnails, expiredMedia);
}
public void onPackageOrphaned(String packageName) {
- final DatabaseHelper helper = mExternalDatabase;
- final SQLiteDatabase db = helper.getWritableDatabase();
+ mExternalDatabase.runWithTransaction((db) -> {
+ onPackageOrphaned(db, packageName);
+ return null;
+ });
+ }
+ public void onPackageOrphaned(@NonNull SQLiteDatabase db, @NonNull String packageName) {
final ContentValues values = new ContentValues();
values.putNull(FileColumns.OWNER_PACKAGE_NAME);
@@ -988,7 +1002,7 @@
"owner_package_name=?", new String[] { packageName });
if (count > 0) {
Log.d(TAG, "Orphaned " + count + " items belonging to "
- + packageName + " on " + helper.mName);
+ + packageName + " on " + db.getPath());
}
}
@@ -2328,12 +2342,12 @@
}
}
- private long insertDirectory(DatabaseHelper helper, String path) {
+ private long insertDirectory(@NonNull SQLiteDatabase db, @NonNull String path) {
if (LOGV) Log.v(TAG, "inserting directory " + path);
ContentValues values = new ContentValues();
values.put(FileColumns.FORMAT, MtpConstants.FORMAT_ASSOCIATION);
values.put(FileColumns.DATA, path);
- values.put(FileColumns.PARENT, getParent(helper, path));
+ values.put(FileColumns.PARENT, getParent(db, path));
values.put(FileColumns.OWNER_PACKAGE_NAME, extractPathOwnerPackageName(path));
values.put(FileColumns.VOLUME_NAME, extractVolumeName(path));
values.put(FileColumns.RELATIVE_PATH, extractRelativePath(path));
@@ -2343,12 +2357,10 @@
if (file.exists()) {
values.put(FileColumns.DATE_MODIFIED, file.lastModified() / 1000);
}
- final SQLiteDatabase db = helper.getWritableDatabase();
- long rowId = db.insert("files", FileColumns.DATE_MODIFIED, values);
- return rowId;
+ return db.insert("files", FileColumns.DATE_MODIFIED, values);
}
- private long getParent(DatabaseHelper helper, String path) {
+ private long getParent(@NonNull SQLiteDatabase db, @NonNull String path) {
final String parentPath = new File(path).getParent();
if (Objects.equals("/", parentPath)) {
return -1;
@@ -2361,13 +2373,12 @@
}
final long id;
- final SQLiteDatabase db = helper.getReadableDatabase();
try (Cursor c = db.query("files", new String[] { FileColumns._ID },
FileColumns.DATA + "=?", new String[] { parentPath }, null, null, null)) {
if (c.moveToFirst()) {
id = c.getLong(0);
} else {
- id = insertDirectory(helper, parentPath);
+ id = insertDirectory(db, parentPath);
}
}
@@ -2487,13 +2498,13 @@
}
public void onLocaleChanged() {
- localizeTitles();
+ mInternalDatabase.runWithTransaction((db) -> {
+ localizeTitles(db);
+ return null;
+ });
}
- private void localizeTitles() {
- final DatabaseHelper helper = mInternalDatabase;
- final SQLiteDatabase db = helper.getWritableDatabase();
-
+ private void localizeTitles(@NonNull SQLiteDatabase db) {
try (Cursor c = db.query("files", new String[]{"_id", "title_resource_uri"},
"title_resource_uri IS NOT NULL", null, null, null, null)) {
while (c.moveToNext()) {
@@ -2615,7 +2626,9 @@
Long parent = values.getAsLong(FileColumns.PARENT);
if (parent == null) {
if (path != null) {
- long parentId = getParent(helper, path);
+ final long parentId = helper.runWithTransaction((db) -> {
+ return getParent(db, path);
+ });
values.put(FileColumns.PARENT, parentId);
}
}
@@ -2638,7 +2651,7 @@
private long insertAllowingUpsert(@NonNull SQLiteQueryBuilder qb,
@NonNull DatabaseHelper helper, @NonNull ContentValues values, String path)
throws SQLiteConstraintException {
- return helper.runWithTransaction(() -> {
+ return helper.runWithTransaction((db) -> {
try {
return qb.insert(helper, values);
} catch (SQLiteConstraintException e) {
@@ -3815,16 +3828,19 @@
// Update any playlists that reference this item
if ((mediaType == FileColumns.MEDIA_TYPE_AUDIO)
&& helper.isExternal()) {
- final SQLiteDatabase db = helper.getReadableDatabase();
- try (Cursor cc = db.query("audio_playlists_map",
- new String[] { "playlist_id" }, "audio_id=" + id,
- null, "playlist_id", null, null)) {
- while (cc.moveToNext()) {
- final Uri playlistUri = ContentUris.withAppendedId(
- Playlists.getContentUri(volumeName), cc.getLong(0));
- resolvePlaylistMembers(playlistUri);
+ helper.runWithTransaction((db) -> {
+ try (Cursor cc = db.query("audio_playlists_map",
+ new String[] { "playlist_id" }, "audio_id=" + id,
+ null, "playlist_id", null, null)) {
+ while (cc.moveToNext()) {
+ final Uri playlistUri = ContentUris.withAppendedId(
+ Playlists.getContentUri(volumeName),
+ cc.getLong(0));
+ resolvePlaylistMembers(playlistUri);
+ }
}
- }
+ return null;
+ });
}
}
} finally {
@@ -3894,7 +3910,7 @@
synchronized (mDirectoryCache) {
mDirectoryCache.clear();
- return (int) helper.runWithTransaction(() -> {
+ return (int) helper.runWithTransaction((db) -> {
int n = 0;
int total = 0;
do {
@@ -3979,15 +3995,16 @@
case MediaStore.GET_VERSION_CALL: {
final String volumeName = extras.getString(Intent.EXTRA_TEXT);
- final SQLiteDatabase db;
+ final DatabaseHelper helper;
try {
- db = getDatabaseForUri(MediaStore.Files.getContentUri(volumeName))
- .getReadableDatabase();
+ helper = getDatabaseForUri(MediaStore.Files.getContentUri(volumeName));
} catch (VolumeNotFoundException e) {
throw e.rethrowAsIllegalArgumentException();
}
- final String version = db.getVersion() + ":" + DatabaseHelper.getOrCreateUuid(db);
+ final String version = helper.runWithoutTransaction((db) -> {
+ return db.getVersion() + ":" + DatabaseHelper.getOrCreateUuid(db);
+ });
final Bundle res = new Bundle();
res.putString(Intent.EXTRA_TEXT, version);
@@ -3996,14 +4013,17 @@
case MediaStore.GET_GENERATION_CALL: {
final String volumeName = extras.getString(Intent.EXTRA_TEXT);
- final long generation;
+ final DatabaseHelper helper;
try {
- generation = getDatabaseForUri(MediaStore.Files.getContentUri(volumeName))
- .getGeneration();
+ helper = getDatabaseForUri(MediaStore.Files.getContentUri(volumeName));
} catch (VolumeNotFoundException e) {
throw e.rethrowAsIllegalArgumentException();
}
+ final long generation = helper.runWithoutTransaction((db) -> {
+ return DatabaseHelper.getGeneration(db);
+ });
+
final Bundle res = new Bundle();
res.putLong(Intent.EXTRA_INDEX, generation);
return res;
@@ -4160,9 +4180,11 @@
mInternalDatabase,
mExternalDatabase
}) {
- final SQLiteDatabase db = helper.getReadableDatabase();
- db.execPerConnectionSQL("SELECT icu_load_collation(?, ?);",
- new String[] { locale, collationName });
+ helper.runWithoutTransaction((db) -> {
+ db.execPerConnectionSQL("SELECT icu_load_collation(?, ?);",
+ new String[] { locale, collationName });
+ return null;
+ });
}
mCustomCollators.add(collationName);
}
@@ -4170,12 +4192,9 @@
return collationName;
}
- private int pruneThumbnails(@NonNull CancellationSignal signal) {
+ private int pruneThumbnails(@NonNull SQLiteDatabase db, @NonNull CancellationSignal signal) {
int prunedCount = 0;
- final DatabaseHelper helper = mExternalDatabase;
- final SQLiteDatabase db = helper.getReadableDatabase();
-
// Determine all known media items
final LongArray knownIds = new LongArray();
try (Cursor c = db.query(true, "files", new String[] { BaseColumns._ID },
@@ -4362,27 +4381,28 @@
}
final DatabaseHelper helper;
- final SQLiteDatabase db;
try {
helper = getDatabaseForUri(uri);
- db = helper.getWritableDatabase();
} catch (VolumeNotFoundException e) {
Log.w(TAG, e);
return;
}
- final String idString = Long.toString(id);
- try (Cursor c = db.rawQuery("select _data from thumbnails where image_id=?"
- + " union all select _data from videothumbnails where video_id=?",
- new String[] { idString, idString })) {
- while (c.moveToNext()) {
- String path = c.getString(0);
- deleteIfAllowed(uri, Bundle.EMPTY, path);
+ helper.runWithTransaction((db) -> {
+ final String idString = Long.toString(id);
+ try (Cursor c = db.rawQuery("select _data from thumbnails where image_id=?"
+ + " union all select _data from videothumbnails where video_id=?",
+ new String[] { idString, idString })) {
+ while (c.moveToNext()) {
+ String path = c.getString(0);
+ deleteIfAllowed(uri, Bundle.EMPTY, path);
+ }
}
- }
- db.execSQL("delete from thumbnails where image_id=?", new String[] { idString });
- db.execSQL("delete from videothumbnails where video_id=?", new String[] { idString });
+ db.execSQL("delete from thumbnails where image_id=?", new String[] { idString });
+ db.execSQL("delete from videothumbnails where video_id=?", new String[] { idString });
+ return null;
+ });
}
/**
@@ -4785,21 +4805,24 @@
private void resolvePlaylistMembers(@NonNull Uri playlistUri) {
Trace.beginSection("resolvePlaylistMembers");
try {
- resolvePlaylistMembersInternal(playlistUri);
+ final DatabaseHelper helper;
+ try {
+ helper = getDatabaseForUri(playlistUri);
+ } catch (VolumeNotFoundException e) {
+ throw e.rethrowAsIllegalArgumentException();
+ }
+
+ helper.runWithTransaction((db) -> {
+ resolvePlaylistMembersInternal(playlistUri, db);
+ return null;
+ });
} finally {
Trace.endSection();
}
}
- private void resolvePlaylistMembersInternal(@NonNull Uri playlistUri) {
- final SQLiteDatabase db;
- try {
- db = getDatabaseForUri(playlistUri).getWritableDatabase();
- } catch (VolumeNotFoundException e) {
- throw e.rethrowAsIllegalArgumentException();
- }
-
- db.beginTransaction();
+ private void resolvePlaylistMembersInternal(@NonNull Uri playlistUri,
+ @NonNull SQLiteDatabase db) {
try {
// Refresh playlist members based on what we parse from disk
final long playlistId = ContentUris.parseId(playlistUri);
@@ -4825,11 +4848,8 @@
}
}
}
- db.setTransactionSuccessful();
} catch (IOException e) {
Log.w(TAG, "Failed to refresh playlist", e);
- } finally {
- db.endTransaction();
}
}
@@ -6518,8 +6538,11 @@
ForegroundThread.getExecutor().execute(() -> {
final DatabaseHelper helper = MediaStore.VOLUME_INTERNAL.equals(volume)
? mInternalDatabase : mExternalDatabase;
- ensureDefaultFolders(volume, helper);
- ensureThumbnailsValid(volume, helper);
+ helper.runWithTransaction((db) -> {
+ ensureDefaultFolders(volume, db);
+ ensureThumbnailsValid(volume, db);
+ return null;
+ });
});
}
return uri;
diff --git a/src/com/android/providers/media/MediaUpgradeReceiver.java b/src/com/android/providers/media/MediaUpgradeReceiver.java
index 932b20b..b9fba09 100644
--- a/src/com/android/providers/media/MediaUpgradeReceiver.java
+++ b/src/com/android/providers/media/MediaUpgradeReceiver.java
@@ -20,7 +20,6 @@
import android.content.Context;
import android.content.Intent;
import android.content.SharedPreferences;
-import android.database.sqlite.SQLiteDatabase;
import android.provider.Column;
import android.util.Log;
@@ -65,19 +64,19 @@
if (MediaProvider.isMediaDatabaseName(file)) {
long startTime = System.currentTimeMillis();
Log.i(TAG, "---> Start upgrade of media database " + file);
- SQLiteDatabase db = null;
try {
DatabaseHelper helper = new DatabaseHelper(
context, file, MediaProvider.isInternalMediaDatabaseName(file),
false, false, Column.class, Metrics::logSchemaChange, null, null,
null);
- db = helper.getWritableDatabase();
+ helper.runWithTransaction((db) -> {
+ // Perform just enough to force database upgrade
+ return db.getVersion();
+ });
+ helper.close();
} catch (Throwable t) {
Log.wtf(TAG, "Error during upgrade of media db " + file, t);
} finally {
- if (db != null) {
- db.close();
- }
}
Log.i(TAG, "<--- Finished upgrade of media database " + file
+ " in " + (System.currentTimeMillis()-startTime) + "ms");
diff --git a/src/com/android/providers/media/util/SQLiteQueryBuilder.java b/src/com/android/providers/media/util/SQLiteQueryBuilder.java
index da279aa..e3e0e71 100644
--- a/src/com/android/providers/media/util/SQLiteQueryBuilder.java
+++ b/src/com/android/providers/media/util/SQLiteQueryBuilder.java
@@ -425,8 +425,10 @@
public Cursor query(DatabaseHelper helper, String[] projectionIn,
String selection, String[] selectionArgs, String groupBy,
String having, String sortOrder, String limit, CancellationSignal cancellationSignal) {
- return query(helper.getReadableDatabase(), projectionIn, selection, selectionArgs, groupBy,
- having, sortOrder, limit, cancellationSignal);
+ return helper.runWithoutTransaction((db) -> {
+ return query(db, projectionIn, selection, selectionArgs, groupBy,
+ having, sortOrder, limit, cancellationSignal);
+ });
}
/**
@@ -524,8 +526,8 @@
public long insert(@NonNull DatabaseHelper helper, @NonNull ContentValues values) {
// We force wrap in a transaction to ensure that all mutations increment
// the generation counter
- return (int) helper.runWithTransaction(() -> {
- return insert(helper.getWritableDatabase(), values);
+ return helper.runWithTransaction((db) -> {
+ return insert(db, values);
});
}
@@ -568,8 +570,8 @@
@Nullable String selection, @Nullable String[] selectionArgs) {
// We force wrap in a transaction to ensure that all mutations increment
// the generation counter
- return (int) helper.runWithTransaction(() -> {
- return update(helper.getWritableDatabase(), values, selection, selectionArgs);
+ return helper.runWithTransaction((db) -> {
+ return update(db, values, selection, selectionArgs);
});
}
@@ -654,8 +656,8 @@
@Nullable String[] selectionArgs) {
// We force wrap in a transaction to ensure that all mutations increment
// the generation counter
- return (int) helper.runWithTransaction(() -> {
- return delete(helper.getWritableDatabase(), selection, selectionArgs);
+ return helper.runWithTransaction((db) -> {
+ return delete(db, selection, selectionArgs);
});
}
diff --git a/tests/src/com/android/providers/media/DatabaseHelperTest.java b/tests/src/com/android/providers/media/DatabaseHelperTest.java
index 1149854..a83cc13 100644
--- a/tests/src/com/android/providers/media/DatabaseHelperTest.java
+++ b/tests/src/com/android/providers/media/DatabaseHelperTest.java
@@ -74,7 +74,7 @@
@Test
public void testFilterVolumeNames() throws Exception {
try (DatabaseHelper helper = new DatabaseHelperR(getContext(), TEST_CLEAN_DB)) {
- SQLiteDatabase db = helper.getWritableDatabase();
+ SQLiteDatabase db = helper.getWritableDatabaseForTest();
{
final ContentValues values = new ContentValues();
values.put(FileColumns.MEDIA_TYPE, FileColumns.MEDIA_TYPE_AUDIO);
@@ -163,7 +163,7 @@
helper.endTransaction();
}
- helper.runWithTransaction(() -> {
+ helper.runWithTransaction((db) -> {
return 0;
});
}
@@ -188,7 +188,7 @@
Class<? extends DatabaseHelper> after) throws Exception {
try (DatabaseHelper helper = before.getConstructor(Context.class, String.class)
.newInstance(getContext(), TEST_DOWNGRADE_DB)) {
- SQLiteDatabase db = helper.getWritableDatabase();
+ SQLiteDatabase db = helper.getWritableDatabaseForTest();
{
final ContentValues values = new ContentValues();
values.put(FileColumns.DATA,
@@ -207,7 +207,7 @@
// Downgrade will wipe data, but at least we don't crash
try (DatabaseHelper helper = after.getConstructor(Context.class, String.class)
.newInstance(getContext(), TEST_DOWNGRADE_DB)) {
- SQLiteDatabase db = helper.getWritableDatabase();
+ SQLiteDatabase db = helper.getWritableDatabaseForTest();
try (Cursor c = db.query("files", null, null, null, null, null, null, null)) {
assertEquals(0, c.getCount());
}
@@ -235,7 +235,7 @@
Class<? extends DatabaseHelper> after) throws Exception {
try (DatabaseHelper helper = before.getConstructor(Context.class, String.class)
.newInstance(getContext(), TEST_RECOMPUTE_DB)) {
- SQLiteDatabase db = helper.getWritableDatabase();
+ SQLiteDatabase db = helper.getWritableDatabaseForTest();
{
final ContentValues values = new ContentValues();
values.put(FileColumns.DATA,
@@ -299,7 +299,7 @@
try (DatabaseHelper helper = after.getConstructor(Context.class, String.class)
.newInstance(getContext(), TEST_RECOMPUTE_DB)) {
- SQLiteDatabase db = helper.getWritableDatabase();
+ SQLiteDatabase db = helper.getWritableDatabaseForTest();
try (Cursor c = db.query("files", null, FileColumns.DISPLAY_NAME + "='global.jpg'",
null, null, null, null)) {
assertEquals(1, c.getCount());
@@ -369,18 +369,18 @@
Class<? extends DatabaseHelper> after) throws Exception {
try (DatabaseHelper helper = before.getConstructor(Context.class, String.class)
.newInstance(getContext(), TEST_UPGRADE_DB)) {
- SQLiteDatabase db = helper.getWritableDatabase();
+ SQLiteDatabase db = helper.getWritableDatabaseForTest();
}
try (DatabaseHelper helper = after.getConstructor(Context.class, String.class)
.newInstance(getContext(), TEST_UPGRADE_DB)) {
- SQLiteDatabase db = helper.getWritableDatabase();
+ SQLiteDatabase db = helper.getWritableDatabaseForTest();
// Create a second isolated instance from scratch and assert that
// upgraded schema is identical
try (DatabaseHelper helper2 = after.getConstructor(Context.class, String.class)
.newInstance(getContext(), TEST_CLEAN_DB)) {
- SQLiteDatabase db2 = helper2.getWritableDatabase();
+ SQLiteDatabase db2 = helper2.getWritableDatabaseForTest();
try (Cursor c1 = db.query("sqlite_master",
null, null, null, null, null, SQLITE_MASTER_ORDER_BY);
@@ -413,8 +413,8 @@
private static Set<String> queryValues(@NonNull DatabaseHelper helper, @NonNull String table,
@NonNull String columnName) {
- try (Cursor c = helper.getReadableDatabase().query(table, new String[] { columnName },
- null, null, null, null, null)) {
+ try (Cursor c = helper.getWritableDatabaseForTest().query(table,
+ new String[] { columnName }, null, null, null, null, null)) {
final ArraySet<String> res = new ArraySet<>();
while (c.moveToNext()) {
res.add(c.getString(0));
@@ -433,11 +433,6 @@
public void onCreate(SQLiteDatabase db) {
createOSchema(db, false);
}
-
- @Override
- public void onOpen(SQLiteDatabase db) {
- // Purposefully empty to leave views intact
- }
}
private static class DatabaseHelperP extends DatabaseHelper {
@@ -450,11 +445,6 @@
public void onCreate(SQLiteDatabase db) {
createPSchema(db, false);
}
-
- @Override
- public void onOpen(SQLiteDatabase db) {
- // Purposefully empty to leave views intact
- }
}
private static class DatabaseHelperQ extends DatabaseHelper {
@@ -467,11 +457,6 @@
public void onCreate(SQLiteDatabase db) {
createQSchema(db, false);
}
-
- @Override
- public void onOpen(SQLiteDatabase db) {
- // Purposefully empty to leave views intact
- }
}
private static class DatabaseHelperR extends DatabaseHelper {