Skip to content

Fix StrictMode disk reads on main thread at app startup#7834

Open
aitorvs wants to merge 3 commits intodevelopfrom
fix/aitorvs/strictmode-disk-reads-startup
Open

Fix StrictMode disk reads on main thread at app startup#7834
aitorvs wants to merge 3 commits intodevelopfrom
fix/aitorvs/strictmode-disk-reads-startup

Conversation

@aitorvs
Copy link
Collaborator

@aitorvs aitorvs commented Feb 28, 2026

Task/Issue URL: https://app.asana.com/1/137249556945/project/1198194956794324/task/1213475367843818?focus=true

Description

Eliminates ~871ms of blocking disk I/O from the main thread at app startup by deferring three categories of StrictMode violations:

P0.1 — Subscriptions URL construction (~397ms)

  • InternalSubscriptionsBaseUrlModule: anonymous object's subscriptionsBaseUrl property changed to by lazy {}
  • RealSubscriptionsUrlProvider: all six URL vals changed to by lazy {} to prevent eager resolution at DI construction time

P0.2 — Room executor feature-flag read (~272ms)

  • RoomDatabaseProviderImpl.applyExecutors: now passes a LazyExecutor proxy to setQueryExecutor/setTransactionExecutor instead of resolving the executor immediately
  • LazyExecutor defers lazyDatabaseExecutorProvider.get() (and the SharedPreferences-backed DatabaseProviderFeature read) to the first actual DB query, which Room dispatches on a background thread

P0.3 — PIR SQLCipher native library loading (~202ms)

  • PirSecureStorageDatabaseFactory: removed init {} block that called LibraryLoader.loadLibrary("sqlcipher") at DI construction time; moved the call into getInnerDatabase() which runs under a Mutex on a background coroutine

Steps to test this PR

  • Build an internal build and verify no StrictMode violations for SharedPreferencesSubscriptionsInternalStore, RealPirSecureStorageDatabaseFactory, or RealDatabaseExecutorProvider on app launch
  • Verify autofill, subscriptions, and PIR features work normally

UI changes

No UI changes.


Note

Medium Risk
Moderate risk because it changes when and how database executors and SQLCipher library loading are initialized, which can affect runtime behavior under concurrency and feature-flag paths, though the changes are localized and covered by updated tests.

Overview
Reduces app-startup StrictMode disk reads by deferring several DI-time initializations until first use.

Room DB setup now always installs LazyExecutor proxies for query/transaction executors, delaying RealDatabaseExecutorProvider (and its feature-flag SharedPreferences read) until the first DB operation and falling back to a cached thread pool when the delegate resolves to null. PIR secure DB creation no longer loads the SQLCipher native library in an init block; it loads on demand in getInnerDatabase() on dispatchers.io(), while preserving coroutine cancellation.

Subscriptions URL generation is made lazy (by lazy) in both the internal base URL provider and RealSubscriptionsUrlProvider, avoiding store reads/URL construction during DI graph creation; related Room executor tests are updated to assert against the new lazy wrappers.

Written by Cursor Bugbot for commit 938b9af. This will update automatically on new commits. Configure here.

@aitorvs aitorvs force-pushed the fix/aitorvs/strictmode-disk-reads-startup branch from fb45cae to bcf17ff Compare February 28, 2026 17:10
internal val delegate: Executor? by lazy { init() }
private val fallback: Executor by lazy { Executors.newCachedThreadPool() }
override fun execute(command: Runnable) {
(delegate ?: fallback).execute(command)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LazyExecutor fallback replaces Room's bounded default with unbounded pools

Medium Severity

Previously when createQueryExecutor/createTransactionExecutor returned null (feature flag off + DatabaseExecutor.Default), no executor was set, so Room used its own bounded default (ArchTaskExecutor). Now a LazyExecutor is always set, and the null-delegate fallback is Executors.newCachedThreadPool() — an unbounded pool with no rejection policy, unlike Room's bounded default. Additionally, each LazyExecutor instance creates its own separate newCachedThreadPool. Since all production callers use the default executor, when the flag is off every database gets two separate unbounded pools (query + transaction). The KDoc claims this is "equivalent to Room's own background thread pool" but it's not.


Please tell me if this was useful or not with a 👍 or 👎.

Additional Locations (1)

Fix in Cursor Fix in Web

return _database
}

