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:
- Partially-provisioned account. The row is added during login before base-url is written. Anything that fires a retry in that window crashes.
- 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
- Catch
AccountNotFoundException inside retryUploads (park/drop the affected uploads, log), or guard createOwnCloudClient with an account-exists check.
- Add a
CoroutineExceptionHandler to ioScope as a backstop.
- 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.
Summary
FileUploadHelper.retryUploadsruns on aCoroutineScope(Dispatchers.IO)with noCoroutineExceptionHandler. WhencreateOwnCloudClientthrowsAccountUtils.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)
Mechanism (read from the code plus the pinned library)
AccountUtils.getBaseUrlForAccountthrowsAccountNotFoundExceptiononly whenAccountManager.getUserData(account, "oc_base_url")returnsnull. (confirmed in the bytecode ofandroid-library@72a95a99, the pinned commit)retryUploadsreaches it viacreateOwnCloudClient, 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,isAnonymousis true, and the crashing call is skipped. What it needs is an account row that exists while itsoc_base_urluser-data reads backnull. Two ways that can happen, both hypotheses for production:getCurrentAccount()snapshotsgetAccounts(), thengetUserData()runs later onDispatchers.IO. If the account is removed in between,getUserDatareturnsnull. (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 noCoroutineExceptionHandler, so any escape fromioScope.launchkills the process. ASupervisorJobalone would not fix this.retryFailedUploadsSemaphoreis released in afinallythat runs whenioScope.launchreturns, not when the coroutine completes. So the "already running" guard never guards the body, and a connectivity flap can run severalretryUploadsconcurrently. That widens the race in (2).Why it matters
The retry fires passively, with no user interaction. Sources:
MainAppinit (MainApp.java:628), the connectivity receiver onCONNECTIVITY_CHANGE/WIFI_STATE_CHANGEwhen connected,ACTION_POWER_CONNECTED, and the power-save receiver (allReceiversHelper.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:DrawerActivityIT#switchAccountViaAccountListReceiveExternalFilesActivityITLauncherActivityITTwo of the three classes (
LauncherActivityIT,ReceiveExternalFilesActivityIT) carry noGrantPermissionRule. 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
AccountNotFoundExceptioninsideretryUploads(park/drop the affected uploads, log), or guardcreateOwnCloudClientwith an account-exists check.CoroutineExceptionHandlertoioScopeas a backstop.Verification: a 15-iteration
DrawerActivityIT#switchAccountViaAccountListloop 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.