Fix concurrent group access to prevent NullPointerException - #1
Fix concurrent group access to prevent NullPointerException#1ShashankFC wants to merge 1 commit into
Conversation
Closes #40368 Signed-off-by: vramik <vramik@redhat.com>
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| if (isUpdated()) return updated.getSubGroupsCount(); | ||
| return getGroupModel().getSubGroupsCount(); | ||
| GroupModel model = modelSupplier.get(); | ||
| return model == null ? null : model.getSubGroupsCount(); |
There was a problem hiding this comment.
Returning null violates documented non-null interface contract
Medium Severity
The GroupModel.getSubGroupsCount() interface contract explicitly documents @return The number of groups beneath this group. Never returns {@code null}. The new null-safety check returns null when the model is not found, violating this contract. This just moves the NullPointerException risk downstream to callers that rely on the documented guarantee (e.g., any auto-unboxing to long). Returning 0L instead of null would satisfy the contract while still avoiding the NPE at this site.
| }); | ||
| deletedAll.set(true); | ||
|
|
||
| assertThat(caughtExceptions, Matchers.empty()); |
There was a problem hiding this comment.
Reader thread not joined before assertion causes false passes
Medium Severity
The reader thread is never joined before the assertion on caughtExceptions. After deletedAll.set(true), the main thread immediately asserts, but the reader thread may still have an HTTP request in-flight. If that request throws an exception, it gets added to caughtExceptions after the assertion has already passed. This race condition means the test can falsely pass, defeating the purpose of validating the concurrent-access fix.
2 participants
Test 10nnn## Summary by CodeRabbitnn## Release Notesnn* Testsn * Added comprehensive concurrent group operations test to validate reliability during simultaneous create, read, and delete operations.nn* Bug Fixesn * Improved group caching stability with enhanced null-safety checks.nn✏️ Tip: You can customize this high-level summary in your review settings.nnn---nReplicated from ai-code-review-evaluation/keycloak-coderabbit#10
Note
Medium Risk
Touches cached group read paths used by admin APIs and changes behavior to return
nullwhen the delegate group disappears; concurrency bugs are subtle, but the change is localized and covered by a new stress-style test.Overview
Fixes a race in the Infinispan group cache adapter by making
GroupAdapter.getSubGroupsCount()null-safe when the underlyingGroupModelcan no longer be loaded (e.g., concurrently deleted).Cleans up
GroupUtilsby removing an unused search helper, and adds an integration test that concurrently creates many groups, continuously lists groups in a background thread, deletes them, and asserts no exceptions are thrown during the concurrent read/delete window.Written by Cursor Bugbot for commit 332c9b6. Configure here.