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

Commit 3c9cc06

Browse files
authored
[macos] FlutterKeyboardManager memory leak fix (#48824)
We are embedding Flutter into MacOS app, and noticed that there is a leak. `leaks` tool says there is a cycle reference: ``` 12 (2.11K) ROOT CYCLE: <FlutterKeyboardManager 0x29ec55f40> [80] 8 (432 bytes) __strong _primaryResponders --> ROOT CYCLE: <NSMutableArray 0x29ec560a0> [64] 7 (368 bytes) ROOT CYCLE: <NSMutableArray (Storage) 0x29ec56140> [32] 4 (224 bytes) ROOT CYCLE: <FlutterEmbedderKeyResponder 0x29ec56310> [80] 1 (48 bytes) __strong _sendEvent --> ROOT CYCLE: <__NSMallocBlock__ 0x29ec56360> [48] __strong [capture] --> CYCLE BACK TO <FlutterKeyboardManager 0x29ec55f40> [80] 1 (48 bytes) __strong _pendingResponses --> <NSMutableDictionary 0x29ec563c0> [48] 1 (48 bytes) __strong _pressingRecords --> <NSMutableDictionary 0x29ec56390> [48] 2 (112 bytes) <FlutterChannelKeyResponder 0x29ec56450> [48] 1 (64 bytes) __strong _channel --> <FlutterBasicMessageChannel 0x29ec564e0> [64] 2 (1.55K) __strong _layoutMap --> <NSMutableDictionary 0x29ec56630> [48] 1 (1.50K) <NSMutableDictionary (Storage) 0x123c34a00> [1536] 1 (64 bytes) __strong _pendingEvents --> <NSMutableArray 0x29ec565f0> [64] ``` This patch uses `weak` pointer to `self` instead of implicit `strong`. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing.
1 parent ecb90ab commit 3c9cc06

File tree

2 files changed

+43
-4
lines changed

2 files changed

+43
-4
lines changed

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,18 +123,24 @@ - (nonnull instancetype)initWithViewDelegate:(nonnull id<FlutterKeyboardViewDele
123123
[FlutterMethodChannel methodChannelWithName:@"flutter/keyboard"
124124
binaryMessenger:[_viewDelegate getBinaryMessenger]
125125
codec:[FlutterStandardMethodCodec sharedInstance]];
126+
126127
[keyboardChannel setMethodCallHandler:^(FlutterMethodCall* call, FlutterResult result) {
127128
[self handleKeyboardMethodCall:call result:result];
128129
}];
130+
129131
_primaryResponders = [[NSMutableArray alloc] init];
132+
133+
__weak __typeof__(self) weakSelf = self;
130134
[self addPrimaryResponder:[[FlutterEmbedderKeyResponder alloc]
131135
initWithSendEvent:^(const FlutterKeyEvent& event,
132136
FlutterKeyEventCallback callback,
133137
void* userData) {
134-
[_viewDelegate sendKeyEvent:event
135-
callback:callback
136-
userData:userData];
138+
__strong __typeof__(weakSelf) strongSelf = weakSelf;
139+
[strongSelf.viewDelegate sendKeyEvent:event
140+
callback:callback
141+
userData:userData];
137142
}]];
143+
138144
[self
139145
addPrimaryResponder:[[FlutterChannelKeyResponder alloc]
140146
initWithChannel:[FlutterBasicMessageChannel
@@ -143,14 +149,14 @@ - (nonnull instancetype)initWithViewDelegate:(nonnull id<FlutterKeyboardViewDele
143149
getBinaryMessenger]
144150
codec:[FlutterJSONMessageCodec
145151
sharedInstance]]]];
152+
146153
_pendingEvents = [[NSMutableArray alloc] init];
147154
_layoutMap = [NSMutableDictionary<NSNumber*, NSNumber*> dictionary];
148155
[self buildLayout];
149156
for (id<FlutterKeyPrimaryResponder> responder in _primaryResponders) {
150157
responder.layoutMap = _layoutMap;
151158
}
152159

153-
__weak __typeof__(self) weakSelf = self;
154160
[_viewDelegate subscribeToKeyboardLayoutChange:^() {
155161
[weakSelf buildLayout];
156162
}];

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,9 +426,11 @@ - (bool)getPressedState;
426426
- (bool)keyboardChannelGetPressedState;
427427
- (bool)racingConditionBetweenKeyAndText;
428428
- (bool)correctLogicalKeyForLayouts;
429+
- (bool)shouldNotHoldStrongReferenceToViewDelegate;
429430
@end
430431

431432
namespace flutter::testing {
433+
432434
TEST(FlutterKeyboardManagerUnittests, SinglePrimaryResponder) {
433435
ASSERT_TRUE([[FlutterKeyboardManagerUnittestsObjC alloc] singlePrimaryResponder]);
434436
}
@@ -461,6 +463,11 @@ - (bool)correctLogicalKeyForLayouts;
461463
ASSERT_TRUE([[FlutterKeyboardManagerUnittestsObjC alloc] correctLogicalKeyForLayouts]);
462464
}
463465

466+
TEST(FlutterKeyboardManagerUnittests, ShouldNotHoldStrongReferenceToViewDelegate) {
467+
ASSERT_TRUE(
468+
[[FlutterKeyboardManagerUnittestsObjC alloc] shouldNotHoldStrongReferenceToViewDelegate]);
469+
}
470+
464471
} // namespace flutter::testing
465472

466473
@implementation FlutterKeyboardManagerUnittestsObjC
@@ -809,4 +816,30 @@ - (bool)correctLogicalKeyForLayouts {
809816
return TRUE;
810817
}
811818

819+
- (bool)shouldNotHoldStrongReferenceToViewDelegate {
820+
__strong FlutterKeyboardManager* strongKeyboardManager;
821+
__weak id weakViewDelegate;
822+
823+
@autoreleasepool {
824+
id binaryMessengerMock = OCMStrictProtocolMock(@protocol(FlutterBinaryMessenger));
825+
OCMStub([binaryMessengerMock setMessageHandlerOnChannel:[OCMArg any]
826+
binaryMessageHandler:[OCMArg any]]);
827+
828+
id viewDelegateMock = OCMStrictProtocolMock(@protocol(FlutterKeyboardViewDelegate));
829+
OCMStub([viewDelegateMock getBinaryMessenger]).andReturn(binaryMessengerMock);
830+
OCMStub([viewDelegateMock subscribeToKeyboardLayoutChange:[OCMArg any]]);
831+
832+
LayoutClue layoutClue;
833+
OCMStub([viewDelegateMock lookUpLayoutForKeyCode:0 shift:NO])
834+
.ignoringNonObjectArgs()
835+
.andReturn(layoutClue);
836+
FlutterKeyboardManager* keyboardManager =
837+
[[FlutterKeyboardManager alloc] initWithViewDelegate:viewDelegateMock];
838+
strongKeyboardManager = keyboardManager;
839+
weakViewDelegate = viewDelegateMock;
840+
}
841+
842+
return weakViewDelegate == nil;
843+
}
844+
812845
@end

0 commit comments

Comments
 (0)