Skip to content

Add tsan annotation for atomic_thread_fence#490

Merged
mpenick merged 1 commit intoapache:masterfrom
tavplubix:add_tsan_annotations
Mar 12, 2021
Merged

Add tsan annotation for atomic_thread_fence#490
mpenick merged 1 commit intoapache:masterfrom
tavplubix:add_tsan_annotations

Conversation

@tavplubix
Copy link
Copy Markdown
Contributor

Thread sanitizer does not support atomic_thread_fence with release/acquire order, so using of atomic_thread_fence(MEMORY_ORDER_ACQUIRE) in RefCounted::dec_ref() causes false positive reports like this: https://gist.github.com/alesapin/96b14caa274a291f12dcf84cddc8ff5c

Explicit annotation avoids such false positives.

@tavplubix
Copy link
Copy Markdown
Contributor Author

Btw, there is another data race which doesn't look false positive: https://gist.github.com/alesapin/bcb6ad9cd99e0c2e198ea4348db38145
I've fixed it with std::shared_ptr in our fork (ClickHouse@9cbc1a8), but I'm not familiar with internals of the library, so there must be a better way to fix it.

@mpenick
Copy link
Copy Markdown
Contributor

mpenick commented Mar 10, 2021

Thanks. I'm running it through CI. Could you run a make format (that breaks our build)?

@tavplubix tavplubix force-pushed the add_tsan_annotations branch from 4d04257 to f7e4df1 Compare March 10, 2021 15:47
@mpenick mpenick merged commit a7f49e6 into apache:master Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants