Skip to content

Commit 4f850ef

Browse files
Don't crash if creating a cache directory fails (#1657)
* Don't crash if creating a cache directory fails I'd previously attempted to recover when writes fail, but I missed this case. * Track an API change --------- Co-authored-by: Jesse Wilson <jwilson@squareup.com>
1 parent f2976cb commit 4f850ef

File tree

18 files changed

+160
-85
lines changed

18 files changed

+160
-85
lines changed

zipline-loader/api/android/zipline-loader.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ public final class app/cash/zipline/loader/SignatureAlgorithmId : java/lang/Enum
102102
}
103103

104104
public final class app/cash/zipline/loader/ZiplineCache : java/io/Closeable {
105+
public synthetic fun <init> (Lapp/cash/sqldelight/db/SqlDriver;Lapp/cash/zipline/loader/internal/cache/Database;Lokio/FileSystem;Lokio/Path;JLapp/cash/zipline/loader/LoaderEventListener;ZLkotlin/jvm/internal/DefaultConstructorMarker;)V
105106
public fun close ()V
106107
}
107108

zipline-loader/api/jvm/zipline-loader.api

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ public final class app/cash/zipline/loader/SignatureAlgorithmId : java/lang/Enum
102102
}
103103

104104
public final class app/cash/zipline/loader/ZiplineCache : java/io/Closeable {
105+
public synthetic fun <init> (Lapp/cash/sqldelight/db/SqlDriver;Lapp/cash/zipline/loader/internal/cache/Database;Lokio/FileSystem;Lokio/Path;JLapp/cash/zipline/loader/LoaderEventListener;ZLkotlin/jvm/internal/DefaultConstructorMarker;)V
105106
public fun close ()V
106107
}
107108

zipline-loader/src/androidMain/kotlin/app/cash/zipline/loader/internal/cache/databaseAndroid.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ import app.cash.sqldelight.db.SqlSchema
2323
import app.cash.sqldelight.driver.android.AndroidSqliteDriver
2424
import okio.Path
2525

