Skip to content

fix(mutex): auto-detect thread safety attributes and annotate lock/unlock (fixes #2263) - #2298

Open
jdymitarai wants to merge 1 commit into
google:mainfrom
jdymitarai:fix-thread-safety-mutex-2263
Open

fix(mutex): auto-detect thread safety attributes and annotate lock/unlock (fixes #2263)#2298
jdymitarai wants to merge 1 commit into
google:mainfrom
jdymitarai:fix-thread-safety-mutex-2263

Conversation

@jdymitarai

Copy link
Copy Markdown

Fixes #2263

Problem

When compiling with Clang with -Wthread-safety enabled (such as on macOS or under Bazel defaults), src/mutex.h produces -Wthread-safety-analysis errors:

src/mutex.h:79:40: error: mutex 'mut_' is still held at the end of function [-Werror,-Wthread-safety-analysis]
   79 |   void lock() ACQUIRE() { mut_.lock(); }
      |                                        ^
src/mutex.h:80:34: error: releasing mutex 'mut_' that was not held [-Werror,-Wthread-safety-analysis]
   80 |   void unlock() RELEASE() { mut_.unlock(); }
      |                                  ^

This occurs because:

  1. HAVE_THREAD_SAFETY_ATTRIBUTES was not defined by default in CMake or Bazel, so thread safety annotations expand to no-ops unless manually supplied via compiler flags.
  2. On toolchains where std::mutex has thread safety annotations (such as libc++ on macOS), calling mut_.lock() and mut_.unlock() within the wrapper methods without NO_THREAD_SAFETY_ANALYSIS flags mut_ as an unreleased lock at function exit.

Solution

  1. Automatically detect thread safety attribute support when compiling with Clang via __has_attribute(capability) && __has_attribute(scoped_lockable) && __has_attribute(guarded_by).
  2. Allow users to explicitly disable annotations via -DHAVE_THREAD_SAFETY_ATTRIBUTES=0.
  3. Add NO_THREAD_SAFETY_ANALYSIS to Mutex::lock() and Mutex::unlock() to suppress internal capability analysis on mut_ while preserving ACQUIRE() / RELEASE() contracts for callers.

Verification

  • Built full test suite with MSVC 2022.
  • Ran all 87 tests via ctest: 100% passed (0 failed).

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.

[BUG] Fail to build with -Werror on macOS.

1 participant