Implemented threadsafe platform channel replies on windows (#36909)

* Implemented threadsafe platform channel replies on windows

* added unit test

* added docstrings

* implemented glfw

* added comments

* made glfw messenger unable to be copied

* stuart feedback 1

* stuart feedback 2: replaced the shared_ptr

* stuart feedback 3

* stuart feedback: remove error log

* Moved FlutterDesktopMessenger to its own file.

* updated licenses

* stuart feedback

Author: gaaclarke

ci/licenses_golden/licenses_flutter

@@ -3099,6 +3099,7 @@ FILE: ../../../flutter/shell/platform/windows/external_texture_d3d.cc
 FILE: ../../../flutter/shell/platform/windows/external_texture_d3d.h
 FILE: ../../../flutter/shell/platform/windows/external_texture_pixelbuffer.cc
 FILE: ../../../flutter/shell/platform/windows/external_texture_pixelbuffer.h
+FILE: ../../../flutter/shell/platform/windows/flutter_desktop_messenger.h
 FILE: ../../../flutter/shell/platform/windows/flutter_key_map.g.cc
 FILE: ../../../flutter/shell/platform/windows/flutter_platform_node_delegate_windows.cc
 FILE: ../../../flutter/shell/platform/windows/flutter_platform_node_delegate_windows.h

shell/platform/common/client_wrapper/core_implementations.cc

@@ -26,6 +26,11 @@ namespace flutter {
 // ========== binary_messenger_impl.h ==========
 
 namespace {
+
+using FlutterDesktopMessengerScopedLock =
+    std::unique_ptr<FlutterDesktopMessenger,
+                    decltype(&FlutterDesktopMessengerUnlock)>;
+
 // Passes |message| to |user_data|, which must be a BinaryMessageHandler, along
 // with a BinaryReply that will send a response on |message|'s response handle.
 //
@@ -36,17 +41,28 @@ void ForwardToHandler(FlutterDesktopMessengerRef messenger,
                       const FlutterDesktopMessage* message,
                       void* user_data) {
   auto* response_handle = message->response_handle;
-  BinaryReply reply_handler = [messenger, response_handle](
+  auto messenger_ptr = std::shared_ptr<FlutterDesktopMessenger>(
+      FlutterDesktopMessengerAddRef(messenger),
+      &FlutterDesktopMessengerRelease);
+  BinaryReply reply_handler = [messenger_ptr, response_handle](
                                   const uint8_t* reply,
                                   size_t reply_size) mutable {
+    // Note: This lambda can be called on any thread.
+    auto lock = FlutterDesktopMessengerScopedLock(
+        FlutterDesktopMessengerLock(messenger_ptr.get()),
+        &FlutterDesktopMessengerUnlock);
+    if (!FlutterDesktopMessengerIsAvailable(messenger_ptr.get())) {
+      // Drop reply if it comes in after the engine is destroyed.
+      return;
+    }
     if (!response_handle) {
       std::cerr << "Error: Response can be set only once. Ignoring "
                    "duplicate response."
                 << std::endl;
       return;
     }
-    FlutterDesktopMessengerSendResponse(messenger, response_handle, reply,
-                                        reply_size);
+    FlutterDesktopMessengerSendResponse(messenger_ptr.get(), response_handle,
+                                        reply, reply_size);
     // The engine frees the response handle once
     // FlutterDesktopSendMessageResponse is called.
     response_handle = nullptr;

shell/platform/common/client_wrapper/testing/stub_flutter_api.cc

@@ -4,6 +4,8 @@
 
 #include "flutter/shell/platform/common/client_wrapper/testing/stub_flutter_api.h"
 
+#include <cassert>
+
 static flutter::testing::StubFlutterApi* s_stub_implementation;
 
 namespace flutter {
@@ -93,6 +95,31 @@ void FlutterDesktopMessengerSetCallback(FlutterDesktopMessengerRef messenger,
   }
 }
 
+FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef(
+    FlutterDesktopMessengerRef messenger) {
+  assert(false);  // not implemented
+  return nullptr;
+}
+
+void FlutterDesktopMessengerRelease(FlutterDesktopMessengerRef messenger) {
+  assert(false);  // not implemented
+}
+
+bool FlutterDesktopMessengerIsAvailable(FlutterDesktopMessengerRef messenger) {
+  assert(false);  // not implemented
+  return false;
+}
+
+FlutterDesktopMessengerRef FlutterDesktopMessengerLock(
+    FlutterDesktopMessengerRef messenger) {
+  assert(false);  // not implemented
+  return nullptr;
+}
+
+void FlutterDesktopMessengerUnlock(FlutterDesktopMessengerRef messenger) {
+  assert(false);  // not implemented
+}
+
 FlutterDesktopTextureRegistrarRef FlutterDesktopRegistrarGetTextureRegistrar(
     FlutterDesktopPluginRegistrarRef registrar) {
   return reinterpret_cast<FlutterDesktopTextureRegistrarRef>(1);

shell/platform/common/public/flutter_messenger.h

@@ -5,6 +5,7 @@
 #ifndef FLUTTER_SHELL_PLATFORM_COMMON_PUBLIC_FLUTTER_MESSENGER_H_
 #define FLUTTER_SHELL_PLATFORM_COMMON_PUBLIC_FLUTTER_MESSENGER_H_
 
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
 
@@ -87,6 +88,53 @@ FLUTTER_EXPORT void FlutterDesktopMessengerSetCallback(
     FlutterDesktopMessageCallback callback,
     void* user_data);
 
+// Increments the reference count for the |messenger|.
+//
+// Operation is thread-safe.
+//
+// See also: |FlutterDesktopMessengerRelease|
+FLUTTER_EXPORT FlutterDesktopMessengerRef
+FlutterDesktopMessengerAddRef(FlutterDesktopMessengerRef messenger);
+
+// Decrements the reference count for the |messenger|.
+//
+// Operation is thread-safe.
+//
+// See also: |FlutterDesktopMessengerAddRef|
+FLUTTER_EXPORT void FlutterDesktopMessengerRelease(
+    FlutterDesktopMessengerRef messenger);
+
+// Returns `true` if the |FlutterDesktopMessengerRef| still references a running
+// engine.
+//
+// This check should be made inside of a |FlutterDesktopMessengerLock| and
+// before any other calls are made to the FlutterDesktopMessengerRef when using
+// it from a thread other than the platform thread.
+FLUTTER_EXPORT bool FlutterDesktopMessengerIsAvailable(
+    FlutterDesktopMessengerRef messenger);
+
+// Locks the `FlutterDesktopMessengerRef` ensuring that
+// |FlutterDesktopMessengerIsAvailable| does not change while locked.
+//
+// All calls to the FlutterDesktopMessengerRef from threads other than the
+// platform thread should happen inside of a lock.
+//
+// Operation is thread-safe.
+//
+// Returns the |messenger| value.
+//
+// See also: |FlutterDesktopMessengerUnlock|
+FLUTTER_EXPORT FlutterDesktopMessengerRef
+FlutterDesktopMessengerLock(FlutterDesktopMessengerRef messenger);
+
+// Unlocks the `FlutterDesktopMessengerRef`.
+//
+// Operation is thread-safe.
+//
+// See also: |FlutterDesktopMessengerLock|
+FLUTTER_EXPORT void FlutterDesktopMessengerUnlock(
+    FlutterDesktopMessengerRef messenger);
+
 #if defined(__cplusplus)
 }  // extern "C"
 #endif

shell/platform/embedder/embedder.cc

@@ -2325,6 +2325,7 @@ FlutterEngineResult FlutterPlatformMessageReleaseResponseHandle(
   return kSuccess;
 }
 
+// Note: This can execute on any thread.
 FlutterEngineResult FlutterEngineSendPlatformMessageResponse(
     FLUTTER_API_SYMBOL(FlutterEngine) engine,
     const FlutterPlatformMessageResponseHandle* handle,

shell/platform/glfw/flutter_glfw.cc

@@ -106,6 +106,10 @@ struct AOTDataDeleter {
 };
 
 using UniqueAotDataPtr = std::unique_ptr<_FlutterEngineAOTData, AOTDataDeleter>;
+/// Maintains one ref on the FlutterDesktopMessenger's internal reference count.
+using FlutterDesktopMessengerReferenceOwner =
+    std::unique_ptr<FlutterDesktopMessenger,
+                    decltype(&FlutterDesktopMessengerRelease)>;
 
 // Struct for storing state of a Flutter engine instance.
 struct FlutterDesktopEngineState {
@@ -116,7 +120,8 @@ struct FlutterDesktopEngineState {
   std::unique_ptr<flutter::EventLoop> event_loop;
 
   // The plugin messenger handle given to API clients.
-  std::unique_ptr<FlutterDesktopMessenger> messenger;
+  FlutterDesktopMessengerReferenceOwner messenger = {
+      nullptr, [](FlutterDesktopMessengerRef ref) {}};
 
   // Message dispatch manager for messages from the Flutter engine.
   std::unique_ptr<flutter::IncomingMessageDispatcher> message_dispatcher;
@@ -149,10 +154,75 @@ struct FlutterDesktopPluginRegistrar {
 
 // State associated with the messenger used to communicate with the engine.
 struct FlutterDesktopMessenger {
+  FlutterDesktopMessenger() = default;
+
+  /// Increments the reference count.
+  ///
+  /// Thread-safe.
+  void AddRef() { ref_count_.fetch_add(1); }
+
+  /// Decrements the reference count and deletes the object if the count has
+  /// gone to zero.
+  ///
+  /// Thread-safe.
+  void Release() {
+    int32_t old_count = ref_count_.fetch_sub(1);
+    if (old_count <= 1) {
+      delete this;
+    }
+  }
+
+  /// Getter for the engine field.
+  FlutterDesktopEngineState* GetEngine() const { return engine_; }
+
+  /// Setter for the engine field.
+  /// Thread-safe.
+  void SetEngine(FlutterDesktopEngineState* engine) {
+    std::scoped_lock lock(mutex_);
+    engine_ = engine;
+  }
+
+  /// Returns the mutex associated with the |FlutterDesktopMessenger|.
+  ///
+  /// This mutex is used to synchronize reading or writing state inside the
+  /// |FlutterDesktopMessenger| (ie |engine_|).
+  std::mutex& GetMutex() { return mutex_; }
+
+  FlutterDesktopMessenger(const FlutterDesktopMessenger& value) = delete;
+  FlutterDesktopMessenger& operator=(const FlutterDesktopMessenger& value) =
+      delete;
+
+ private:
   // The engine that backs this messenger.
-  FlutterDesktopEngineState* engine;
+  FlutterDesktopEngineState* engine_;
+  std::atomic<int32_t> ref_count_ = 0;
+  std::mutex mutex_;
 };
 
+FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef(
+    FlutterDesktopMessengerRef messenger) {
+  messenger->AddRef();
+  return messenger;
+}
+
+void FlutterDesktopMessengerRelease(FlutterDesktopMessengerRef messenger) {
+  messenger->Release();
+}
+
+bool FlutterDesktopMessengerIsAvailable(FlutterDesktopMessengerRef messenger) {
+  return messenger->GetEngine() != nullptr;
+}
+
+FlutterDesktopMessengerRef FlutterDesktopMessengerLock(
+    FlutterDesktopMessengerRef messenger) {
+  messenger->GetMutex().lock();
+  return messenger;
+}
+
+void FlutterDesktopMessengerUnlock(FlutterDesktopMessengerRef messenger) {
+  messenger->GetMutex().unlock();
+}
+
 // Retrieves state bag for the window in question from the GLFWWindow.
 static FlutterDesktopWindowControllerState* GetWindowController(
     GLFWwindow* window) {
@@ -743,8 +813,10 @@ static void SetUpLocales(FlutterDesktopEngineState* state) {
 static void SetUpCommonEngineState(FlutterDesktopEngineState* state,
                                    GLFWwindow* window) {
   // Messaging.
-  state->messenger = std::make_unique<FlutterDesktopMessenger>();
-  state->messenger->engine = state;
+  state->messenger = FlutterDesktopMessengerReferenceOwner(
+      FlutterDesktopMessengerAddRef(new FlutterDesktopMessenger()),
+      &FlutterDesktopMessengerRelease);
+  state->messenger->SetEngine(state);
   state->message_dispatcher =
       std::make_unique<flutter::IncomingMessageDispatcher>(
           state->messenger.get());
@@ -846,6 +918,7 @@ FlutterDesktopWindowControllerRef FlutterDesktopCreateWindow(
 }
 
 void FlutterDesktopDestroyWindow(FlutterDesktopWindowControllerRef controller) {
+  controller->engine->messenger->SetEngine(nullptr);
   FlutterDesktopPluginRegistrarRef registrar =
       controller->engine->plugin_registrar.get();
   if (registrar->destruction_handler) {
@@ -1045,7 +1118,8 @@ bool FlutterDesktopMessengerSendWithReply(FlutterDesktopMessengerRef messenger,
   FlutterPlatformMessageResponseHandle* response_handle = nullptr;
   if (reply != nullptr && user_data != nullptr) {
     FlutterEngineResult result = FlutterPlatformMessageCreateResponseHandle(
-        messenger->engine->flutter_engine, reply, user_data, &response_handle);
+        messenger->GetEngine()->flutter_engine, reply, user_data,
+        &response_handle);
     if (result != kSuccess) {
       std::cout << "Failed to create response handle\n";
       return false;
@@ -1061,11 +1135,11 @@ bool FlutterDesktopMessengerSendWithReply(FlutterDesktopMessengerRef messenger,
   };
 
   FlutterEngineResult message_result = FlutterEngineSendPlatformMessage(
-      messenger->engine->flutter_engine, &platform_message);
+      messenger->GetEngine()->flutter_engine, &platform_message);
 
   if (response_handle != nullptr) {
     FlutterPlatformMessageReleaseResponseHandle(
-        messenger->engine->flutter_engine, response_handle);
+        messenger->GetEngine()->flutter_engine, response_handle);
   }
 
   return message_result == kSuccess;
@@ -1084,16 +1158,16 @@ void FlutterDesktopMessengerSendResponse(
     const FlutterDesktopMessageResponseHandle* handle,
     const uint8_t* data,
     size_t data_length) {
-  FlutterEngineSendPlatformMessageResponse(messenger->engine->flutter_engine,
-                                           handle, data, data_length);
+  FlutterEngineSendPlatformMessageResponse(
+      messenger->GetEngine()->flutter_engine, handle, data, data_length);
 }
 
 void FlutterDesktopMessengerSetCallback(FlutterDesktopMessengerRef messenger,
                                         const char* channel,
                                         FlutterDesktopMessageCallback callback,
                                         void* user_data) {
-  messenger->engine->message_dispatcher->SetMessageCallback(channel, callback,
-                                                            user_data);
+  messenger->GetEngine()->message_dispatcher->SetMessageCallback(
+      channel, callback, user_data);
 }
 
 FlutterDesktopTextureRegistrarRef FlutterDesktopRegistrarGetTextureRegistrar(