Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Implemented threadsafe platform channel replies on windows #36909

1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 19 additions & 3 deletions shell/platform/common/client_wrapper/core_implementations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand All @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional nit: this block is entirely local, so could be done before acquiring the lock instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsAvailable has to happen inside the safety of the lock.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comment was on this code:

    if (!response_handle) {
      std::cerr << "Error: Response can be set only once. Ignoring "
                   "duplicate response."
                << std::endl;
      return;
    }

which doesn't have any calls to IsAvailable.

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;
Expand Down
27 changes: 27 additions & 0 deletions shell/platform/common/client_wrapper/testing/stub_flutter_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -93,6 +95,31 @@ void FlutterDesktopMessengerSetCallback(FlutterDesktopMessengerRef messenger,
}
}

FlutterDesktopMessengerRef FlutterDesktopMessengerAddRef(
FlutterDesktopMessengerRef messenger) {
assert(false); // not implemented
return nullptr;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No existing tests for the client_wrapper actually hit this code. In terms of code coverage it isn't strictly needed since I have coverage in the windows unit tests.

}

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);
Expand Down
48 changes: 48 additions & 0 deletions shell/platform/common/public/flutter_messenger.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions shell/platform/embedder/embedder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
96 changes: 85 additions & 11 deletions shell/platform/glfw/flutter_glfw.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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(
Expand Down
Loading