@W-24269724: Clean up Android refresh state on logout

[wmathurin]

Description

Removes Android's process-local refresh coordination state when an account session ends.

• Removes only the departing account's RefreshState from the coordination map during SDK cleanup.
• Keeps the remover package-private and exposes it across SDK packages through an existing module-internal bridge pattern.
• Uses per-state lifecycle leases so logout cannot unlink an active coordinator and allow a quick same-identity login to create a second refresh winner.
• Scrubs published credential fields, including the DPoP UI session ID, immediately; removes idle state immediately; and defers active-state removal until the final caller exits.
• Makes new callers fail closed while cleanup is pending and prevents an in-flight winner from republishing a result after cleanup.
• Fences post-response persistence, logout, RTR registration, and broadcasts under the cleanup lock so an old response cannot affect a quickly relogged same-identity account.
• Revalidates immediately after state acquisition so account removal cannot recreate an orphaned entry.
• Adds direct account-isolation, absent-state, logout integration, election-race, and logout/relogin concurrency regressions without logging account or token material.
• Updates the Android token-lifecycle auth documentation with the cleanup lifecycle.

Spec: https://git.soma.salesforce.com/SalesforceMobileSDK/SalesforceMobileSDK-Workspace/pull/113

Resolves

W-24269724

Testing

Tests in the PR

• Manual Tests
• Unit Tests
• Integration Tests

Validation

Validation ran on a Pixel 9 Pro XL API 36 emulator after merging current upstream dev:

• :libs:SalesforceSDK:build :libs:SalesforceSDK:assembleAndroidTest passed, including lint (173 tasks).
• Complete ClientManagerMockTest, SalesforceSDKManagerClientManagerTest, and RestClientDPoPGateTests classes passed: 66 tests, 0 failures.
• Deterministic logout/in-flight-refresh/quick-relogin regressions verify one coordinator, no overlapping token POSTs, deferred removal, UI-session-ID scrubbing, no stale success or invalid_grant side effects on a recreated backing Android Account, and a successful fresh-state retry.
• git diff --check passed.

[JohnsonEricAtSalesforce]

Traced the revalidate-after-election logic against both orderings of the logout-vs-computeIfAbsent race this change targets. The logout-then-refresh-arrives ordering was already closed by pre-existing validation and doesn't need this fix. The refresh-arrives-then-logout ordering (the actual gap) is closed for the orphaned-entry case — I could not construct an ordering that leaves a dangling entry.

However, tracing further with multiple concurrent providers for the same identity (which this file's own coordination design assumes — see the class comment on AccMgrAuthTokenProvider) surfaced a different, more consequential race: the same recheck-and-remove can unlink a RefreshState that a second provider has already started relying on, rather than one that's genuinely orphaned. I left the detailed trace as an inline comment on the remove call. The net effect is that this fix, while closing the leak the linked bug report describes, opens a narrower but more severe window for a split refresh under logout/relogin churn — which is a plausible pattern for the multi-account, long-lived-process consumers the bug report itself names as affected.

Given that the underlying leak is explicitly characterized as small and bounded on its own, it may be worth weighing whether removing the entry at this point in the code is worth the tradeoff, versus accepting the rarer residual leak here and closing it some other way that doesn't touch a key while another provider could be mid-election on it.

Two smaller items, not blocking:

• The recheck adds a second full getValidatedUser(true) call (AccountManager existence check plus decrypt) back-to-back with the one at the winner's actual refresh a few lines later. Given the account-resolution caching regression fixed elsewhere in this codebase recently, calling that out so it gets a deliberate look rather than being absorbed silently — this path is far less exposed than that regression was, since it's refresh-only rather than per-request.
• clearRefreshState's null guard on user is unreachable from its only call site, which passes a non-null value — harmless, just dead code for this call graph.

Everything else held up: removal is correctly scoped to only the departing account's key, an in-flight thread holding its own RefreshState reference is unaffected by removal from the map, this hooks into the same cleanup choke point used for the existing client-manager cache invalidation rather than a parallel path, and the Kotlin/Java bridge reuses the existing module-internal pattern. The new/changed tests exercise real behavior without mocking the class under test, and the election-race test forces the relevant call sequence deterministically rather than relying on timing.

This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.

[JohnsonEricAtSalesforce]

Verified both fixes directly against the committed diffs, not just the commit messages.

c48ba9e80 (the first fix): the post-election revalidation in getNewAuthToken() no longer removes the shared RefreshState on a failed recheck — it fails closed with a bare return instead, so a concurrent provider that already grabbed a reference can't have it unlinked out from under it. The accompanying test was renamed and now asserts the state stays mapped and that no HTTP call is made.

e4d67cdb9 (the second fix): clearRefreshState now defers actual removal behind a lifecycle lease (activeCallers / cleanupRequested) instead of removing unconditionally. Logout scrubs the published credentials and marks the state for cleanup immediately, but only unlinks it once every active caller has released its lease. A same-identity relogin that arrives while the old state is still leased sees cleanupRequested and fails closed rather than creating a second, independent coordinator. I traced the remaining orderings — a loser parked in the wait loop when cleanup fires, a winner finishing its POST after cleanup has already started, and a fresh computeIfAbsent racing the deferred removal — and each one either fails closed or resolves onto a single coordinator; none leaves two live, independently-refreshing states for the same identity. The new testClearRefreshState_DefersRemovalUntilInFlightCallerDrains test is deterministic (latch-based, no sleep) and exercises exactly that scenario: the relogin attempt blocks with zero additional token-endpoint calls while the old refresh is in flight, and only gets its own single refresh after the old caller drains.

Both fixes read as closing the hazard rather than narrowing it, CI is green on the current head, and I don't see a remaining correctness gap in this area. Approving.

This response was generated by an AI agent on behalf of @JohnsonEricAtSalesforce.