[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

We're hitting a false positive in grpc after this:

  > ../../third_party/grpc/src/src/core/lib/gprpp/ref_counted_ptr.h:335:31: error: calling function 'TlsSessionKeyLoggerCache' requires holding mutex 'g_tls_session_key_log_cache_mu' exclusively [-Werror,-Wthread-safety-analysis]
  >   return RefCountedPtr<T>(new T(std::forward<Args>(args)...));
  >                               ^
  > ../../third_party/grpc/src/src/core/tsi/ssl/key_logging/ssl_key_logging.cc:121:26: note: in instantiation of function template specialization 'grpc_core::MakeRefCounted<tsi::TlsSessionKeyLoggerCache>' requested here
  >      cache = grpc_core::MakeRefCounted<TlsSessionKeyLoggerCache>();
  >                         ^
  
  
  The code looks like this:
  
  grpc_core::RefCountedPtr<TlsSessionKeyLogger> TlsSessionKeyLoggerCache::Get(
      std::string tls_session_key_log_file_path) {
    gpr_once_init(&g_cache_mutex_init, do_cache_mutex_init);
    GPR_DEBUG_ASSERT(g_tls_session_key_log_cache_mu != nullptr);
    if (tls_session_key_log_file_path.empty()) {
      return nullptr;
    }
    {
      grpc_core::MutexLock lock(g_tls_session_key_log_cache_mu);        <---------- holding the mutex
      grpc_core::RefCountedPtr<TlsSessionKeyLoggerCache> cache;
      if (g_cache_instance == nullptr) {
        // This will automatically set g_cache_instance.
        cache = grpc_core::MakeRefCounted<TlsSessionKeyLoggerCache>();      <------ line 121
  
  
  
  lock is holding a MutexLock (I assume that's an exclusive thing) on g_tls_session_key_log_cache_mu.

See https://bugs.chromium.org/p/chromium/issues/detail?id=1372394#c4 for how to reproduce.

I've reverted this in https://github.com/llvm/llvm-project/commit/a4afa2bde6f4db215ddd3267a8d11c04367812e5 in the meantime.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129755/new/

https://reviews.llvm.org/D129755