Skip to content

fix(seed-vault-rn): observer crash on API 24-29 and listener leak - #772

Open
Michaelembiring wants to merge 1 commit into
solana-mobile:mainfrom
Michaelembiring:fix/seed-vault-rn-observer-crash
Open

fix(seed-vault-rn): observer crash on API 24-29 and listener leak#772
Michaelembiring wants to merge 1 commit into
solana-mobile:mainfrom
Michaelembiring:fix/seed-vault-rn-observer-crash

Conversation

@Michaelembiring

Copy link
Copy Markdown

Problem

The android module of @solana-mobile/seed-vault-lib declares minSdkVersion 24, making all three legacy ContentObserver.onChange overloads reachable on API 24–29 devices.

  1. Runtime crash on API 24–29 — Legacy onChange overloads were TODO() calls that threw NotImplementedError whenever the platform invoked them instead of the API 30+ Collection<Uri> variant.
  2. ActivityEventListener leakregisterForActivityResult attached anonymous listeners to reactContext with no removal mechanism on timeout, completion, or module teardown.
  3. ContentObserver leak — Registered as an inline anonymous object with no retained reference to unregister on teardown.

Solution

  1. All four onChange overloads delegate to dispatchContentChange(uris). Legacy callbacks carrying no URI fall back to listOf(WalletContractV1.WALLET_PROVIDER_CONTENT_URI_BASE).
    • Note: Legacy callbacks provide no URI, so the fallback is the provider base URI rather than the specific changed URI available on API 30+.
  2. Pending activity result listeners are managed via mPendingListeners (ConcurrentHashMap) and removed exactly once via a per-listener AtomicBoolean (settled).
  3. cleanupResources() unregisters the observer and clears pending listeners. Guarded by an AtomicBoolean (cleanedUp) and wired to both invalidate() and onCatalystInstanceDestroy().

Verification

  • Single-file change scoped strictly to SolanaMobileSeedVaultLibModule.kt.
  • Verified via toolchain compilation generating .class bytecode artifacts without errors.

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.

1 participant