Guard the service protocol's global handlers list with a reader/writer lock (#6888)

ci/licenses_golden/licenses_flutter

@@ -164,6 +164,8 @@ FILE: ../../../flutter/fml/platform/posix/file_posix.cc
 FILE: ../../../flutter/fml/platform/posix/mapping_posix.cc
 FILE: ../../../flutter/fml/platform/posix/native_library_posix.cc
 FILE: ../../../flutter/fml/platform/posix/paths_posix.cc
+FILE: ../../../flutter/fml/platform/posix/shared_mutex_posix.cc
+FILE: ../../../flutter/fml/platform/posix/shared_mutex_posix.h
 FILE: ../../../flutter/fml/platform/win/errors_win.cc
 FILE: ../../../flutter/fml/platform/win/errors_win.h
 FILE: ../../../flutter/fml/platform/win/file_win.cc
@@ -176,9 +178,13 @@ FILE: ../../../flutter/fml/platform/win/wstring_conversion.h
 FILE: ../../../flutter/fml/string_view.cc
 FILE: ../../../flutter/fml/string_view.h
 FILE: ../../../flutter/fml/string_view_unittest.cc
+FILE: ../../../flutter/fml/synchronization/atomic_object.h
 FILE: ../../../flutter/fml/synchronization/count_down_latch.cc
 FILE: ../../../flutter/fml/synchronization/count_down_latch.h
 FILE: ../../../flutter/fml/synchronization/count_down_latch_unittests.cc
+FILE: ../../../flutter/fml/synchronization/shared_mutex.h
+FILE: ../../../flutter/fml/synchronization/shared_mutex_std.cc
+FILE: ../../../flutter/fml/synchronization/shared_mutex_std.h
 FILE: ../../../flutter/fml/synchronization/thread_annotations.h
 FILE: ../../../flutter/fml/synchronization/thread_annotations_unittest.cc
 FILE: ../../../flutter/fml/synchronization/thread_checker.h

fml/BUILD.gn

