fix(api): don't crash when the service connects after the API was closed#1023
fix(api): don't crash when the service connects after the API was closed#1023MiMoHo wants to merge 1 commit into
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 9 |
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 <[email protected]> Signed-off-by: MiMoHo <[email protected]>
23c0d23 to
26fe7b1
Compare
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
fix(api): don't crash when the service connects after the API was closed
Problem
Host apps using this library keep crashing with:
Reported for example in nextcloud/notes-android#2622 (labeled "Upstream :: SSO").
Root cause
NextcloudAPIstarts binding to theAccountManagerServiceasynchronously.When the consumer calls
NextcloudAPI#close()before the connection isestablished (e.g. after a request error invalidates a cached API instance),
NetworkRequest#close()nulls theApiConnectedListener— butAidlNetworkRequest#unbindService()only calledContext#unbindService()when the connection was already established (
mBound == true). The pendingServiceConnectiontherefore stayed registered with the framework and laterdelivered
onServiceConnected()on the main thread, which invokedonConnected()on the nulled callback and crashed the host app.Fix
so a not-yet-established connection is released on
close()as well.onServiceConnected()against the remaining race whereclose()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:onServiceConnected()afterclose()no longer throws(fails with exactly the reported NPE without the fix),
close()before the connection is established unbinds theServiceConnection(fails without the fix).unitTests.returnDefaultValues = truewas enabled so the plain JVM tests canrun
connect(), which touchesIntent/ComponentName. All existing testsand 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.