diff --git a/shell/platform/darwin/common/framework/Source/FlutterBinaryMessengerRelay.h b/shell/platform/darwin/common/framework/Source/FlutterBinaryMessengerRelay.h index 1c17329498873..392d6bac3775a 100644 --- a/shell/platform/darwin/common/framework/Source/FlutterBinaryMessengerRelay.h +++ b/shell/platform/darwin/common/framework/Source/FlutterBinaryMessengerRelay.h @@ -12,7 +12,7 @@ FLUTTER_DARWIN_EXPORT #endif @interface FlutterBinaryMessengerRelay : NSObject -@property(nonatomic, assign) NSObject* parent; +@property(nonatomic, weak) NSObject* parent; - (instancetype)initWithParent:(NSObject*)parent; @end diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index f6764167dec4f..648c5d213b08c 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -13,6 +13,7 @@ #include "flutter/shell/platform/common/engine_switches.h" #include "flutter/shell/platform/embedder/embedder.h" +#import "flutter/shell/platform/darwin/common/framework/Source/FlutterBinaryMessengerRelay.h" #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterAppDelegate.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterAppDelegate_Internal.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterCompositor.h" @@ -402,6 +403,9 @@ @implementation FlutterEngine { // Whether any portion of the application is currently visible. BOOL _visible; + + // Proxy to allow plugins, channels to hold a weak reference to the binary messenger (self). + FlutterBinaryMessengerRelay* _binaryMessenger; } - (instancetype)initWithName:(NSString*)labelPrefix project:(FlutterDartProject*)project { @@ -420,6 +424,7 @@ - (instancetype)initWithName:(NSString*)labelPrefix _currentMessengerConnection = 1; _allowHeadlessExecution = allowHeadlessExecution; _semanticsEnabled = NO; + _binaryMessenger = [[FlutterBinaryMessengerRelay alloc] initWithParent:self]; _isResponseValid = [[NSMutableArray alloc] initWithCapacity:1]; [_isResponseValid addObject:@YES]; // kFlutterImplicitViewId is reserved for the implicit view. @@ -723,9 +728,7 @@ - (FlutterCompositor*)createFlutterCompositor { } - (id)binaryMessenger { - // TODO(stuartmorgan): Switch to FlutterBinaryMessengerRelay to avoid plugins - // keeping the engine alive. - return self; + return _binaryMessenger; } #pragma mark - Framework-internal methods diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm index fea57a9adf40f..9b3468a33c03c 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEngineTest.mm @@ -2,10 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h" -#include "gtest/gtest.h" + +#include #include #include @@ -14,6 +14,7 @@ #include "flutter/lib/ui/window/platform_message.h" #include "flutter/shell/platform/common/accessibility_bridge.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h" +#import "flutter/shell/platform/darwin/common/framework/Source/FlutterBinaryMessengerRelay.h" #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterAppDelegate.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngineTestUtils.h" #import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTestUtils.h" @@ -21,6 +22,7 @@ #include "flutter/shell/platform/embedder/embedder_engine.h" #include "flutter/shell/platform/embedder/test_utils/proc_table_replacement.h" #include "flutter/testing/test_dart_native_resolver.h" +#include "gtest/gtest.h" // CREATE_NATIVE_ENTRY and MOCK_ENGINE_PROC are leaky by design // NOLINTBEGIN(clang-analyzer-core.StackAddressEscape) @@ -496,6 +498,38 @@ - (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication* _Nonnu EXPECT_TRUE(called); } +// Verify that the engine is not retained indirectly via the binary messenger held by channels and +// plugins. Previously, FlutterEngine.binaryMessenger returned the engine itself, and thus plugins +// could cause a retain cycle, preventing the engine from being deallocated. +// FlutterEngine.binaryMessenger now returns a FlutterBinaryMessengerRelay whose pointer back to +// the engine is cleared when the engine is deallocated. +// Issue: https://github.com/flutter/flutter/issues/116445 +TEST_F(FlutterEngineTest, FlutterBinaryMessengerNullsParentOnEngineRelease) { + FlutterBinaryMessengerRelay* relay = nil; + @autoreleasepool { + // Create a test engine. + NSString* fixtures = @(flutter::testing::GetFixturesPath()); + FlutterDartProject* project = [[FlutterDartProject alloc] + initWithAssetsPath:fixtures + ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]]; + FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"test" + project:project + allowHeadlessExecution:true]; + + // Get the binary messenger for the engine. + id binaryMessenger = engine.binaryMessenger; + ASSERT_TRUE([binaryMessenger isKindOfClass:[FlutterBinaryMessengerRelay class]]); + relay = (FlutterBinaryMessengerRelay*)binaryMessenger; + + // Verify the relay parent (the engine) is non-nil. + EXPECT_NE(relay.parent, nil); + } + + // Once the engine has been deallocated, verify the relay parent is nil, and thus the engine is + // not retained by the holder of the relay. + EXPECT_EQ(relay.parent, nil); +} + // If a channel overrides a previous channel with the same name, cleaning // the previous channel should not affect the new channel. //