26-
internal actual class SqlDriverFactory(
26+
internal class AndroidSqliteDriverFactory(
2727
private val context: Context,
28-
) {
29-
actual fun create(path: Path, schema: SqlSchema<QueryResult.Value<Unit>>): SqlDriver {
28+
) : SqlDriverFactory {
29+
override fun create(path: Path, schema: SqlSchema<QueryResult.Value<Unit>>): SqlDriver {
3030
validateDbPath(path)
3131
return AndroidSqliteDriver(
3232
schema = schema,

zipline-loader/src/androidMain/kotlin/app/cash/zipline/loader/loaderAndroid.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
package app.cash.zipline.loader
1717

1818
import android.content.Context
19-
import app.cash.zipline.loader.internal.cache.SqlDriverFactory
19+
import app.cash.zipline.loader.internal.cache.AndroidSqliteDriverFactory
2020
import okio.FileSystem
2121
import okio.Path
2222

@@ -28,7 +28,7 @@ fun ZiplineCache(
2828
loaderEventListener: LoaderEventListener,
2929
): ZiplineCache {
3030
return ZiplineCache(
31-
sqlDriverFactory = SqlDriverFactory(context),
31+
sqlDriverFactory = AndroidSqliteDriverFactory(context),
3232
fileSystem = fileSystem,
3333
directory = directory,
3434
maxSizeInBytes = maxSizeInBytes,
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* Copyright (C) 2025 Cash App
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package app.cash.zipline.loader
17+
18+
import app.cash.sqldelight.Query
19+
import app.cash.sqldelight.db.QueryResult
20+
import app.cash.sqldelight.db.SqlCursor
21+
import app.cash.sqldelight.db.SqlDriver
22+
import app.cash.sqldelight.db.SqlPreparedStatement
23+
import okio.IOException
24+
25+
/**
26+
* A SQL Driver that always throws [IOException]. Used this when creating a driver fails, such as
27+
* when the disk is full.
28+
*/
29+
internal class NullSqlDriver : SqlDriver {
30+
override fun <R> executeQuery(
31+
identifier: Int?,
32+
sql: String,
33+
mapper: (SqlCursor) -> QueryResult<R>,
34+
parameters: Int,
35+
binders: (SqlPreparedStatement.() -> Unit)?,
36+
) = throw IOException("NullSqlDriver")
37+
38+
override fun execute(
39+
identifier: Int?,
40+
sql: String,
41+
parameters: Int,
42+
binders: (SqlPreparedStatement.() -> Unit)?,
43+
) = throw IOException("NullSqlDriver")
44+
45+
override fun newTransaction() = throw IOException("NullSqlDriver")
46+
47+
override fun currentTransaction() = throw IOException("NullSqlDriver")
48+
49+
override fun addListener(vararg queryKeys: String, listener: Query.Listener) = Unit
50+
51+
override fun removeListener(vararg queryKeys: String, listener: Query.Listener) = Unit
52+
53+
override fun notifyListeners(vararg queryKeys: String) = Unit
54+
55+
override fun close() = Unit
56+
}

zipline-loader/src/commonMain/kotlin/app/cash/zipline/loader/ZiplineCache.kt

Lines changed: 40 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import okio.ByteString.Companion.decodeHex
2727
import okio.Closeable
2828
import okio.FileNotFoundException
2929
import okio.FileSystem
30-
import okio.IOException
3130
import okio.Path
3231

3332
/**
@@ -44,15 +43,15 @@ import okio.Path
4443
* If multiple threads in a single process operate on a cache instance simultaneously, downloads may
4544
* be repeated but no thread will be blocked.
4645
*/
47-
class ZiplineCache internal constructor(
46+
class ZiplineCache private constructor(
4847
private val driver: SqlDriver,
4948
private val database: Database,
5049
private val fileSystem: FileSystem,
5150
private val directory: Path,
5251
private val maxSizeInBytes: Long,
5352
private val loaderEventListener: LoaderEventListener,
53+
private var hasWriteFailures: Boolean,
5454
) : Closeable {
55-
private var hasWriteFailures = false
5655

5756
/*
5857
* Files are named by their SHA-256 hashes. We use a SQLite database for file metadata: which
@@ -192,7 +191,7 @@ class ZiplineCache internal constructor(
192191
fileSystem.read(path) {
193192
readByteString()
194193
}
195-
} catch (e: FileNotFoundException) {
194+
} catch (_: FileNotFoundException) {
196195
null // Might have been pruned while we were trying to read?
197196
}
198197

@@ -201,19 +200,14 @@ class ZiplineCache internal constructor(
201200
try {
202201
fileSystem.delete(path)
203202
database.filesQueries.delete(metadata.id)
204-
} catch (ignored: Exception) {
203+
} catch (_: Exception) {
205204
}
206205
return null
207206
}
208207

209208
return result
210209
}
211210

212-
internal fun pin(applicationName: String, sha256: ByteString) {
213-
val fileId = database.filesQueries.get(sha256.hex()).executeAsOneOrNull()?.id ?: return
214-
createPinIfNotExists(applicationName, fileId)
215-
}
216-
217211
internal fun unpin(applicationName: String, sha256: ByteString) {
218212
val fileId = database.filesQueries.get(sha256.hex()).executeAsOneOrNull()?.id ?: return
219213
database.pinsQueries.delete_pin(applicationName, fileId)
@@ -361,11 +355,11 @@ class ZiplineCache internal constructor(
361355
}
362356

363357
private fun createPinIfNotExists(
364-
application_name: String,
365-
file_id: Long,
358+
applicationName: String,
359+
fileId: Long,
366360
) {
367-
database.pinsQueries.get_pin(file_id, application_name).executeAsOneOrNull()
368-
?: database.pinsQueries.create_pin(file_id, application_name)
361+
database.pinsQueries.get_pin(fileId, applicationName).executeAsOneOrNull()
362+
?: database.pinsQueries.create_pin(fileId, applicationName)
369363
}
370364

371365
/**
@@ -414,7 +408,7 @@ class ZiplineCache internal constructor(
414408
*
415409
* It will also delete dirty files that were open when the previous run completed.
416410
*/
417-
internal fun initialize() {
411+
private fun initialize() {
418412
try {
419413
deleteDirtyFiles()
420414
prune()
@@ -459,9 +453,7 @@ class ZiplineCache internal constructor(
459453
/** Returns the number of pins in the cache DB. */
460454
internal fun countPins() = database.pinsQueries.count().executeAsOne().toInt()
461455

462-
private fun path(metadata: Files): Path {
463-
return directory / "entry-${metadata.id}.bin"
464-
}
456+
private fun path(metadata: Files): Path = directory / "entry-${metadata.id}.bin"
465457

466458
/**
467459
* Update file record freshAt timestamp to reflect that the manifest is still seen as fresh.
@@ -490,26 +482,35 @@ class ZiplineCache internal constructor(
490482
loaderEventListener.cacheStorageFailed(applicationName, e)
491483
}
492484
}
493-
}
494485

495-
internal fun ZiplineCache(
496-
sqlDriverFactory: SqlDriverFactory,
497-
fileSystem: FileSystem,
498-
directory: Path,
499-
maxSizeInBytes: Long,
500-
loaderEventListener: LoaderEventListener,
501-
): ZiplineCache {
502-
fileSystem.createDirectories(directory, mustCreate = false)
503-
val driver: SqlDriver = sqlDriverFactory.create(directory / "zipline.db", Database.Schema)
504-
val database = createDatabase(driver = driver)
505-
val cache = ZiplineCache(
506-
driver = driver,
507-
database = database,
508-
fileSystem = fileSystem,
509-
directory = directory,
510-
maxSizeInBytes = maxSizeInBytes,
511-
loaderEventListener = loaderEventListener,
512-
)
513-
cache.initialize()
514-
return cache
486+
internal companion object {
487+
internal operator fun invoke(
488+
sqlDriverFactory: SqlDriverFactory,
489+
fileSystem: FileSystem,
490+
directory: Path,
491+
maxSizeInBytes: Long,
492+
loaderEventListener: LoaderEventListener,
493+
): ZiplineCache {
494+
val (driver, hasWriteFailures) = try {
495+
fileSystem.createDirectories(directory, mustCreate = false)
496+
sqlDriverFactory.create(directory / "zipline.db", Database.Schema) to false
497+
} catch (e: Exception) {
498+
loaderEventListener.cacheStorageFailed(null, e)
499+
NullSqlDriver() to true
500+
}
501+
502+
val database = createDatabase(driver = driver)
503+
val cache = ZiplineCache(
504+
driver = driver,
505+
database = database,
506+
fileSystem = fileSystem,
507+
directory = directory,
508+
maxSizeInBytes = maxSizeInBytes,
509+
loaderEventListener = loaderEventListener,
510+
hasWriteFailures = hasWriteFailures,
511+
)
512+
cache.initialize()
513+
return cache
514+
}
515+
}
515516
}

zipline-loader/src/commonMain/kotlin/app/cash/zipline/loader/internal/cache/database.kt

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import app.cash.sqldelight.db.SqlDriver
2121
import app.cash.sqldelight.db.SqlSchema
2222
import okio.Path
2323

24-
internal expect class SqlDriverFactory {
24+
internal interface SqlDriverFactory {
2525
/**
2626
* Create a SqlDriver to be used in creating and managing a SqlLite instance on disk.
2727
*
@@ -39,11 +39,9 @@ internal fun validateDbPath(path: Path) {
3939
}
4040
}
4141

42-
internal fun createDatabase(driver: SqlDriver): Database {
43-
return Database(
42+
internal fun createDatabase(driver: SqlDriver): Database = Database(
4443
driver,
4544
filesAdapter = Files.Adapter(
4645
file_stateAdapter = EnumColumnAdapter(),
4746
),
4847
)
49-
}

zipline-loader/src/commonTest/kotlin/app/cash/zipline/loader/internal/cache/CacheFaultsTester.kt

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package app.cash.zipline.loader.internal.cache
1818
import app.cash.sqldelight.db.QueryResult
1919
import app.cash.sqldelight.db.SqlDriver
2020
import app.cash.sqldelight.db.SqlPreparedStatement
21+
import app.cash.sqldelight.db.SqlSchema
2122
import app.cash.zipline.ZiplineManifest
2223
import app.cash.zipline.loader.LoaderEventListener
2324
import app.cash.zipline.loader.ZiplineCache
@@ -94,7 +95,7 @@ class CacheFaultsTester {
9495
* https://www.sqlite.org/tempfiles.html
9596
*/
9697
val fileNames: List<String>
97-
get() = fileSystem.list(directory)
98+
get() = (fileSystem.listOrNull(directory) ?: listOf())
9899
.map { it.name }
99100
.filterNot { it.startsWith("zipline.db-") }
100101

@@ -104,22 +105,9 @@ class CacheFaultsTester {
104105
*/
105106
var storageFailureCount = 0
106107

107-
init {
108-
fileSystem.createDirectories(directory)
109-
}
110-
111108
suspend fun withCache(block: suspend Session.() -> Unit) {
112-
val driver = LimitWritesSqlDriver(
113-
testSqlDriverFactory().create(
114-
path = directory / "zipline.db",
115-
schema = Database.Schema,
116-
),
117-
)
118-
val database = createDatabase(driver)
119-
120109
val cache = ZiplineCache(
121-
driver = driver,
122-
database = database,
110+
sqlDriverFactory = LimitWritesSqlDriverFactory(),
123111
fileSystem = LimitWritesFileSystem(fileSystem),
124112
directory = directory,
125113
maxSizeInBytes = cacheSize,
@@ -131,7 +119,6 @@ class CacheFaultsTester {
131119
)
132120

133121
try {
134-
cache.initialize()
135122
Session(cache).block()
136123
} finally {
137124
cache.close()
@@ -197,6 +184,13 @@ class CacheFaultsTester {
197184
private inner class LimitWritesFileSystem(
198185
delegate: FileSystem,
199186
) : ForwardingFileSystem(delegate) {
187+
override fun createDirectory(dir: Path, mustCreate: Boolean) {
188+
fileSystemWriteCount++
189+
if (fileSystemWriteCount >= fileSystemWriteLimit) throw IOException("write limit exceeded")
190+
191+
super.createDirectory(dir, mustCreate)
192+
}
193+
200194
override fun sink(file: Path, mustCreate: Boolean): Sink {
201195
fileSystemWriteCount++
202196
if (fileSystemWriteCount >= fileSystemWriteLimit) throw IOException("write limit exceeded")
@@ -229,6 +223,25 @@ class CacheFaultsTester {
229223
}
230224
}
231225

226+
private inner class LimitWritesSqlDriverFactory : SqlDriverFactory {
227+
override fun create(
228+
path: Path,
229+
schema: SqlSchema<QueryResult.Value<Unit>>,
230+
): SqlDriver {
231+
fileSystemWriteCount++
232+
if (fileSystemWriteCount >= fileSystemWriteLimit) {
233+
throw SqlFaultException("write limit exceeded")
234+
}
235+
236+
return LimitWritesSqlDriver(
237+
testSqlDriverFactory().create(
238+
path = directory / "zipline.db",
239+
schema = Database.Schema,
240+
),
241+
)
242+
}
243+
}
244+
232245
private inner class LimitWritesSqlDriver(
233246
private val delegate: SqlDriver,
234247
) : SqlDriver by delegate {

0 commit comments

Comments
 (0)