Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Next Next commit
Implemented threadsafe platform channel replies on windows
  • Loading branch information
gaaclarke committed Nov 9, 2022
commit e64a9ae2bd28b494e9a6b4c3ce6620ee0cf7a12c
19 changes: 16 additions & 3 deletions shell/platform/common/client_wrapper/core_implementations.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
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
16 changes: 16 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,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
Expand Down
25 changes: 25 additions & 0 deletions shell/platform/glfw/flutter_glfw.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
34 changes: 29 additions & 5 deletions shell/platform/windows/flutter_windows.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand Down
6 changes: 4 additions & 2 deletions shell/platform/windows/flutter_windows_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,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;

Expand Down Expand Up @@ -210,6 +211,7 @@ FlutterWindowsEngine::FlutterWindowsEngine(
}

FlutterWindowsEngine::~FlutterWindowsEngine() {
messenger_->SetEngine(nullptr);
Stop();
}

Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/flutter_windows_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,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_;
Expand Down
25 changes: 25 additions & 0 deletions shell/platform/windows/window_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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_