Add sqlcipher-loader module and migrate PIR/autofill#8009
Add sqlcipher-loader module and migrate PIR/autofill#8009karlenDimla merged 8 commits intodevelopfrom
Conversation
3fae854 to
5a37b6d
Compare
5b98aac to
d87f207
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed:
runCatchingswallowsCancellationExceptionbreaking structured concurrencywaitForLibraryLoad()now uses explicit try/catch that rethrowsCancellationExceptionand only wraps non-cancellation failures inResult.failure.
- ✅ Fixed: Missing ATB parameter removal for migrated pixel
- Added a
PixelParamRemovalPlugincontribution insqlcipher-loader-implto remove ATB fromlibrary_load_failure_sqlcipherbefore sending.
- Added a
Or push these changes by commenting:
@cursor push 6a071da47a
Preview (6a071da47a)
diff --git a/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt b/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
--- a/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
+++ b/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
@@ -24,6 +24,8 @@
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Daily
import com.duckduckgo.common.utils.DispatcherProvider
+import com.duckduckgo.common.utils.plugins.pixel.PixelParamRemovalPlugin
+import com.duckduckgo.common.utils.plugins.pixel.PixelParamRemovalPlugin.PixelParameter
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.library.loader.LibraryLoader
import com.duckduckgo.sqlcipher.loader.api.SqlCipherLoader
@@ -31,6 +33,7 @@
import com.squareup.anvil.annotations.ContributesBinding
import com.squareup.anvil.annotations.ContributesMultibinding
import dagger.SingleInstanceIn
+import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
@@ -75,7 +78,14 @@
if (!libraryLoaded.isCompleted) {
appCoroutineScope.launch(dispatchers.io()) { doLoad() }
}
- return runCatching { libraryLoaded.await() }
+ return try {
+ libraryLoaded.await()
+ Result.success(Unit)
+ } catch (cancellationException: CancellationException) {
+ throw cancellationException
+ } catch (throwable: Throwable) {
+ Result.failure(throwable)
+ }
}
private fun doLoad() {
@@ -100,3 +110,15 @@
enum class SqlCipherPixelName(override val pixelName: String) : Pixel.PixelName {
LIBRARY_LOAD_FAILURE_SQLCIPHER("library_load_failure_sqlcipher"),
}
+
+@ContributesMultibinding(
+ scope = AppScope::class,
+ boundType = PixelParamRemovalPlugin::class,
+)
+object SqlCipherPixelsRequiringDataCleaning : PixelParamRemovalPlugin {
+ override fun names(): List<Pair<String, Set<PixelParameter>>> {
+ return listOf(
+ LIBRARY_LOAD_FAILURE_SQLCIPHER.pixelName to PixelParameter.removeAtb(),
+ )
+ }
+}
...cipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
Outdated
Show resolved
Hide resolved
autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/impl/pixel/AutofillPixelNames.kt
Show resolved
Hide resolved
d87f207 to
a00ef9f
Compare
|
Tested and everything looks good for me:
|
Introduces a dedicated sqlcipher-loader-api/impl module that owns SQLCipher native library loading. RealSqlCipherLoader pre-warms the library eagerly at app startup (via MainProcessLifecycleObserver) on the IO dispatcher, and exposes a single waitForLibraryLoad() API backed by a CompletableDeferred — eliminating concurrent load races. Fires LIBRARY_LOAD_FAILURE_SQLCIPHER (daily pixel) on load failure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace direct LibraryLoader calls and the autofill-local SqlCipherLibraryLoader with the new shared SqlCipherLoader API. - pir-impl: inject SqlCipherLoader, remove direct LibraryLoader call - autofill-impl: inject SqlCipherLoader, delete SqlCipherLibraryLoader and its test, remove sqlCipherAsyncLoading feature flag, remove LIBRARY_LOAD_TIMEOUT/FAILURE_SQLCIPHER pixels (failure pixel now lives in sqlcipher-loader-impl) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: No guard prevents concurrent
doLoad()native library loading- Added an AtomicBoolean compare-and-set guard in doLoad() so only one native sqlcipher load can be started across all call sites.
Or push these changes by commenting:
@cursor push 5d5d97d9d9
Preview (5d5d97d9d9)
diff --git a/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt b/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
--- a/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
+++ b/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
@@ -38,6 +38,7 @@
import kotlinx.coroutines.TimeoutCancellationException
import kotlinx.coroutines.launch
import kotlinx.coroutines.withTimeout
+import java.util.concurrent.atomic.AtomicBoolean
import logcat.LogPriority.ERROR
import logcat.logcat
import javax.inject.Inject
@@ -54,6 +55,7 @@
) : SqlCipherLoader, MainProcessLifecycleObserver, PirProcessLifecycleObserver {
private val libraryLoaded = CompletableDeferred<Unit>()
+ private val loadStarted = AtomicBoolean(false)
// Eagerly kick off the load at app startup, well before autofill or PIR need it.
override fun onCreate(owner: LifecycleOwner) {
@@ -98,6 +100,9 @@
}
private fun doLoad() {
+ if (!loadStarted.compareAndSet(false, true)) {
+ return
+ }
logcat { "SqlCipher: starting async library load" }
LibraryLoader.loadLibrary(
context,
@@ -110,7 +115,6 @@
override fun failure(throwable: Throwable) {
logcat(ERROR) { "SqlCipher: native library load failed: ${throwable.javaClass.simpleName} - ${throwable.message}" }
- // Guard ensures the pixel fires exactly once even if doLoad() races.
if (libraryLoaded.completeExceptionally(throwable)) {
pixel.fire(LIBRARY_LOAD_FAILURE_SQLCIPHER, type = Daily())
}
...cipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
Show resolved
Hide resolved
landomen
left a comment
There was a problem hiding this comment.
Checked autofill and PIR flows, including background workers, works as expected.
d5897c3 to
c89b863
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Missing try-catch in doLoad leaves deferred permanently pending
- Wrapped the synchronous
LibraryLoader.loadLibraryinvocation in atry/catchthat completes the deferred exceptionally and fires the failure pixel so waiters fail immediately instead of timing out.
- Wrapped the synchronous
Or push these changes by commenting:
@cursor push 8a17146051
Preview (8a17146051)
diff --git a/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt b/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
--- a/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
+++ b/sqlcipher-loader/sqlcipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
@@ -103,23 +103,30 @@
private fun doLoad() {
logcat { "SqlCipher: starting async library load" }
- LibraryLoader.loadLibrary(
- context,
- SQLCIPHER_LIB_NAME,
- object : LibraryLoader.LibraryLoaderListener {
- override fun success() {
- logcat { "SqlCipher: native library loaded successfully" }
- libraryLoaded.complete(Unit)
- }
+ try {
+ LibraryLoader.loadLibrary(
+ context,
+ SQLCIPHER_LIB_NAME,
+ object : LibraryLoader.LibraryLoaderListener {
+ override fun success() {
+ logcat { "SqlCipher: native library loaded successfully" }
+ libraryLoaded.complete(Unit)
+ }
- override fun failure(throwable: Throwable) {
- logcat(ERROR) { "SqlCipher: native library load failed: ${throwable.javaClass.simpleName} - ${throwable.message}" }
- if (libraryLoaded.completeExceptionally(throwable)) {
- pixel.fire(LIBRARY_LOAD_FAILURE_SQLCIPHER, type = Daily())
+ override fun failure(throwable: Throwable) {
+ logcat(ERROR) { "SqlCipher: native library load failed: ${throwable.javaClass.simpleName} - ${throwable.message}" }
+ if (libraryLoaded.completeExceptionally(throwable)) {
+ pixel.fire(LIBRARY_LOAD_FAILURE_SQLCIPHER, type = Daily())
+ }
}
- }
- },
- )
+ },
+ )
+ } catch (throwable: Throwable) {
+ logcat(ERROR) { "SqlCipher: native library load setup failed: ${throwable.javaClass.simpleName} - ${throwable.message}" }
+ if (libraryLoaded.completeExceptionally(throwable)) {
+ pixel.fire(LIBRARY_LOAD_FAILURE_SQLCIPHER, type = Daily())
+ }
+ }
}
private companion object {
...cipher-loader-impl/src/main/java/com/duckduckgo/sqlcipher/loader/impl/RealSqlCipherLoader.kt
Show resolved
Hide resolved
CDRussell
left a comment
There was a problem hiding this comment.
LGTM.
Tested scenarios where autofill wouldn't be available due to both:
sqlcipherlibrary failuresqlcipherlibrary timeout
Nit: this PR has thrown out tests that would still be useful to have that were captured in SqlCipherLibraryLoaderTest. I think Claude would easily port them over for you.



Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1213721049904377?focus=true
Description
Introduces a dedicated
sqlcipher-loadermodule that centralises SQLCipher native library loading and fixes a JNI deadlock causing a 7x spike inLIBRARY_LOAD_TIMEOUT_SQLCIPHER(350 → 2,500/day).Root cause: removing PIR's eager
System.loadLibrary("sqlcipher")call (f6c879e) inadvertently removed the pre-warm that autofill depended on. Autofill'sSqlCipherLibraryLoaderthen performed the first full load on a background thread, and when PIR's lazy load raced concurrently, a JNI loading deadlock could occur — triggering the 10s timeout.Fix:
sqlcipher-loader-apimodule exposes aSqlCipherLoaderinterface with a singlewaitForLibraryLoad(): Result<Unit>API.sqlcipher-loader-implprovidesRealSqlCipherLoader, which:MainProcessLifecycleObserverto eagerly pre-warm SQLCipher on the IO dispatcher at app startup, before autofill or PIR need it.CompletableDeferred<Unit>(initialised at construction) so all callers share the same load — concurrentcomplete()calls are no-ops, making the race structurally impossible.LIBRARY_LOAD_FAILURE_SQLCIPHER(daily pixel) if the load throws.SqlCipherLoaderand callwaitForLibraryLoad()instead of loading independently.SqlCipherLibraryLoader(autofill-local) and its test deleted.sqlCipherAsyncLoadingfeature flag andLIBRARY_LOAD_TIMEOUT_SQLCIPHERpixel removed — no timeout needed when the load is predictable and early.Steps to test this PR
SQLCipher loads correctly
SqlCipher: native library loaded successfullyappearing once at startup (not twice, not on demand)No regression on autofill
LIBRARY_LOAD_TIMEOUT_SQLCIPHERorLIBRARY_LOAD_FAILURE_SQLCIPHERpixels fire under normal conditionsVerify build
./gradlew :sqlcipher-loader-impl:testDebugUnitTest./gradlew :autofill-impl:testDebugUnitTest./gradlew :pir-impl:testDebugUnitTestSQLCipher loads correctly on PIR process
Note
Medium Risk
Moderate risk because it changes when/how the SQLCipher native library is loaded and gates creation of encrypted databases for both Autofill and PIR, which can impact data access and startup behavior.
Overview
Centralizes SQLCipher native library loading into a new
sqlcipher-loadermodule (SqlCipherLoaderAPI +RealSqlCipherLoaderimpl) that eagerly starts async loading via process lifecycle observers and provides a sharedwaitForLibraryLoadfor all callers (with timeout/failure pixels).Migrates Autofill and PIR secure DB factories to inject
SqlCipherLoaderinstead of doing their own loads, deleting Autofill’s localSqlCipherLibraryLoaderand its feature flag (sqlCipherAsyncLoading) and moving the SQLCipher load pixels out ofautofill.json5intosqlcipher_loader.json5. The app wiring is updated to depend on the new modules and to strip ATB params for the new SQLCipher pixels.Written by Cursor Bugbot for commit 33c8a5f. This will update automatically on new commits. Configure here.