Skip to content

fix(api): don't crash when the service connects after the API was closed#1023

Open
MiMoHo wants to merge 1 commit into
nextcloud:mainfrom
MiMoHo:fix/close-connect-race
Open

fix(api): don't crash when the service connects after the API was closed#1023
MiMoHo wants to merge 1 commit into
nextcloud:mainfrom
MiMoHo:fix/close-connect-race

Conversation

@MiMoHo

@MiMoHo MiMoHo commented Jul 8, 2026

Copy link
Copy Markdown

fix(api): don't crash when the service connects after the API was closed

Problem

Host apps using this library keep crashing with:

java.lang.NullPointerException: Attempt to invoke interface method
'void com.nextcloud.android.sso.api.NextcloudAPI$ApiConnectedListener.onConnected()'
on a null object reference
    at com.nextcloud.android.sso.api.AidlNetworkRequest$1.onServiceConnected(AidlNetworkRequest.java:69)

Reported for example in nextcloud/notes-android#2622 (labeled "Upstream :: SSO").

Root cause

NextcloudAPI starts binding to the AccountManagerService asynchronously.
When the consumer calls NextcloudAPI#close() before the connection is
established (e.g. after a request error invalidates a cached API instance),
NetworkRequest#close() nulls the ApiConnectedListener — but
AidlNetworkRequest#unbindService() only called Context#unbindService()
when the connection was already established (mBound == true). The pending
ServiceConnection therefore stayed registered with the framework and later
delivered onServiceConnected() on the main thread, which invoked
onConnected() on the nulled callback and crashed the host app.

Fix

  • Track whether a binding has been requested and always unbind in that case,
    so a not-yet-established connection is released on close() as well.
  • Guard onServiceConnected() against the remaining race where close()
    runs on another thread while the connection callback is already being
    dispatched: capture the callback locally and ignore stale connections.

Tests

Three new unit tests in AidlNetworkRequestTest:

  • the regular flow still invokes the callback,
  • a stale onServiceConnected() after close() no longer throws
    (fails with exactly the reported NPE without the fix),
  • close() before the connection is established unbinds the
    ServiceConnection (fails without the fix).

unitTests.returnDefaultValues = true was enabled so the plain JVM tests can
run connect(), which touches Intent/ComponentName. All existing tests
and the CI checks (test, assembleDebug, lib:lint, lib:detekt,
lib:spotlessKotlinCheck) pass locally.

AI disclosure

This contribution was developed with AI assistance (Claude Code, model
claude-fable-5). I have reviewed, tested and take responsibility for the
changes, in accordance with the Nextcloud AI contribution policy.

@codacy-production

codacy-production Bot commented Jul 8, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 9 complexity

Metric Results
Complexity 9

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

NextcloudAPI.close() nulls the ApiConnectedListener while the binding
to the AccountManagerService may still be in flight. unbindService()
only unbound established connections, so a pending ServiceConnection
stayed registered and later invoked onConnected() on the nulled
callback, crashing the host app on the main thread with a
NullPointerException in AidlNetworkRequest$1.onServiceConnected().

Unbind the ServiceConnection whenever a binding has been requested,
not only once it is established, and ignore stale onServiceConnected()
dispatches that race with close().

Fixes the crash reported in nextcloud/notes-android#2622.

Assisted-by: Claude Code:claude-fable-5
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: MiMoHo <37556964+MiMoHo@users.noreply.github.com>
@MiMoHo MiMoHo force-pushed the fix/close-connect-race branch from 23c0d23 to 26fe7b1 Compare July 8, 2026 03:43
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