Made platform message responses threadsafe for macos. (flutter#37607)

Author: gaaclarke

shell/platform/darwin/macos/framework/Source/FlutterEngine.mm

@@ -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.
  */
@@ -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);
@@ -262,6 +271,10 @@ - (instancetype)initWithName:(NSString*)labelPrefix
 }
 
 - (void)dealloc {
+  @synchronized(_isResponseValid) {
+    [_isResponseValid removeAllObjects];
+    [_isResponseValid addObject:@NO];
+  }
   [self shutDownEngine];
   if (_aotData) {
     _embedderAPI.CollectAOTData(_aotData);
@@ -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);
+      }
     }
   };
 

shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm

@@ -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)

shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.h

@@ -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_;
 

shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.mm

@@ -31,6 +31,11 @@
   native_resolver_.reset();
 }
 
+void FlutterEngineTest::ShutDownEngine() {
+  [engine_ shutDownEngine];
+  engine_ = nil;
+}
+
 void FlutterEngineTest::IsolateCreateCallback(void* user_data) {
   native_resolver_->SetNativeResolverForIsolate();
 }

shell/platform/darwin/macos/framework/Source/fixtures/flutter_desktop_test.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:io';
+import 'dart:typed_data';
 import 'dart:ui';
 
 @pragma('vm:external-name', 'SignalNativeTest')
@@ -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) {});
+}