logcat { "PIR-DB: Loading the sqlcipher native library" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CDRussell actually has a fix for autofill secure storage that moves the sqlcipher lib loading logic out of the init block.

We also move the logic in the same block but with different way to handle the loading.

I am not 100% sure if just moving it here could cause any issues or not. Can we put this change behind a FF?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both approaches defer the load to first DB access — neither pre-loads at startup. The autofill complexity stems from using the async loadLibrary overload, which is callback-based: you need a CompletableDeferred to bridge the callback to a coroutine, and a timeout because the raw Thread it spawns is outside coroutine control. Feature flag and pixels sit on top of that.

For PIR we can do this cleanly: sync overload + withContext(dispatchers.io()) achieves the same guarantee with none of that machinery — no callback, no CompletableDeferred, no timeout needed since exceptions from the IO thread propagate normally. I've added that withContext wrap in this PR.

I don't think a feature flag is needed here — this is a correctness fix, not a behaviour change.

@aitorvs aitorvs force-pushed the fix/aitorvs/strictmode-disk-reads-startup branch from df67699 to c73baf9 Compare March 2, 2026 21:07
Copy link
Collaborator Author

aitorvs commented Mar 2, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: Catching Throwable swallows CancellationException breaking structured concurrency
    • The catch block now rethrows CancellationException so coroutine cancellation propagates instead of being swallowed during SQLCipher library loading.

Create PR

Or push these changes by commenting:

@cursor push 4cf059712a
Preview (4cf059712a)
diff --git a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/secure/PirSecureStorageDatabaseFactory.kt b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/secure/PirSecureStorageDatabaseFactory.kt
--- a/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/secure/PirSecureStorageDatabaseFactory.kt
+++ b/pir/pir-impl/src/main/java/com/duckduckgo/pir/impl/store/secure/PirSecureStorageDatabaseFactory.kt
@@ -24,6 +24,7 @@
 import com.duckduckgo.pir.impl.store.PirDatabase
 import com.squareup.anvil.annotations.ContributesBinding
 import dagger.SingleInstanceIn
+import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
@@ -72,6 +73,7 @@
             }
             logcat { "PIR-DB: sqlcipher native library loaded ok" }
         } catch (t: Throwable) {
+            if (t is CancellationException) throw t
             // error loading the library
             logcat(ERROR) { "PIR-DB: Error loading sqlcipher library: ${t.asLog()}" }
         }

@aitorvs aitorvs force-pushed the fix/aitorvs/strictmode-disk-reads-startup branch from c73baf9 to fc05a9d Compare March 2, 2026 21:51
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.

@aitorvs aitorvs force-pushed the fix/aitorvs/strictmode-disk-reads-startup branch from fc05a9d to fc101ac Compare March 2, 2026 22:14
aitorvs and others added 3 commits March 3, 2026 21:54
- Defer subscriptions base URL SharedPreferences read: make
  `subscriptionsBaseUrl` lazy in `InternalSubscriptionsBaseUrlModule`
  and all URL properties lazy in `RealSubscriptionsUrlProvider`, so the
  read happens on the background thread that first makes a subscription
  API call instead of at DI graph construction time (~397ms saving).

- Move PIR sqlcipher native library load out of `RealPirSecureStorageDatabaseFactory`
  `init {}` block into `getInnerDatabase()`, which is called from a
  suspend function behind a Mutex. The library is now loaded on the
  coroutine dispatcher of the first PIR DB access rather than at DI
  construction time (~202ms saving).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
RealDatabaseExecutorProvider's constructor reads a SharedPreferences-backed
feature flag (DatabaseProviderFeature), causing a ~272ms StrictMode disk-read
violation at app startup. The root cause is that applyExecutors called
lazyDatabaseExecutorProvider.get() eagerly during DI graph construction on
the main thread.

Fix: wrap the query and transaction executors in a LazyExecutor proxy whose
delegate is resolved on the first execute() call — which Room dispatches on
a background thread, not during Application.onCreate.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wrap LibraryLoader.loadLibrary in withContext(dispatchers.io()) so the
blocking System.loadLibrary call always runs on a background thread,
regardless of the caller's dispatcher.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@aitorvs aitorvs force-pushed the fix/aitorvs/strictmode-disk-reads-startup branch from fc101ac to 938b9af Compare March 3, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants