fix: Cache errors when rapidly switching sources (#68) (#69)

muxinc · Oct 31, 2024 · f56b238 · f56b238
1 parent 5493827
commit f56b238
Show file tree

Hide file tree

Showing 7 changed files with 79 additions and 45 deletions.
diff --git a/library/build.gradle b/library/build.gradle
@@ -16,6 +16,8 @@ android {
 
     testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
     consumerProguardFiles "consumer-rules.pro"
+
+    multiDexEnabled true
   }
 
   buildTypes {

diff --git a/library/src/androidTest/java/com/mux/player/CacheDatastoreInstrumentationTests.kt b/library/src/androidTest/java/com/mux/player/CacheDatastoreInstrumentationTests.kt
@@ -8,6 +8,7 @@ import com.mux.player.internal.Constants
 import com.mux.player.internal.cache.CacheDatastore
 import com.mux.player.internal.cache.FileRecord
 import com.mux.player.internal.cache.filesDirNoBackupCompat
+import com.mux.player.internal.createLogcatLogger
 import org.junit.Assert
 import org.junit.Before
 import org.junit.Test
@@ -56,7 +57,7 @@ class CacheDatastoreInstrumentationTests {
 
   @Test
   fun testBasicInitialization() {
-    val datastore = CacheDatastore(appContext)
+    val datastore = CacheDatastore(appContext, logger = createLogcatLogger())
     datastore.use { it.open() }
 
     Assert.assertTrue(
@@ -80,7 +81,7 @@ class CacheDatastoreInstrumentationTests {
 
   @Test
   fun testCreateTempDownloadFile() {
-    val datastore = CacheDatastore(appContext)
+    val datastore = CacheDatastore(appContext, logger = createLogcatLogger())
     datastore.use {
       it.open()
 
@@ -102,7 +103,7 @@ class CacheDatastoreInstrumentationTests {
     val oldFileData = "old data".toByteArray(Charsets.UTF_8)
     val newFileData = "new data".toByteArray(Charsets.UTF_8)
 
-    val datastore = CacheDatastore(appContext)
+    val datastore = CacheDatastore(appContext, logger = createLogcatLogger())
     datastore.open()
     datastore.use {
       // Write one file...
@@ -132,7 +133,7 @@ class CacheDatastoreInstrumentationTests {
 
   @Test
   fun testWriteRecordReplacesOnKey() {
-    val datastore = CacheDatastore(appContext)
+    val datastore = CacheDatastore(appContext, logger = createLogcatLogger())
     datastore.use {
       datastore.open()
 
@@ -183,7 +184,7 @@ class CacheDatastoreInstrumentationTests {
   @Test
   fun testReadRecord() {
     fun testTheCase(url: String) {
-      CacheDatastore(appContext).use { datastore ->
+      CacheDatastore(appContext, logger = createLogcatLogger()).use { datastore ->
         datastore.open()
 
         val originalRecord = FileRecord(
@@ -216,7 +217,7 @@ class CacheDatastoreInstrumentationTests {
   @Test
   fun testReadLeastRecentFiles() {
     val maxCacheSize = 5L
-    CacheDatastore(appContext, maxDiskSize = maxCacheSize).use { datastore ->
+    CacheDatastore(appContext, maxDiskSize = maxCacheSize, createLogcatLogger()).use { datastore ->
       datastore.open()
       // For this test, size "units" are like one digit.
       //  time "units" start in the 3-digit range and tick at ~10 units per call to fakeNow()
@@ -279,7 +280,7 @@ class CacheDatastoreInstrumentationTests {
     val maxCacheSize = 5500L
     val dummyFileSize = 1000L
 
-    CacheDatastore(appContext, maxDiskSize = maxCacheSize).use { datastore ->
+    CacheDatastore(appContext, maxDiskSize = maxCacheSize, createLogcatLogger()).use { datastore ->
       datastore.open()
       //  time "units" start in the 3-digit range and tick at ~10 units per call to fakeNow()
       var fakeLastAccess = 200L // increment by some amount when you need to

diff --git a/library/src/main/java/com/mux/player/MuxPlayer.kt b/library/src/main/java/com/mux/player/MuxPlayer.kt
@@ -138,7 +138,10 @@ class MuxPlayer private constructor(
     private var customerData: CustomerData = CustomerData()
     private var exoPlayerBinding: ExoPlayerBinding? = null
     private var network: INetworkRequest? = null
-    private var muxPlayerCache: MuxPlayerCache = MuxPlayerCache.create(context.applicationContext)
+    private var muxPlayerCache: MuxPlayerCache = MuxPlayerCache.create(
+      context.applicationContext,
+      logger = logger ?: createNoLogger()
+    )
 
     constructor(context: Context) : this(context, ExoPlayer.Builder(context))
 

diff --git a/library/src/main/java/com/mux/player/internal/cache/CacheDatastore.kt b/library/src/main/java/com/mux/player/internal/cache/CacheDatastore.kt
@@ -5,7 +5,11 @@ import android.database.sqlite.SQLiteDatabase
 import android.database.sqlite.SQLiteOpenHelper
 import android.os.Build
 import android.util.Base64
+import android.util.Log
+import com.google.common.cache.Cache
 import com.mux.player.internal.Constants
+import com.mux.player.internal.Logger
+import com.mux.player.internal.createNoLogger
 import com.mux.player.oneOf
 import java.io.Closeable
 import java.io.File
@@ -24,6 +28,7 @@ import java.net.URL
 internal class CacheDatastore(
   val context: Context,
   val maxDiskSize: Long = 256 * 1024 * 1024,
+  val logger: Logger,
 ) : Closeable {
 
   companion object {
@@ -33,9 +38,13 @@ internal class CacheDatastore(
   }
 
   private val dbGuard = Any()
+
   // note - guarded by dbHelperGuard
   private var dbHelper: DbHelper? = null
 
+  // note - guarded by dbHelperGuard
+  private var sqlDb: SQLiteDatabase? = null
+
   /**
    * Opens the datastore, blocking until it is ready to use
    *
@@ -55,16 +64,15 @@ internal class CacheDatastore(
         ensureDirs()
 
         val newHelper = DbHelper(context.applicationContext, indexDbDir())
-        // acquire an extra reference until closed. prevents DB underneath from closing/reopening
-        newHelper.writableDatabase.acquireReference()
         this.dbHelper = newHelper
+        this.sqlDb = newHelper.writableDatabase
       }
     }
   }
 
   fun isOpen(): Boolean {
     synchronized(dbGuard) {
-      return dbHelper != null
+      return dbHelper != null && (sqlDb?.isOpen ?: false)
     }
   }
 
@@ -94,13 +102,11 @@ internal class CacheDatastore(
   }
 
   fun writeFileRecord(fileRecord: FileRecord): Result<Unit> {
-    val rowId = databaseOrThrow().use {
-      it.insertWithOnConflict(
-        IndexSql.Files.name, null,
-        fileRecord.toContentValues(),
-        SQLiteDatabase.CONFLICT_REPLACE
-      )
-    }
+    val rowId = databaseOrThrow().insertWithOnConflict(
+      IndexSql.Files.name, null,
+      fileRecord.toContentValues(),
+      SQLiteDatabase.CONFLICT_REPLACE
+    )
 
     return if (rowId >= 0) {
       Result.success(Unit)
@@ -110,32 +116,47 @@ internal class CacheDatastore(
   }
 
   fun readRecordByLookupKey(key: String): FileRecord? {
-    return databaseOrThrow().use {
-      it.query(
-        IndexSql.Files.name, null,
-        "${IndexSql.Files.Columns.lookupKey} is ?",
-        arrayOf(key),
-        null, null, null
-      ).use { cursor ->
-        if (cursor.count > 0 && cursor.moveToFirst()) {
-          cursor.toFileRecord()
-        } else {
-          null
-        }
+    return databaseOrThrow().query(
+      IndexSql.Files.name, null,
+      "${IndexSql.Files.Columns.lookupKey} is ?",
+      arrayOf(key),
+      null, null, null
+    ).use { cursor ->
+      if (cursor.count > 0 && cursor.moveToFirst()) {
+        cursor.toFileRecord()
+      } else {
+        null
       }
     }
   }
 
   fun readRecordByUrl(url: String): FileRecord? {
-    return databaseOrThrow().use {
-      it.query(
+    logger.i(
+      TAG,
+      "readRecordByUrl() tid=${Thread.currentThread().id} called for datastore $this\n\tfor url $url"
+    )
+    return databaseOrThrow().run {
+      query(
         IndexSql.Files.name, null,
         "${IndexSql.Files.Columns.lookupKey} is ?",
         arrayOf(safeCacheKey(URL(url))),
         null, null, null
       ).use { cursor ->
+        logger.d(
+          TAG,
+          "readRecordByUrl() tid=${Thread.currentThread().id} about to talk to the cursor $this\n\t btw is it closed according to Cursor? ${cursor.isClosed}"
+        )
+        logger.i(
+          TAG,
+          "readRecordByUrl() tid=${Thread.currentThread().id} \n\tcursor closed ${cursor.isClosed}\n\t db closed ${!isOpen}"
+        )
         if (cursor.count > 0 && cursor.moveToFirst()) {
-          cursor.toFileRecord()
+          cursor.toFileRecord().also {
+            logger.i(
+              TAG,
+              "readRecordByUrl() tid=${Thread.currentThread().id} returning with record\n\tfor $url"
+            )
+          }
         } else {
           null
         }
@@ -176,18 +197,18 @@ internal class CacheDatastore(
    * recently-used items
    */
   fun evictByLru(): Result<Int> {
-    return databaseOrThrow().use { doEvictByLru(it) }
+    return doEvictByLru(databaseOrThrow())
 
   }
 
   @Throws(IOException::class)
   override fun close() {
+    logger.i(TAG, "close() tid=${Thread.currentThread().id} called for datastore $this")
     synchronized(dbGuard) {
-      // release reference from when we opened. if any loader threads are reading/writing then
-      //  the db will close once they wind down
-      dbHelper?.writableDatabase?.releaseReference()
+      sqlDb?.close()
       dbHelper?.close()
       dbHelper = null
+      sqlDb = null
     }
   }
 
@@ -198,7 +219,7 @@ internal class CacheDatastore(
   internal fun readLeastRecentFiles(): List<FileRecord> {
     // This function is only visible for testing. doReadLeastRecentFiles() is called internally in
     //  a transaction with an already-open db
-    return databaseOrThrow().use { doReadLeastRecentFiles(it) }
+    return doReadLeastRecentFiles(databaseOrThrow())
   }
 
   /**
@@ -383,8 +404,7 @@ internal class CacheDatastore(
   @Throws(IOException::class)
   private fun databaseOrThrow(): SQLiteDatabase {
     synchronized(dbGuard) {
-      val helper = dbHelper ?: throw IOException("CacheDatastore was closed")
-      return helper.writableDatabase
+      return sqlDb?.takeIf { it.isOpen } ?: throw IOException("CacheDatastore was closed")
     }
   }
 }
@@ -400,6 +420,7 @@ private class DbHelper(
 ) {
 
   companion object {
+    private const val TAG = "CacheDatastore"
     private const val DB_FILE = "mux-player-cache.db"
   }
 

diff --git a/library/src/main/java/com/mux/player/internal/cache/MuxPlayerCache.kt b/library/src/main/java/com/mux/player/internal/cache/MuxPlayerCache.kt
@@ -1,6 +1,8 @@
 package com.mux.player.internal.cache
 
 import android.content.Context
+import com.mux.player.internal.Logger
+import com.mux.player.internal.createNoLogger
 import java.io.BufferedInputStream
 import java.io.BufferedOutputStream
 import java.io.Closeable
@@ -21,9 +23,11 @@ import java.util.concurrent.TimeUnit
  */
 class MuxPlayerCache private constructor(
   private val datastore: CacheDatastore,
+  private val logger: Logger,
 ) {
 
-  constructor(appContext: Context) : this(CacheDatastore(appContext.applicationContext))
+  constructor(appContext: Context, logger: Logger)
+      : this(CacheDatastore(appContext.applicationContext, logger = logger), logger)
 
   companion object {
     private const val TAG = "CacheController"
@@ -34,9 +38,10 @@ class MuxPlayerCache private constructor(
     val RX_S_MAX_AGE = Regex("""s-max-age=([0-9].*)""")
 
     @JvmSynthetic
-    internal fun create(appContext: Context) = MuxPlayerCache(appContext)
+    internal fun create(appContext: Context, logger: Logger) = MuxPlayerCache(appContext, logger)
     @JvmSynthetic
-    internal fun create(datastore: CacheDatastore) = MuxPlayerCache(datastore)
+    internal fun create(datastore: CacheDatastore, logger: Logger) =
+      MuxPlayerCache(datastore, logger)
   }
 
   /**

diff --git a/library/src/test/java/com/mux/player/CacheDatastoreTests.kt b/library/src/test/java/com/mux/player/CacheDatastoreTests.kt
@@ -2,6 +2,7 @@ package com.mux.player
 
 import android.content.Context
 import com.mux.player.internal.cache.CacheDatastore
+import com.mux.player.internal.createNoLogger
 import io.mockk.every
 import io.mockk.mockk
 import org.junit.Assert
@@ -21,7 +22,7 @@ class CacheDatastoreTests : AbsRobolectricTest() {
       }
     }
 
-    cacheDatastore = CacheDatastore(mockContext)
+    cacheDatastore = CacheDatastore(mockContext, logger = createNoLogger())
   }
 
   @Test

diff --git a/library/src/test/java/com/mux/player/MuxPlayerCacheTests.kt b/library/src/test/java/com/mux/player/MuxPlayerCacheTests.kt
@@ -3,6 +3,7 @@ package com.mux.player
 import android.content.Context
 import com.mux.player.internal.cache.MuxPlayerCache
 import com.mux.player.internal.cache.CacheDatastore
+import com.mux.player.internal.createNoLogger
 import io.mockk.every
 import io.mockk.mockk
 import org.junit.Assert
@@ -24,7 +25,7 @@ class MuxPlayerCacheTests : AbsRobolectricTest() {
       }
     }
 
-    muxPlayerCache = MuxPlayerCache.create(mockDatastore)
+    muxPlayerCache = MuxPlayerCache.create(mockDatastore, logger = createNoLogger())
   }
 
   @Test