Skip to content

Crash: uncaught AccountNotFoundException in FileUploadHelper.retryUploads kills the app process #17337

Description

@andreBurnt

Summary

FileUploadHelper.retryUploads runs on a CoroutineScope(Dispatchers.IO) with no CoroutineExceptionHandler. When createOwnCloudClient throws AccountUtils.AccountNotFoundException, nothing catches it. The exception reaches the default handler and the app process dies.

The retry is triggered passively (app start, connectivity change, charger plug-in, power-save toggle), so no user interaction is needed.

I found this while running a diagnostic pass over the public CI history plus a debug build from source. It reproduces on-device. The production trigger is narrower, and I've labelled those parts as hypotheses below rather than overstate them.

Stack (representative, 5 captured logcats, all identical)

FATAL EXCEPTION: DefaultDispatcher-worker-N
  AccountUtils$AccountNotFoundException: Account not found
    at AccountUtils.getBaseUrlForAccount(AccountUtils.java:72)
    at OwnCloudClientFactory.createOwnCloudClient(OwnCloudClientFactory.java:72)
    at FileUploadHelper$retryUploads$2.invokeSuspend(FileUploadHelper.kt:194)
    at FileUploadHelper.retryUploads(FileUploadHelper.kt:180)
    at FileUploadHelper$retryFailedUploads$1(FileUploadHelper.kt:141)

Mechanism (read from the code plus the pinned library)

AccountUtils.getBaseUrlForAccount throws AccountNotFoundException only when AccountManager.getUserData(account, "oc_base_url") returns null. (confirmed in the bytecode of android-library@72a95a99, the pinned commit)

retryUploads reaches it via createOwnCloudClient, behind a single guard: if (!currentAccount.isAnonymous(context)).

Not the obvious path. "The user removed their account" does not trigger this. With zero accounts, getCurrentAccount() returns the anonymous account, isAnonymous is true, and the crashing call is skipped. What it needs is an account row that exists while its oc_base_url user-data reads back null. Two ways that can happen, both hypotheses for production:

  1. Partially-provisioned account. The row is added during login before base-url is written. Anything that fires a retry in that window crashes.
  2. Removal race (TOCTOU). getCurrentAccount() snapshots getAccounts(), then getUserData() runs later on Dispatchers.IO. If the account is removed in between, getUserData returns null. (the worst crasher on my bench was the account-switching test)

Two aggravating details in the same function

ioScope = CoroutineScope(Dispatchers.IO) (FileUploadHelper.kt:77) has no CoroutineExceptionHandler, so any escape from ioScope.launch kills the process. A SupervisorJob alone would not fix this.

retryFailedUploadsSemaphore is released in a finally that runs when ioScope.launch returns, not when the coroutine completes. So the "already running" guard never guards the body, and a connectivity flap can run several retryUploads concurrently. That widens the race in (2).

Why it matters

The retry fires passively, with no user interaction. Sources: MainApp init (MainApp.java:628), the connectivity receiver on CONNECTIVITY_CHANGE / WIFI_STATE_CHANGE when connected, ACTION_POWER_CONNECTED, and the power-save receiver (all ReceiversHelper.java). A user in either state above who walks onto Wi-Fi or plugs in a charger loses the app process.

How reproducible

Bench batteries against a debug build. 17 iterations ended in Process crashed, across three test classes on two API levels:

Test class Device Crashed / iterations
DrawerActivityIT#switchAccountViaAccountList API-28 emulator 11 / 15 (0 passes)
ReceiveExternalFilesActivityIT API-29 emulator 4 / 25
LauncherActivityIT API-29 emulator 2 / 25

Two of the three classes (LauncherActivityIT, ReceiveExternalFilesActivityIT) carry no GrantPermissionRule. So this is independent of the CI permission-grant failures, not a downstream effect of them. In CI history it hides inside generic "Process crashed" noise.

Note on overlap with #17268: that PR modifies these three test classes, but it does not touch the production file (FileUploadHelper.kt). The crash path is orthogonal to the test fixes.

Suggested fix

  1. Catch AccountNotFoundException inside retryUploads (park/drop the affected uploads, log), or guard createOwnCloudClient with an account-exists check.
  2. Add a CoroutineExceptionHandler to ioScope as a backstop.
  3. Release the semaphore when the coroutine completes, not when it's launched.

Verification: a 15-iteration DrawerActivityIT#switchAccountViaAccountList loop on an API-28 AVD. Currently 11/15 process deaths, expect 0.

Evidence

Logcats plus iteration ledgers available on request. 5 full stacks captured, plus the per-loop ledgers. Happy to attach or link whatever's most useful.

cc @tobiasKaminsky, filing per your reply on the CI thread.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions