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
This commit is contained in:
christosts 2021-01-08 13:29:02 +00:00 committed by Ian Baker
parent ff8c8645ab
commit 04250031bc

View file

@ -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);
}
writableDatabase.setTransactionSuccessful();
} finally {
writableDatabase.endTransaction();
}
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();
}
}
initialized = true;
} catch (SQLException e) {
throw new DatabaseIOException(e);
}
initialized = true;
} catch (SQLException e) {
throw new DatabaseIOException(e);
}
}