Commit 3db26981 by christosts Committed by Ian Baker

Synchronize initialization of DefaultDownloadIndex

There is a race condition when initializing the downloads database. The
constructor of the DownloadManager kicks-off the database initialization
in its internal thread, but at the same time an app can try to access
the database directly through the manager's download index, e.g. doing

DonwloadManager manager = new ...
manager.getDownloadIndex().getDownload("id");

might enter DefaultDownloadIndex.ensureInitialized() from two threads.
When upgrading the downloads table from version 2 to version 3, the
first thread that enters the database transaction in ensureInitialized()
will drop and recreate the table using the v3 schema. Then, the second
thread will attempt to read from the newly created table using the v2
schema, which will fail.

This race condition was not introduced in 2.12 but was there already.
However, prior to 2.12, the code only dropped and re-created and the
table and did not attempt to read any data. Hence, if the race condition
happened, the code would drop and create the table twice, but no error
would occur.

Issue: #8420
#minor-release
PiperOrigin-RevId: 350745463
parent 6b892796
......@@ -21,6 +21,7 @@ import android.database.SQLException;
import android.database.sqlite.SQLiteDatabase;
import android.database.sqlite.SQLiteException;
import android.net.Uri;
import androidx.annotation.GuardedBy;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import com.google.android.exoplayer2.database.DatabaseIOException;
......@@ -136,7 +137,9 @@ public final class DefaultDownloadIndex implements WritableDownloadIndex {
private final String name;
private final String tableName;
private final DatabaseProvider databaseProvider;
private final Object initializationLock;
@GuardedBy("initializationLock")
private boolean initialized;
/**
......@@ -168,6 +171,7 @@ public final class DefaultDownloadIndex implements WritableDownloadIndex {
this.name = name;
this.databaseProvider = databaseProvider;
tableName = TABLE_PREFIX + name;
initializationLock = new Object();
}
@Override
......@@ -273,33 +277,35 @@ public final class DefaultDownloadIndex implements WritableDownloadIndex {
}
private void ensureInitialized() throws DatabaseIOException {
if (initialized) {
return;
}
try {
SQLiteDatabase readableDatabase = databaseProvider.getReadableDatabase();
int version = VersionTable.getVersion(readableDatabase, VersionTable.FEATURE_OFFLINE, name);
if (version != TABLE_VERSION) {
SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase();
writableDatabase.beginTransactionNonExclusive();
try {
VersionTable.setVersion(
writableDatabase, VersionTable.FEATURE_OFFLINE, name, TABLE_VERSION);
List<Download> upgradedDownloads =
version == 2 ? loadDownloadsFromVersion2(writableDatabase) : new ArrayList<>();
writableDatabase.execSQL("DROP TABLE IF EXISTS " + tableName);
writableDatabase.execSQL("CREATE TABLE " + tableName + " " + TABLE_SCHEMA);
for (Download download : upgradedDownloads) {
putDownloadInternal(download, writableDatabase);
synchronized (initializationLock) {
if (initialized) {
return;
}
try {
SQLiteDatabase readableDatabase = databaseProvider.getReadableDatabase();
int version = VersionTable.getVersion(readableDatabase, VersionTable.FEATURE_OFFLINE, name);
if (version != TABLE_VERSION) {
SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase();
writableDatabase.beginTransactionNonExclusive();
try {
VersionTable.setVersion(
writableDatabase, VersionTable.FEATURE_OFFLINE, name, TABLE_VERSION);
List<Download> upgradedDownloads =
version == 2 ? loadDownloadsFromVersion2(writableDatabase) : new ArrayList<>();
writableDatabase.execSQL("DROP TABLE IF EXISTS " + tableName);
writableDatabase.execSQL("CREATE TABLE " + tableName + " " + TABLE_SCHEMA);
for (Download download : upgradedDownloads) {
putDownloadInternal(download, writableDatabase);
}
writableDatabase.setTransactionSuccessful();
} finally {
writableDatabase.endTransaction();
}
writableDatabase.setTransactionSuccessful();
} finally {
writableDatabase.endTransaction();
}
initialized = true;
} catch (SQLException e) {
throw new DatabaseIOException(e);
}
initialized = true;
} catch (SQLException e) {
throw new DatabaseIOException(e);
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment