Report multiple in-process primary connections
This adds dumpsys and logcat reporting for multiple primary
connections to a single database file. Multiple primary connections
are not bad by themselves but misuse can lead to SQLITE_BUSY
failures. These changes are meant to simplify debugging the
SQLITE_BUSY failures.
If any database file is opened more than once (concurrently) then the
dumpsys output contains the following text:
Concurrently opened database files
<list of files>
If no database files were opened concurrently then the dumpsys output
does not contain the specified block.
This change only detects multiple primary connections in a single
process.
Test: atest
* FrameworksCoreTests:android.database
* CtsDatabaseTestCases
* SQLiteDatabasePerfTest
Bug: 309135899
Change-Id: I6550c4a0d6ff0785f67bd9d7d539c1ac52828b87
diff --git a/core/java/android/database/sqlite/SQLiteDatabase.java b/core/java/android/database/sqlite/SQLiteDatabase.java
index 8a4f678..35ae3c9 100644
--- a/core/java/android/database/sqlite/SQLiteDatabase.java
+++ b/core/java/android/database/sqlite/SQLiteDatabase.java
@@ -40,12 +40,15 @@
import android.os.OperationCanceledException;
import android.os.SystemProperties;
import android.text.TextUtils;
+import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.EventLog;
import android.util.Log;
import android.util.Pair;
import android.util.Printer;
+import com.android.internal.annotations.GuardedBy;
+import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.Preconditions;
import dalvik.annotation.optimization.NeverCompile;
@@ -103,8 +106,14 @@
// Stores reference to all databases opened in the current process.
// (The referent Object is not used at this time.)
// INVARIANT: Guarded by sActiveDatabases.
+ @GuardedBy("sActiveDatabases")
private static WeakHashMap<SQLiteDatabase, Object> sActiveDatabases = new WeakHashMap<>();
+ // Tracks which database files are currently open. If a database file is opened more than
+ // once at any given moment, the associated databases are marked as "concurrent".
+ @GuardedBy("sActiveDatabases")
+ private static final OpenTracker sOpenTracker = new OpenTracker();
+
// Thread-local for database sessions that belong to this database.
// Each thread has its own database session.
// INVARIANT: Immutable.
@@ -510,6 +519,7 @@
private void dispose(boolean finalized) {
final SQLiteConnectionPool pool;
+ final String path;
synchronized (mLock) {
if (mCloseGuardLocked != null) {
if (finalized) {
@@ -520,10 +530,12 @@
pool = mConnectionPoolLocked;
mConnectionPoolLocked = null;
+ path = isInMemoryDatabase() ? null : getPath();
}
if (!finalized) {
synchronized (sActiveDatabases) {
+ sOpenTracker.close(path);
sActiveDatabases.remove(this);
}
@@ -1132,6 +1144,74 @@
}
}
+ /**
+ * Track the number of times a database file has been opened. There is a primary connection
+ * associated with every open database, and these can contend with each other, leading to
+ * unexpected SQLiteDatabaseLockedException exceptions. The tracking here is only advisory:
+ * multiply-opened databases are logged but no other action is taken.
+ *
+ * This class is not thread-safe.
+ */
+ private static class OpenTracker {
+ // The list of currently-open databases. This maps the database file to the number of
+ // currently-active opens.
+ private final ArrayMap<String, Integer> mOpens = new ArrayMap<>();
+
+ // The maximum number of concurrently open database paths that will be stored. Once this
+ // many paths have been recorded, further paths are logged but not saved.
+ private static final int MAX_RECORDED_PATHS = 20;
+
+ // The list of databases that were ever concurrently opened.
+ private final ArraySet<String> mConcurrent = new ArraySet<>();
+
+ /** Return the canonical path. On error, just return the input path. */
+ private static String normalize(String path) {
+ try {
+ return new File(path).toPath().toRealPath().toString();
+ } catch (Exception e) {
+ // If there is an IO or security exception, just continue, using the input path.
+ return path;
+ }
+ }
+
+ /** Return true if the path is currently open in another SQLiteDatabase instance. */
+ void open(@Nullable String path) {
+ if (path == null) return;
+ path = normalize(path);
+
+ Integer count = mOpens.get(path);
+ if (count == null || count == 0) {
+ mOpens.put(path, 1);
+ return;
+ } else {
+ mOpens.put(path, count + 1);
+ if (mConcurrent.size() < MAX_RECORDED_PATHS) {
+ mConcurrent.add(path);
+ }
+ Log.w(TAG, "multiple primary connections on " + path);
+ return;
+ }
+ }
+
+ void close(@Nullable String path) {
+ if (path == null) return;
+ path = normalize(path);
+ Integer count = mOpens.get(path);
+ if (count == null || count <= 0) {
+ Log.e(TAG, "open database counting failure on " + path);
+ } else if (count == 1) {
+ // Implicitly set the count to zero, and make mOpens smaller.
+ mOpens.remove(path);
+ } else {
+ mOpens.put(path, count - 1);
+ }
+ }
+
+ ArraySet<String> getConcurrentDatabasePaths() {
+ return new ArraySet<>(mConcurrent);
+ }
+ }
+
private void open() {
try {
try {
@@ -1153,14 +1233,17 @@
}
private void openInner() {
+ final String path;
synchronized (mLock) {
assert mConnectionPoolLocked == null;
mConnectionPoolLocked = SQLiteConnectionPool.open(mConfigurationLocked);
mCloseGuardLocked.open("close");
+ path = isInMemoryDatabase() ? null : getPath();
}
synchronized (sActiveDatabases) {
sActiveDatabases.put(this, null);
+ sOpenTracker.open(path);
}
}
@@ -2345,6 +2428,17 @@
}
/**
+ * Return list of databases that have been concurrently opened.
+ * @hide
+ */
+ @VisibleForTesting
+ public static ArraySet<String> getConcurrentDatabasePaths() {
+ synchronized (sActiveDatabases) {
+ return sOpenTracker.getConcurrentDatabasePaths();
+ }
+ }
+
+ /**
* Returns true if the new version code is greater than the current database version.
*
* @param newVersion The new version code.
@@ -2766,6 +2860,19 @@
dumpDatabaseDirectory(printer, new File(dir), isSystem);
}
}
+
+ // Dump concurrently-opened database files, if any
+ final ArraySet<String> concurrent;
+ synchronized (sActiveDatabases) {
+ concurrent = sOpenTracker.getConcurrentDatabasePaths();
+ }
+ if (concurrent.size() > 0) {
+ printer.println("");
+ printer.println("Concurrently opened database files");
+ for (String f : concurrent) {
+ printer.println(" " + f);
+ }
+ }
}
private static void dumpDatabaseDirectory(Printer pw, File dir, boolean isSystem) {
diff --git a/core/tests/coretests/src/android/database/sqlite/SQLiteDatabaseTest.java b/core/tests/coretests/src/android/database/sqlite/SQLiteDatabaseTest.java
index e118c98d..3ee565f 100644
--- a/core/tests/coretests/src/android/database/sqlite/SQLiteDatabaseTest.java
+++ b/core/tests/coretests/src/android/database/sqlite/SQLiteDatabaseTest.java
@@ -403,4 +403,41 @@
}
assertFalse(allowed);
}
+
+ /** Return true if the path is in the list of strings. */
+ private boolean isConcurrent(String path) throws Exception {
+ path = new File(path).toPath().toRealPath().toString();
+ return SQLiteDatabase.getConcurrentDatabasePaths().contains(path);
+ }
+
+ @Test
+ public void testDuplicateDatabases() throws Exception {
+ // The two database paths in this test are assumed not to have been opened earlier in this
+ // process.
+
+ // A database path that will be opened twice.
+ final String dbName = "never-used-db.db";
+ final File dbFile = mContext.getDatabasePath(dbName);
+ final String dbPath = dbFile.getPath();
+
+ // A database path that will be opened only once.
+ final String okName = "never-used-ok.db";
+ final File okFile = mContext.getDatabasePath(okName);
+ final String okPath = okFile.getPath();
+
+ SQLiteDatabase db1 = SQLiteDatabase.openOrCreateDatabase(dbFile, null);
+ assertFalse(isConcurrent(dbPath));
+ SQLiteDatabase db2 = SQLiteDatabase.openOrCreateDatabase(dbFile, null);
+ assertTrue(isConcurrent(dbPath));
+ db1.close();
+ assertTrue(isConcurrent(dbPath));
+ db2.close();
+ assertTrue(isConcurrent(dbPath));
+
+ SQLiteDatabase db3 = SQLiteDatabase.openOrCreateDatabase(okFile, null);
+ db3.close();
+ db3 = SQLiteDatabase.openOrCreateDatabase(okFile, null);
+ assertFalse(isConcurrent(okPath));
+ db3.close();
+ }
}