Multi-process lock for opening DB (and migrations too)
Adds a lock at the FrameworkSQLite* level, to protect multi-process 1st time database creation and migrations. To avoid a performance regression, process-lock is only done when the database instance is not known to be opened, such that consecutive getDatabase() calls are quick and do not have to use a FileLock. Note that in-memory databases (null name) also don't use a FileLock.
Bug: 193182592
Test: n/a
Relnote: Added API for multi-process lock and usage at the FrameworkSQLite* level, to protect multi-process 1st time database creation and migrations.
Change-Id: Ied267cd32e3248cec5ebb772778d2e94fd450270
diff --git a/jetifier/jetifier/core/src/main/resources/default.config b/jetifier/jetifier/core/src/main/resources/default.config
index 8dce36f..18617cf 100644
--- a/jetifier/jetifier/core/src/main/resources/default.config
+++ b/jetifier/jetifier/core/src/main/resources/default.config
@@ -1258,6 +1258,10 @@
"to" : "androidx/sqlite/db/framework"
},
{
+ "from" : "android/arch/persistence/util",
+ "to" : "androidx/sqlite/util"
+ },
+ {
"from" : "android/arch/persistence/db",
"to" : "androidx/sqlite/db"
},
diff --git a/jetifier/jetifier/core/src/main/resources/default.generated.config b/jetifier/jetifier/core/src/main/resources/default.generated.config
index 072323d..cadb76f 100644
--- a/jetifier/jetifier/core/src/main/resources/default.generated.config
+++ b/jetifier/jetifier/core/src/main/resources/default.generated.config
@@ -1209,6 +1209,10 @@
"to": "androidx/sqlite/db/framework"
},
{
+ "from": "android/arch/persistence/util",
+ "to": "androidx/sqlite/util"
+ },
+ {
"from": "android/arch/persistence/db",
"to": "androidx/sqlite/db"
},
diff --git a/jetifier/jetifier/migration.config b/jetifier/jetifier/migration.config
index 35cac7a..dbf327f 100644
--- a/jetifier/jetifier/migration.config
+++ b/jetifier/jetifier/migration.config
@@ -549,6 +549,10 @@
"to": "ignore"
},
{
+ "from": "androidx/sqlite/util/(.*)",
+ "to": "ignore"
+ },
+ {
"from": "androidx/sqlite/db/(.*)",
"to": "ignore"
},
diff --git a/room/room-runtime/api/restricted_current.ignore b/room/room-runtime/api/restricted_current.ignore
new file mode 100644
index 0000000..e6a9bf9
--- /dev/null
+++ b/room/room-runtime/api/restricted_current.ignore
@@ -0,0 +1,3 @@
+// Baseline format: 1.0
+RemovedClass: androidx.room.util.CopyLock:
+ Removed class androidx.room.util.CopyLock
diff --git a/room/room-runtime/api/restricted_current.txt b/room/room-runtime/api/restricted_current.txt
index 17a238e..8f29006 100644
--- a/room/room-runtime/api/restricted_current.txt
+++ b/room/room-runtime/api/restricted_current.txt
@@ -249,12 +249,6 @@
package androidx.room.util {
- @RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX) public class CopyLock {
- ctor public CopyLock(String, java.io.File, boolean);
- method public void lock();
- method public void unlock();
- }
-
@RestrictTo(androidx.annotation.RestrictTo.Scope.LIBRARY_GROUP_PREFIX) public class CursorUtil {
method public static android.database.Cursor copyAndClose(android.database.Cursor);
method public static int getColumnIndex(android.database.Cursor, String);
diff --git a/room/room-runtime/src/main/java/androidx/room/SQLiteCopyOpenHelper.java b/room/room-runtime/src/main/java/androidx/room/SQLiteCopyOpenHelper.java
index f7b8568..cf8df6e 100644
--- a/room/room-runtime/src/main/java/androidx/room/SQLiteCopyOpenHelper.java
+++ b/room/room-runtime/src/main/java/androidx/room/SQLiteCopyOpenHelper.java
@@ -23,12 +23,12 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.RequiresApi;
-import androidx.room.util.CopyLock;
import androidx.room.util.DBUtil;
import androidx.room.util.FileUtil;
import androidx.sqlite.db.SupportSQLiteDatabase;
import androidx.sqlite.db.SupportSQLiteOpenHelper;
import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory;
+import androidx.sqlite.util.ProcessLock;
import java.io.File;
import java.io.FileInputStream;
@@ -129,7 +129,8 @@
File databaseFile = mContext.getDatabasePath(databaseName);
boolean processLevelLock = mDatabaseConfiguration == null
|| mDatabaseConfiguration.multiInstanceInvalidation;
- CopyLock copyLock = new CopyLock(databaseName, mContext.getFilesDir(), processLevelLock);
+ ProcessLock copyLock = new ProcessLock(databaseName, mContext.getFilesDir(),
+ processLevelLock);
try {
// Acquire a copy lock, this lock works across threads and processes, preventing
// concurrent copy attempts from occurring.
diff --git a/sqlite/sqlite-framework/api/current.txt b/sqlite/sqlite-framework/api/current.txt
index 526c565..7ad7e09 100644
--- a/sqlite/sqlite-framework/api/current.txt
+++ b/sqlite/sqlite-framework/api/current.txt
@@ -8,3 +8,14 @@
}
+package androidx.sqlite.util {
+
+ public final class ProcessLock {
+ ctor public ProcessLock(String, java.io.File, boolean);
+ method public void lock();
+ method public void lock(boolean);
+ method public void unlock();
+ }
+
+}
+
diff --git a/sqlite/sqlite-framework/api/public_plus_experimental_current.txt b/sqlite/sqlite-framework/api/public_plus_experimental_current.txt
index 526c565..7ad7e09 100644
--- a/sqlite/sqlite-framework/api/public_plus_experimental_current.txt
+++ b/sqlite/sqlite-framework/api/public_plus_experimental_current.txt
@@ -8,3 +8,14 @@
}
+package androidx.sqlite.util {
+
+ public final class ProcessLock {
+ ctor public ProcessLock(String, java.io.File, boolean);
+ method public void lock();
+ method public void lock(boolean);
+ method public void unlock();
+ }
+
+}
+
diff --git a/sqlite/sqlite-framework/api/restricted_current.txt b/sqlite/sqlite-framework/api/restricted_current.txt
index 526c565..7ad7e09 100644
--- a/sqlite/sqlite-framework/api/restricted_current.txt
+++ b/sqlite/sqlite-framework/api/restricted_current.txt
@@ -8,3 +8,14 @@
}
+package androidx.sqlite.util {
+
+ public final class ProcessLock {
+ ctor public ProcessLock(String, java.io.File, boolean);
+ method public void lock();
+ method public void lock(boolean);
+ method public void unlock();
+ }
+
+}
+
diff --git a/sqlite/sqlite-framework/build.gradle b/sqlite/sqlite-framework/build.gradle
index bfac2ce..1be738a 100644
--- a/sqlite/sqlite-framework/build.gradle
+++ b/sqlite/sqlite-framework/build.gradle
@@ -23,7 +23,7 @@
}
dependencies {
- api("androidx.annotation:annotation:1.0.0")
+ api("androidx.annotation:annotation:1.2.0")
api(project(":sqlite:sqlite"))
}
diff --git a/sqlite/sqlite-framework/src/main/java/androidx/sqlite/db/framework/FrameworkSQLiteOpenHelper.java b/sqlite/sqlite-framework/src/main/java/androidx/sqlite/db/framework/FrameworkSQLiteOpenHelper.java
index cc9415b..b3f6a12 100644
--- a/sqlite/sqlite-framework/src/main/java/androidx/sqlite/db/framework/FrameworkSQLiteOpenHelper.java
+++ b/sqlite/sqlite-framework/src/main/java/androidx/sqlite/db/framework/FrameworkSQLiteOpenHelper.java
@@ -26,8 +26,10 @@
import androidx.sqlite.db.SupportSQLiteCompat;
import androidx.sqlite.db.SupportSQLiteDatabase;
import androidx.sqlite.db.SupportSQLiteOpenHelper;
+import androidx.sqlite.util.ProcessLock;
import java.io.File;
+import java.util.UUID;
class FrameworkSQLiteOpenHelper implements SupportSQLiteOpenHelper {
@@ -132,6 +134,9 @@
final Callback mCallback;
// see b/78359448
private boolean mMigrated;
+ // see b/193182592
+ private final ProcessLock mLock;
+ private boolean mOpened;
OpenHelper(Context context, String name, final FrameworkSQLiteDatabase[] dbRef,
final Callback callback) {
@@ -144,28 +149,40 @@
});
mCallback = callback;
mDbRef = dbRef;
+ mLock = new ProcessLock(name == null ? UUID.randomUUID().toString() : name,
+ context.getCacheDir(), false);
}
- synchronized SupportSQLiteDatabase getWritableSupportDatabase() {
- mMigrated = false;
- SQLiteDatabase db = super.getWritableDatabase();
- if (mMigrated) {
- // there might be a connection w/ stale structure, we should re-open.
- close();
- return getWritableSupportDatabase();
+ SupportSQLiteDatabase getWritableSupportDatabase() {
+ try {
+ mLock.lock(!mOpened && getDatabaseName() != null);
+ mMigrated = false;
+ SQLiteDatabase db = super.getWritableDatabase();
+ if (mMigrated) {
+ // there might be a connection w/ stale structure, we should re-open.
+ close();
+ return getWritableSupportDatabase();
+ }
+ return getWrappedDb(db);
+ } finally {
+ mLock.unlock();
}
- return getWrappedDb(db);
}
- synchronized SupportSQLiteDatabase getReadableSupportDatabase() {
- mMigrated = false;
- SQLiteDatabase db = super.getReadableDatabase();
- if (mMigrated) {
- // there might be a connection w/ stale structure, we should re-open.
- close();
- return getReadableSupportDatabase();
+ SupportSQLiteDatabase getReadableSupportDatabase() {
+ try {
+ mLock.lock(!mOpened && getDatabaseName() != null);
+ mMigrated = false;
+ SQLiteDatabase db = super.getReadableDatabase();
+ if (mMigrated) {
+ // there might be a connection w/ stale structure, we should re-open.
+ close();
+ return getReadableSupportDatabase();
+ }
+ return getWrappedDb(db);
+ } finally {
+ mLock.unlock();
}
- return getWrappedDb(db);
}
FrameworkSQLiteDatabase getWrappedDb(SQLiteDatabase sqLiteDatabase) {
@@ -200,12 +217,20 @@
// if we've migrated, we'll re-open the db so we should not call the callback.
mCallback.onOpen(getWrappedDb(db));
}
+ mOpened = true;
}
@Override
- public synchronized void close() {
- super.close();
- mDbRef[0] = null;
+ @SuppressWarnings("UnsynchronizedOverridesSynchronized") // No need sync due to locks.
+ public void close() {
+ try {
+ mLock.lock();
+ super.close();
+ mDbRef[0] = null;
+ mOpened = false;
+ } finally {
+ mLock.unlock();
+ }
}
static FrameworkSQLiteDatabase getWrappedDb(FrameworkSQLiteDatabase[] refHolder,
diff --git a/room/room-runtime/src/main/java/androidx/room/util/CopyLock.java b/sqlite/sqlite-framework/src/main/java/androidx/sqlite/util/ProcessLock.java
similarity index 72%
rename from room/room-runtime/src/main/java/androidx/room/util/CopyLock.java
rename to sqlite/sqlite-framework/src/main/java/androidx/sqlite/util/ProcessLock.java
index 8db020c..c8d2591 100644
--- a/room/room-runtime/src/main/java/androidx/room/util/CopyLock.java
+++ b/sqlite/sqlite-framework/src/main/java/androidx/sqlite/util/ProcessLock.java
@@ -14,10 +14,9 @@
* limitations under the License.
*/
-package androidx.room.util;
+package androidx.sqlite.util;
import androidx.annotation.NonNull;
-import androidx.annotation.RestrictTo;
import java.io.File;
import java.io.FileOutputStream;
@@ -29,8 +28,8 @@
import java.util.concurrent.locks.ReentrantLock;
/**
- * Utility class for in-process and multi-process key-based lock mechanism for safely copying
- * database files.
+ * Utility class for in-process and multi-process key-based lock mechanism for safely doing
+ * synchronized operations.
* <p>
* Acquiring the lock will be quick if no other thread or process has a lock with the same key.
* But if the lock is already held then acquiring it will block, until the other thread or process
@@ -45,16 +44,13 @@
* <li>
* Multi-process locking is done via a lock file whose name contains the key and FileLock
* objects.
- *
- * @hide
*/
-@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP_PREFIX)
-public class CopyLock {
+public final class ProcessLock {
// in-process lock map
private static final Map<String, Lock> sThreadLocks = new HashMap<>();
- private final File mCopyLockFile;
+ private final File mLockFile;
private final Lock mThreadLock;
private final boolean mFileLevelLock;
private FileChannel mLockChannel;
@@ -64,11 +60,13 @@
* lock files.
* @param name the name of this lock.
* @param lockDir the directory where the lock files will be located.
- * @param processLock whether to use file for process level locking or not.
+ * @param processLock whether to use file for process level locking or not by default. The
+ * behaviour can be overridden via the {@link #lock(boolean)}} method.
*/
- public CopyLock(@NonNull String name, @NonNull File lockDir, boolean processLock) {
- mCopyLockFile = new File(lockDir, name + ".lck");
- mThreadLock = getThreadLock(mCopyLockFile.getAbsolutePath());
+ @SuppressWarnings("StreamFiles") // Overloads with FileDescriptor or Stream are invalid.
+ public ProcessLock(@NonNull String name, @NonNull File lockDir, boolean processLock) {
+ mLockFile = new File(lockDir, name + ".lck");
+ mThreadLock = getThreadLock(mLockFile.getAbsolutePath());
mFileLevelLock = processLock;
}
@@ -76,13 +74,27 @@
* Attempts to grab the lock, blocking if already held by another thread or process.
*/
public void lock() {
+ lock(mFileLevelLock);
+ }
+
+ /**
+ * Attempts to grab the lock, blocking if already held by another thread or process.
+ *
+ * @param processLock whether to use file for process level locking or not.
+ */
+ public void lock(boolean processLock) {
mThreadLock.lock();
- if (mFileLevelLock) {
+ if (processLock) {
try {
- mLockChannel = new FileOutputStream(mCopyLockFile).getChannel();
+ // Verify parent dir
+ File parentDir = mLockFile.getParentFile();
+ if (parentDir != null) {
+ parentDir.mkdirs();
+ }
+ mLockChannel = new FileOutputStream(mLockFile).getChannel();
mLockChannel.lock();
} catch (IOException e) {
- throw new IllegalStateException("Unable to grab copy lock.", e);
+ throw new IllegalStateException("Unable to grab file lock.", e);
}
}
}