R0CKing666 opened a new issue, #11189: URL: https://github.com/apache/rocketmq/issues/11189
### Before Creating the Bug Report - [x] I found a bug, not just asking a question, which should be created in [GitHub Discussions](https://github.com/apache/rocketmq/discussions). - [x] I have searched the [GitHub Issues](https://github.com/apache/rocketmq/issues) and [GitHub Discussions](https://github.com/apache/rocketmq/discussions) of this repository and believe that this is not a duplicate. - [x] I have confirmed that this bug belongs to the current repository, not other repositories of RocketMQ. ### Runtime platform environment OS: all platforms ### RocketMQ version branch: develop version: 5.5.1 ### Describe the Bug `AuthorizationMetadataManagerImpl.createAcl/updateAcl/deleteAcl` first read the instance returned by `getAcl` as the "old value", then call `updatePolicy`/`deletePolicy` on it in place, and then hand the same object to `updateAcl` for persistence. `AuthenticationMetadataManagerImpl.updateUser` likewise calls `setPassword/setUserType/setUserStatus` directly on the `User` returned by `getUser`. `getAcl`/`getUser` are ultimately implemented by `LocalAuthorizationMetadataProvider`/`LocalAuthenticationMetadataProvider`, which return the very object reference held in the Caffeine cache (no defensive copy). As a result, the admin write path and the broker authorization/authentication hot path read the same mutable object. Readers can observe a half-updated state (some fields old, some new), breaking atomic visibility of ACL/User, and concurrent structural modification can throw `ConcurrentModificationException`. ### Steps to Reproduce 1. Enable ACL 2.0 (local metadata provider) and start the broker. 2. Create a user and an ACL. 3. While clients continuously issue requests that trigger authorization (`AclAuthorizationHandler` -> `getAcl` -> cached instance), concurrently send `UPDATE_ACL`/`CREATE_ACL` (or `UPDATE_USER`) via `AdminBrokerProcessor`. 4. The manager mutates the cached `Acl`/`User` in place (e.g. `oldAcl.updatePolicy(...)` in `AuthorizationMetadataManagerImpl#updateAcl`) before the RocksDB write and cache invalidation. Observed: authorization results can flip between ALLOW/DENY (half-updated policy set), requests can fail with `ConcurrentModificationException`, and if the RocksDB write fails, the in-memory cache diverges from disk. ### What Did You Expect to See? Readers on the hot path should only ever observe a complete old value or a complete new value; a new value should become visible only after it has been persisted, and concurrent reads/writes should not throw `ConcurrentModificationException`. ### What Did You See Instead? In-place mutation of the cached object is visible to readers before persistence, can produce torn reads, and can throw `ConcurrentModificationException`. ### Additional Context Root cause: the local providers return the cached mutable reference without a defensive copy, and the managers mutate that reference in place. - `LocalAuthorizationMetadataProvider#getAcl` returns `aclCache.get(...)` directly. - `LocalAuthenticationMetadataProvider#getUser` returns `userCache.get(...)` directly. - `AuthorizationMetadataManagerImpl#createAcl`/`updateAcl`/`deleteAcl` mutate `oldAcl` in place. - `AuthenticationMetadataManagerImpl#updateUser` mutates the cached `User` in place. Suggested fix: the write path must not reuse the cached instance. Construct a new object (deep copy) and apply the change to the copy before persisting it, so the cached instance is only ever replaced atomically after a successful write. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
