Implemented threadsafe platform channel replies on windows

Author: gaaclarke

shell/platform/common/client_wrapper/core_implementations.cc

@@ -36,17 +36,30 @@ 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 {
+    auto lock = std::unique_ptr<FlutterDesktopMessenger,
+                                decltype(&FlutterDesktopMessengerUnlock)>(
+        FlutterDesktopMessengerLock(messenger_ptr.get()),
+        &FlutterDesktopMessengerUnlock);
+    if (!FlutterDesktopMessengerIsAvailable(messenger_ptr.get())) {
+      std::cerr << "Error: Responding to platform channel after the engine has"
+                   "been deleted."
+                << std::endl;
+      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,21 @@ FLUTTER_EXPORT void FlutterDesktopMessengerSetCallback(
     FlutterDesktopMessageCallback callback,
     void* user_data);
 
+FLUTTER_EXPORT FlutterDesktopMessengerRef
+FlutterDesktopMessengerAddRef(FlutterDesktopMessengerRef messenger);
+
+FLUTTER_EXPORT void FlutterDesktopMessengerRelease(
+    FlutterDesktopMessengerRef messenger);
+
+FLUTTER_EXPORT bool FlutterDesktopMessengerIsAvailable(
+    FlutterDesktopMessengerRef messenger);
+
+FLUTTER_EXPORT FlutterDesktopMessengerRef
+FlutterDesktopMessengerLock(FlutterDesktopMessengerRef messenger);
+
+FLUTTER_EXPORT void FlutterDesktopMessengerUnlock(
+    FlutterDesktopMessengerRef messenger);
+
 #if defined(__cplusplus)
 }  // extern "C"
 #endif

shell/platform/glfw/flutter_glfw.cc

@@ -1096,6 +1096,31 @@ void FlutterDesktopMessengerSetCallback(FlutterDesktopMessengerRef messenger,
                                                             user_data);
 }
 
+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) {
   std::cerr << "GLFW Texture support is not implemented yet." << std::endl;

shell/platform/windows/flutter_windows.cc

@@ -262,8 +262,8 @@ bool FlutterDesktopMessengerSendWithReply(FlutterDesktopMessengerRef messenger,
                                           const size_t message_size,
                                           const FlutterDesktopBinaryReply reply,
                                           void* user_data) {
-  return messenger->engine->SendPlatformMessage(channel, message, message_size,
-                                                reply, user_data);
+  return messenger->GetEngine()->SendPlatformMessage(
+      channel, message, message_size, reply, user_data);
 }
 
 bool FlutterDesktopMessengerSend(FlutterDesktopMessengerRef messenger,
@@ -279,15 +279,39 @@ void FlutterDesktopMessengerSendResponse(
     const FlutterDesktopMessageResponseHandle* handle,
     const uint8_t* data,
     size_t data_length) {
-  messenger->engine->SendPlatformMessageResponse(handle, data, data_length);
+  messenger->GetEngine()->SendPlatformMessageResponse(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);
+}
+
+FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef(
+    FlutterDesktopMessengerRef messenger) {
+  return messenger->AddRef();
+}
+
+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();
 }
 
 FlutterDesktopTextureRegistrarRef FlutterDesktopRegistrarGetTextureRegistrar(

shell/platform/windows/flutter_windows_engine.cc

@@ -175,8 +175,9 @@ FlutterWindowsEngine::FlutterWindowsEngine(
           });
 
   // Set up the legacy structs backing the API handles.
-  messenger_ = std::make_unique<FlutterDesktopMessenger>();
-  messenger_->engine = this;
+  messenger_ =
+      fml::RefPtr<FlutterDesktopMessenger>(new FlutterDesktopMessenger());
+  messenger_->SetEngine(this);
   plugin_registrar_ = std::make_unique<FlutterDesktopPluginRegistrar>();
   plugin_registrar_->engine = this;
 
@@ -198,6 +199,7 @@ FlutterWindowsEngine::FlutterWindowsEngine(
 }
 
 FlutterWindowsEngine::~FlutterWindowsEngine() {
+  messenger_->SetEngine(nullptr);
   Stop();
 }
 

shell/platform/windows/flutter_windows_engine.h

@@ -271,7 +271,7 @@ class FlutterWindowsEngine {
   std::unique_ptr<TaskRunner> task_runner_;
 
   // The plugin messenger handle given to API clients.
-  std::unique_ptr<FlutterDesktopMessenger> messenger_;
+  fml::RefPtr<FlutterDesktopMessenger> messenger_;
 
   // A wrapper around messenger_ for interacting with client_wrapper-level APIs.
   std::unique_ptr<BinaryMessengerImpl> messenger_wrapper_;

shell/platform/windows/window_state.h

@@ -5,6 +5,8 @@
 #ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOW_STATE_H_
 #define FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOW_STATE_H_
 
+#include <mutex>
+
 #include "flutter/shell/platform/common/client_wrapper/include/flutter/plugin_registrar.h"
 #include "flutter/shell/platform/common/incoming_message_dispatcher.h"
 #include "flutter/shell/platform/embedder/embedder.h"
@@ -36,8 +38,31 @@ struct FlutterDesktopPluginRegistrar {
 // Wrapper to distinguish the messenger ref from the engine ref given out
 // in the C API.
 struct FlutterDesktopMessenger {
+ public:
+  FlutterDesktopMessenger() = default;
+  flutter::FlutterWindowsEngine* GetEngine() const { return engine; }
+  void SetEngine(flutter::FlutterWindowsEngine* arg_engine) {
+    std::scoped_lock lock(mutex_);
+    engine = arg_engine;
+  }
+  FlutterDesktopMessenger* AddRef() {
+    ref_count_.fetch_add(1);
+    return this;
+  }
+  void Release() {
+    int32_t old_count = ref_count_.fetch_sub(1);
+    if (old_count <= 1) {
+      delete this;
+    }
+  }
+  std::mutex& GetMutex() { return mutex_; }
+
+ private:
+  FML_DISALLOW_COPY_AND_ASSIGN(FlutterDesktopMessenger);
   // The engine that owns this state object.
   flutter::FlutterWindowsEngine* engine = nullptr;
+  std::mutex mutex_;
+  std::atomic<int32_t> ref_count_ = 0;
 };
 
 #endif  // FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOW_STATE_H_