@@ -46,8 +46,10 @@ source_set("fml") {
     "paths.h",
     "string_view.cc",
     "string_view.h",
+    "synchronization/atomic_object.h",
     "synchronization/count_down_latch.cc",
     "synchronization/count_down_latch.h",
+    "synchronization/shared_mutex.h",
     "synchronization/thread_annotations.h",
     "synchronization/thread_checker.h",
     "synchronization/waitable_event.cc",
@@ -83,6 +85,12 @@ source_set("fml") {
 
   libs = []
 
+  if (is_ios || is_mac) {
+    sources += [ "platform/posix/shared_mutex_posix.cc" ]
+  } else {
+    sources += [ "synchronization/shared_mutex_std.cc" ]
+  }
+
   if (is_ios || is_mac) {
     sources += [
       "platform/darwin/cf_utils.cc",

fml/platform/posix/shared_mutex_posix.cc

@@ -0,0 +1,30 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/fml/platform/posix/shared_mutex_posix.h"
+#include "flutter/fml/logging.h"
+
+namespace fml {
+
+SharedMutex* SharedMutex::Create() {
+  return new SharedMutexPosix();
+}
+
+SharedMutexPosix::SharedMutexPosix() {
+  FML_CHECK(pthread_rwlock_init(&rwlock_, nullptr) == 0);
+}
+
+void SharedMutexPosix::Lock() {
+  pthread_rwlock_wrlock(&rwlock_);
+}
+
+void SharedMutexPosix::LockShared() {
+  pthread_rwlock_rdlock(&rwlock_);
+}
+
+void SharedMutexPosix::Unlock() {
+  pthread_rwlock_unlock(&rwlock_);
+}
+
+}  // namespace fml

fml/platform/posix/shared_mutex_posix.h

@@ -0,0 +1,28 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef FLUTTER_FML_SYNCHRONIZATION_SHARED_MUTEX_POSIX_H_
+#define FLUTTER_FML_SYNCHRONIZATION_SHARED_MUTEX_POSIX_H_
+
+#include <shared_mutex>
+#include "flutter/fml/synchronization/shared_mutex.h"
+
+namespace fml {
+
+class SharedMutexPosix : public SharedMutex {
+ public:
+  virtual void Lock();
+  virtual void LockShared();
+  virtual void Unlock();
+
+ private:
+  friend SharedMutex* SharedMutex::Create();
+  SharedMutexPosix();
+
+  pthread_rwlock_t rwlock_;
+};
+
+}  // namespace fml
+
+#endif  // FLUTTER_FML_SYNCHRONIZATION_SHARED_MUTEX_POSIX_H_

fml/synchronization/atomic_object.h

@@ -0,0 +1,36 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef FLUTTER_FML_SYNCHRONIZATION_ATOMIC_OBJECT_H_
+#define FLUTTER_FML_SYNCHRONIZATION_ATOMIC_OBJECT_H_
+
+#include <mutex>
+
+namespace fml {
+
+// A wrapper for an object instance that can be read or written atomically.
+template <typename T>
+class AtomicObject {
+ public:
+  AtomicObject() = default;
+  AtomicObject(T object) : object_(object) {}
+
+  T Load() const {
+    std::lock_guard<std::mutex> lock(mutex_);
+    return object_;
+  }
+
+  void Store(const T& object) {
+    std::lock_guard<std::mutex> lock(mutex_);
+    object_ = object;
+  }
+
+ private:
+  mutable std::mutex mutex_;
+  T object_;
+};
+
+}  // namespace fml
+
+#endif  // FLUTTER_FML_SYNCHRONIZATION_ATOMIC_OBJECT_H_

fml/synchronization/shared_mutex.h

@@ -0,0 +1,49 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef FLUTTER_FML_SYNCHRONIZATION_SHARED_MUTEX_H_
+#define FLUTTER_FML_SYNCHRONIZATION_SHARED_MUTEX_H_
+
+namespace fml {
+
+// Interface for a reader/writer lock.
+class SharedMutex {
+ public:
+  static SharedMutex* Create();
+  virtual ~SharedMutex() = default;
+
+  virtual void Lock() = 0;
+  virtual void LockShared() = 0;
+  virtual void Unlock() = 0;
+};
+
+// RAII wrapper that does a shared acquire of a SharedMutex.
+class SharedLock {
+ public:
+  explicit SharedLock(SharedMutex& shared_mutex) : shared_mutex_(shared_mutex) {
+    shared_mutex_.LockShared();
+  }
+
+  ~SharedLock() { shared_mutex_.Unlock(); }
+
+ private:
+  SharedMutex& shared_mutex_;
+};
+
+// RAII wrapper that does an exclusive acquire of a SharedMutex.
+class UniqueLock {
+ public:
+  explicit UniqueLock(SharedMutex& shared_mutex) : shared_mutex_(shared_mutex) {
+    shared_mutex_.Lock();
+  }
+
+  ~UniqueLock() { shared_mutex_.Unlock(); }
+
+ private:
+  SharedMutex& shared_mutex_;
+};
+
+}  // namespace fml
+
+#endif  // FLUTTER_FML_SYNCHRONIZATION_SHARED_MUTEX_H_

fml/synchronization/shared_mutex_std.cc

@@ -0,0 +1,25 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "flutter/fml/synchronization/shared_mutex_std.h"
+
+namespace fml {
+
+SharedMutex* SharedMutex::Create() {
+  return new SharedMutexStd();
+}
+
+void SharedMutexStd::Lock() {
+  mutex_.lock();
+}
+
+void SharedMutexStd::LockShared() {
+  mutex_.lock_shared();
+}
+
+void SharedMutexStd::Unlock() {
+  mutex_.unlock();
+}
+
+}  // namespace fml

fml/synchronization/shared_mutex_std.h

@@ -0,0 +1,28 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef FLUTTER_FML_SYNCHRONIZATION_SHARED_MUTEX_STD_H_
+#define FLUTTER_FML_SYNCHRONIZATION_SHARED_MUTEX_STD_H_
+
+#include <shared_mutex>
+#include "flutter/fml/synchronization/shared_mutex.h"
+
+namespace fml {
+
+class SharedMutexStd : public SharedMutex {
+ public:
+  virtual void Lock();
+  virtual void LockShared();
+  virtual void Unlock();
+
+ private:
+  friend SharedMutex* SharedMutex::Create();
+  SharedMutexStd() = default;
+
+  std::shared_timed_mutex mutex_;
+};
+
+}  // namespace fml
+
+#endif  // FLUTTER_FML_SYNCHRONIZATION_SHARED_MUTEX_STD_H_

runtime/service_protocol.cc

@@ -45,29 +45,30 @@ ServiceProtocol::ServiceProtocol()
           kRunInViewExtensionName,
           kFlushUIThreadTasksExtensionName,
           kSetAssetBundlePathExtensionName,
-      }) {}
+      }),
+      handlers_mutex_(fml::SharedMutex::Create()) {}
 
 ServiceProtocol::~ServiceProtocol() {
   ToggleHooks(false);
 }
 
 void ServiceProtocol::AddHandler(Handler* handler,
                                  Handler::Description description) {
-  std::lock_guard<std::mutex> lock(handlers_mutex_);
+  fml::UniqueLock lock(*handlers_mutex_);
   handlers_.emplace(handler, description);
 }
 
 void ServiceProtocol::RemoveHandler(Handler* handler) {
-  std::lock_guard<std::mutex> lock(handlers_mutex_);
+  fml::UniqueLock lock(*handlers_mutex_);
   handlers_.erase(handler);
 }
 
 void ServiceProtocol::SetHandlerDescription(Handler* handler,
                                             Handler::Description description) {
-  std::lock_guard<std::mutex> lock(handlers_mutex_);
+  fml::SharedLock lock(*handlers_mutex_);
   auto it = handlers_.find(handler);
   if (it != handlers_.end())
-    it->second = description;
+    it->second.Store(description);
 }
 
 void ServiceProtocol::ToggleHooks(bool set) {
@@ -175,7 +176,7 @@ bool ServiceProtocol::HandleMessage(fml::StringView method,
     return HandleListViewsMethod(response);
   }
 
-  std::lock_guard<std::mutex> lock(handlers_mutex_);
+  fml::SharedLock lock(*handlers_mutex_);
 
   if (handlers_.size() == 0) {
     WriteServerErrorResponse(response,
@@ -246,12 +247,11 @@ void ServiceProtocol::Handler::Description::Write(
 
 bool ServiceProtocol::HandleListViewsMethod(
     rapidjson::Document& response) const {
-  // Collect handler descriptions on their respective task runners.
-  std::lock_guard<std::mutex> lock(handlers_mutex_);
+  fml::SharedLock lock(*handlers_mutex_);
   std::vector<std::pair<intptr_t, Handler::Description>> descriptions;
   for (const auto& handler : handlers_) {
     descriptions.emplace_back(reinterpret_cast<intptr_t>(handler.first),
-                              handler.second);
+                              handler.second.Load());
   }
 
   auto& allocator = response.GetAllocator();

runtime/service_protocol.h

@@ -6,13 +6,14 @@
 #define FLUTTER_RUNTIME_SERVICE_PROTOCOL_H_
 
 #include <map>
-#include <mutex>
 #include <set>
 #include <string>
 
 #include "flutter/fml/compiler_specific.h"
 #include "flutter/fml/macros.h"
 #include "flutter/fml/string_view.h"
+#include "flutter/fml/synchronization/atomic_object.h"
+#include "flutter/fml/synchronization/shared_mutex.h"
 #include "flutter/fml/synchronization/thread_annotations.h"
 #include "flutter/fml/task_runner.h"
 #include "rapidjson/document.h"
@@ -72,8 +73,8 @@ class ServiceProtocol {
 
  private:
   const std::set<fml::StringView> endpoints_;
-  mutable std::mutex handlers_mutex_;
-  std::map<Handler*, Handler::Description> handlers_;
+  std::unique_ptr<fml::SharedMutex> handlers_mutex_;
+  std::map<Handler*, fml::AtomicObject<Handler::Description>> handlers_;
 
   FML_WARN_UNUSED_RESULT
   static bool HandleMessage(const char* method,