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

Made platform message responses thread-safe for macos. #37607

Merged
merged 1 commit into from
Nov 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 31 additions & 10 deletions shell/platform/darwin/macos/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ - (instancetype)initWithConnection:(NSNumber*)connection
*/
@interface FlutterEngine () <FlutterBinaryMessenger>

/**
* A mutable array that holds one bool value that determines if responses to platform messages are
* clear to execute. This value should be read or written only inside of a synchronized block and
* will return `NO` after the FlutterEngine has been dealloc'd.
*/
@property(nonatomic, strong) NSMutableArray<NSNumber*>* isResponseValid;

/**
* Sends the list of user-preferred locales to the Flutter engine.
*/
Expand Down Expand Up @@ -242,6 +249,8 @@ - (instancetype)initWithName:(NSString*)labelPrefix
_allowHeadlessExecution = allowHeadlessExecution;
_semanticsEnabled = NO;
_viewProvider = [[FlutterViewEngineProvider alloc] initWithEngine:self];
_isResponseValid = [[NSMutableArray alloc] initWithCapacity:1];
[_isResponseValid addObject:@YES];

_embedderAPI.struct_size = sizeof(FlutterEngineProcTable);
FlutterEngineGetProcAddresses(&_embedderAPI);
Expand All @@ -262,6 +271,10 @@ - (instancetype)initWithName:(NSString*)labelPrefix
}

- (void)dealloc {
@synchronized(_isResponseValid) {
[_isResponseValid removeAllObjects];
[_isResponseValid addObject:@NO];
}
[self shutDownEngine];
if (_aotData) {
_embedderAPI.CollectAOTData(_aotData);
Expand Down Expand Up @@ -639,17 +652,25 @@ - (void)engineCallbackOnPlatformMessage:(const FlutterPlatformMessage*)message {
}
NSString* channel = @(message->channel);
__block const FlutterPlatformMessageResponseHandle* responseHandle = message->response_handle;

__block FlutterEngine* weakSelf = self;
NSMutableArray* isResponseValid = self.isResponseValid;
FlutterEngineSendPlatformMessageResponseFnPtr sendPlatformMessageResponse =
_embedderAPI.SendPlatformMessageResponse;
FlutterBinaryReply binaryResponseHandler = ^(NSData* response) {
if (responseHandle) {
_embedderAPI.SendPlatformMessageResponse(self->_engine, responseHandle,
static_cast<const uint8_t*>(response.bytes),
response.length);
responseHandle = NULL;
} else {
NSLog(@"Error: Message responses can be sent only once. Ignoring duplicate response "
"on channel '%@'.",
channel);
@synchronized(isResponseValid) {
if (![isResponseValid[0] boolValue]) {
// Ignore, engine was killed.
return;
}
if (responseHandle) {
sendPlatformMessageResponse(weakSelf->_engine, responseHandle,
static_cast<const uint8_t*>(response.bytes), response.length);
responseHandle = NULL;
} else {
NSLog(@"Error: Message responses can be sent only once. Ignoring duplicate response "
"on channel '%@'.",
channel);
}
}
};

Expand Down
42 changes: 42 additions & 0 deletions shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,48 @@ @interface FlutterEngine (Test)
EXPECT_TRUE(value);
}

TEST_F(FlutterEngineTest, ResponseAfterEngineDied) {
FlutterEngine* engine = GetFlutterEngine();
FlutterBasicMessageChannel* channel = [[FlutterBasicMessageChannel alloc]
initWithName:@"foo"
binaryMessenger:engine.binaryMessenger
codec:[FlutterStandardMessageCodec sharedInstance]];
__block BOOL didCallCallback = NO;
[channel setMessageHandler:^(id message, FlutterReply callback) {
ShutDownEngine();
callback(nil);
didCallCallback = YES;
}];
EXPECT_TRUE([engine runWithEntrypoint:@"sendFooMessage"]);
engine = nil;

while (!didCallCallback) {
[[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.1]];
}
}

TEST_F(FlutterEngineTest, ResponseFromBackgroundThread) {
FlutterEngine* engine = GetFlutterEngine();
FlutterBasicMessageChannel* channel = [[FlutterBasicMessageChannel alloc]
initWithName:@"foo"
binaryMessenger:engine.binaryMessenger
codec:[FlutterStandardMessageCodec sharedInstance]];
__block BOOL didCallCallback = NO;
[channel setMessageHandler:^(id message, FlutterReply callback) {
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
callback(nil);
dispatch_async(dispatch_get_main_queue(), ^{
didCallCallback = YES;
});
});
}];
EXPECT_TRUE([engine runWithEntrypoint:@"sendFooMessage"]);

while (!didCallCallback) {
[[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.1]];
}
}

} // namespace flutter::testing

// NOLINTEND(clang-analyzer-core.StackAddressEscape)
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ class FlutterEngineTest : public ::testing::Test {

static void IsolateCreateCallback(void* user_data);

void ShutDownEngine();

private:
inline static std::shared_ptr<TestDartNativeResolver> native_resolver_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
native_resolver_.reset();
}

void FlutterEngineTest::ShutDownEngine() {
[engine_ shutDownEngine];
engine_ = nil;
}

void FlutterEngineTest::IsolateCreateCallback(void* user_data) {
native_resolver_->SetNativeResolverForIsolate();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

import 'dart:io';
import 'dart:typed_data';
import 'dart:ui';

@pragma('vm:external-name', 'SignalNativeTest')
Expand Down Expand Up @@ -63,3 +64,8 @@ void backgroundTest() {
PlatformDispatcher.instance.views.first.render(SceneBuilder().build());
signalNativeTest(); // should look black
}

@pragma('vm:entry-point')
void sendFooMessage() {
PlatformDispatcher.instance.sendPlatformMessage('foo', null, (ByteData? result) {});
}