Fix race condition in plugin resolution during disconnect

[andremion]

Goal

Fix a race condition where disconnect() clears the plugins list between the initializationState check and the plugins.find() call in resolvePluginDependency(), causing IllegalStateException: Plugin was not found crashes

#6156

Implementation

• Added @Volatile to the plugins property to ensure cross-thread visibility of writes.
• Reordered reads in resolvePluginDependency(): snapshot plugins before checking initializationState. Since disconnectUserSuspend() sets NOT_INITIALIZED before clearing plugins, if the snapshot captures populated plugins and then we see COMPLETE, the snapshot is guaranteed valid.

🎨 UI Changes

No UI changes.

Testing

• Run DependencyResolverTest — a new test (Should resolve dependency when plugins are cleared during resolution) uses a DisconnectSimulatingStateFlow to deterministically simulate disconnect() clearing plugins mid-resolution. It verifies the dependency resolves successfully because the plugins snapshot was taken before the state was read.

Note

I can't reproduce using the sample app.

Summary by CodeRabbit

• Bug Fixes
  • Improved plugin operation reliability with enhanced synchronization
  • Better handling of edge cases during plugin disconnect scenarios

[github-actions]

PR checklist ✅

All required conditions are satisfied:

• Title length is OK (or ignored by label).
• At least one pr: label exists.
• Sections ### Goal, ### Implementation, and ### Testing are filled.

🎉 Great job! This PR is ready for review.

[github-actions]

SDK Size Comparison 📏

SDK	Before	After	Difference	Status
stream-chat-android-client	5.26 MB	5.26 MB	0.00 MB	🟢
stream-chat-android-offline	5.48 MB	5.48 MB	0.00 MB	🟢
stream-chat-android-ui-components	10.63 MB	10.63 MB	0.00 MB	🟢
stream-chat-android-compose	12.85 MB	12.85 MB	0.00 MB	🟢

[coderabbitai]

Walkthrough

The PR introduces thread synchronization around plugin operations in ChatClient using a new private lock, refactors plugin dependency resolution into a dedicated helper function that enforces initialization state checks, and adds a test case verifying correct error handling when disconnect races occur during dependency resolution.

Changes

Cohort / File(s)	Summary
Plugin synchronization and resolution refactoring stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt	Added pluginLock synchronization primitive for serializing plugin state changes; extracted plugin dependency resolution logic into new findPluginAndResolve() helper function with enhanced initialization state validation and error messaging; refactored resolvePluginDependency() to delegate to the new helper.
Disconnect race condition test stream-chat-android-client/src/test/java/io/getstream/chat/android/client/DependencyResolverTest.kt	Added test case "Should throw connectUser error instead of plugin not found when disconnect races" with DisconnectSimulatingStateFlow helper class to simulate plugin state changes during dependency resolution, validating error precedence when initialization races occur.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Locked and loaded, threads now play fair,
Plugins synchronized with utmost care,
Race conditions caught before they spread,
Dependencies resolved with clarity instead,
A rabbit's refactor, clean and bright! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix race condition in plugin resolution during disconnect' directly and clearly summarizes the main change: addressing a race condition that occurs when disconnect() clears plugins during dependency resolution.
Description check	✅ Passed	The PR description covers the Goal (race condition fix) and Implementation (using volatile + snapshot approach), includes a Testing section, and provides a linked issue (#6156). However, it lacks some non-critical template sections like UI Changes images, videos, and incomplete contributor/reviewer checklists.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch bug/AND-1124-resolve-plugin-dependency-race-condition

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[VelikovPetar]

Just a quick question: Do we have some potential implications if we reach this point in the code - but disconnect() was already called? (fully understand that this is an extreme edge case, but I am just thinking of possible side effects)

Also, is it possible that currentPlugins is resolved as emptyList() before the awaitInitializationState()?