[refine](exec) replace std::shared_mutex/std::shared_lock with annotated wrappers for thread safety analysis#63109
[refine](exec) replace std::shared_mutex/std::shared_lock with annotated wrappers for thread safety analysis#63109Mryange wants to merge 1 commit intoapache:masterfrom
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated code review summary:\n\nNo blocking issues found in the actual PR diff. The change is focused on adding annotated shared-mutex/shared-lock wrappers and applying them to VDataStreamMgr and RuntimeFilterMergeControllerEntity without changing the effective locking order or heavy-work placement.\n\nCritical checkpoints:\n- Goal/test: The PR goal appears to be enabling Clang thread-safety analysis for shared-mutex users; the code accomplishes this for the touched classes. No test changes are included, but this is a static-analysis annotation/refactor with no intended runtime behavior change.\n- Scope: The modification is small and focused.\n- Concurrency: Existing shared/exclusive lock protection is preserved. VDataStreamMgr keeps receiver-map/set operations under the same mutex, and cancel still copies receivers before doing heavier cancellation. RuntimeFilterMergeControllerEntity preserves the map mutex plus per-filter mutex pattern.\n- Lifecycle/static initialization: No new lifecycle-sensitive globals or static initialization dependencies were introduced.\n- Configuration/compatibility/persistence/data writes: Not applicable.\n- Parallel paths: The two touched shared-mutex users are updated consistently in their declarations and call sites.\n- Tests/results: No runtime test result changes. A compile/static-analysis run in CI should validate the annotation intent.\n- Observability/performance: No new logs or metrics needed; runtime lock behavior remains equivalent to std::shared_mutex/std::shared_lock.\n- User focus: No additional user-provided review focus was specified.
Mryange
changed the title
|
run buildall |
TPC-H: Total hot run time: 29710 ms |
TPC-DS: Total hot run time: 170673 ms |
2 participants
What problem does this PR solve?
Issue Number: N/A
Problem Summary:
After #63070 replaced
std::mutex/std::lock_guardwith annotated wrappers (AnnotatedMutex/LockGuard), shared mutex usage still relied on rawstd::shared_mutexandstd::shared_lock/std::unique_lock, which are invisible to Clang's thread safety analysis. This leaves shared-lock sites unverified —GUARDED_BY,REQUIRES_SHARED, and other annotations cannot be enforced.AnnotatedSharedMutex(wrappingstd::shared_mutexwithCAPABILITY/ACQUIRE/RELEASE/ACQUIRE_SHARED/RELEASE_SHAREDannotations) andSharedLockGuard(RAIISCOPED_CAPABILITYwithACQUIRE_SHARED/RELEASE) inthread_safety_annotations.h.VDataStreamMgrandRuntimeFilterMergeControllerEntityfromstd::shared_mutextoAnnotatedSharedMutex, and fromstd::unique_lock/std::shared_locktoLockGuard/SharedLockGuard.GUARDED_BYannotations to the protected maps._find_recvras a private helper annotated withREQUIRES_SHARED(_lock), eliminating thebool acquire_lockparameter that previously bypassed lock tracking.Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)