From 803f7f60e6cdfe94e44aa0867443738c70ecb093 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 14 Jun 2019 09:38:36 -0700 Subject: [PATCH 1/6] Fixed memory leak by way of accidental retain on implicit self. --- .../darwin/ios/framework/Source/FlutterChannels.mm | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm b/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm index 732c983f0e3b1..23eaceb6874f7 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm @@ -208,15 +208,17 @@ - (void)setMethodCallHandler:(FlutterMethodCallHandler)handler { [_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil]; return; } + // Make sure the block captures the codec, not self. + NSObject* codec = _codec; FlutterBinaryMessageHandler messageHandler = ^(NSData* message, FlutterBinaryReply callback) { - FlutterMethodCall* call = [_codec decodeMethodCall:message]; + FlutterMethodCall* call = [codec decodeMethodCall:message]; handler(call, ^(id result) { if (result == FlutterMethodNotImplemented) callback(nil); else if ([result isKindOfClass:[FlutterError class]]) - callback([_codec encodeErrorEnvelope:(FlutterError*)result]); + callback([codec encodeErrorEnvelope:(FlutterError*)result]); else - callback([_codec encodeSuccessEnvelope:result]); + callback([codec encodeSuccessEnvelope:result]); }); }; [_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:messageHandler]; From f8dd3e5db8a023236a411ab6c0d488548e720d77 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 14 Jun 2019 10:30:31 -0700 Subject: [PATCH 2/6] Fixed incorrect type. --- shell/platform/darwin/ios/framework/Source/FlutterChannels.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm b/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm index 23eaceb6874f7..9588817e8d2f7 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm @@ -209,7 +209,7 @@ - (void)setMethodCallHandler:(FlutterMethodCallHandler)handler { return; } // Make sure the block captures the codec, not self. - NSObject* codec = _codec; + NSObject* codec = _codec; FlutterBinaryMessageHandler messageHandler = ^(NSData* message, FlutterBinaryReply callback) { FlutterMethodCall* call = [codec decodeMethodCall:message]; handler(call, ^(id result) { From 31dc79575e2420ffe264d2a8b1bc8d89d9a48ea7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 14 Jun 2019 14:14:12 -0700 Subject: [PATCH 3/6] Fixed the memory leak in FlutterBasicMessageChannel and FlutterEventChannel, too. --- .../ios/framework/Source/FlutterChannels.mm | 37 +++++++++++-------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm b/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm index 9588817e8d2f7..edc34daba2473 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterChannels.mm @@ -61,9 +61,11 @@ - (void)setMessageHandler:(FlutterMessageHandler)handler { [_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil]; return; } + // Grab reference to avoid retain on self. + NSObject* codec = _codec; FlutterBinaryMessageHandler messageHandler = ^(NSData* message, FlutterBinaryReply callback) { - handler([_codec decode:message], ^(id reply) { - callback([_codec encode:reply]); + handler([codec decode:message], ^(id reply) { + callback([codec encode:reply]); }); }; [_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:messageHandler]; @@ -271,8 +273,11 @@ - (void)setStreamHandler:(NSObject*)handler { return; } __block FlutterEventSink currentSink = nil; + // self retains _messenger, _messenger retains this block, so it is safe to use a weak reference + // to self. + __block FlutterEventChannel* blockSelf = self; FlutterBinaryMessageHandler messageHandler = ^(NSData* message, FlutterBinaryReply callback) { - FlutterMethodCall* call = [_codec decodeMethodCall:message]; + FlutterMethodCall* call = [blockSelf->_codec decodeMethodCall:message]; if ([call.method isEqual:@"listen"]) { if (currentSink) { FlutterError* error = [handler onCancelWithArguments:nil]; @@ -282,32 +287,34 @@ - (void)setStreamHandler:(NSObject*)handler { } currentSink = ^(id event) { if (event == FlutterEndOfEventStream) - [_messenger sendOnChannel:_name message:nil]; + [blockSelf->_messenger sendOnChannel:blockSelf->_name message:nil]; else if ([event isKindOfClass:[FlutterError class]]) - [_messenger sendOnChannel:_name - message:[_codec encodeErrorEnvelope:(FlutterError*)event]]; + [blockSelf->_messenger + sendOnChannel:blockSelf->_name + message:[blockSelf->_codec encodeErrorEnvelope:(FlutterError*)event]]; else - [_messenger sendOnChannel:_name message:[_codec encodeSuccessEnvelope:event]]; + [blockSelf->_messenger sendOnChannel:blockSelf->_name + message:[blockSelf->_codec encodeSuccessEnvelope:event]]; }; FlutterError* error = [handler onListenWithArguments:call.arguments eventSink:currentSink]; if (error) - callback([_codec encodeErrorEnvelope:error]); + callback([blockSelf->_codec encodeErrorEnvelope:error]); else - callback([_codec encodeSuccessEnvelope:nil]); + callback([blockSelf->_codec encodeSuccessEnvelope:nil]); } else if ([call.method isEqual:@"cancel"]) { if (!currentSink) { - callback( - [_codec encodeErrorEnvelope:[FlutterError errorWithCode:@"error" - message:@"No active stream to cancel" - details:nil]]); + callback([blockSelf->_codec + encodeErrorEnvelope:[FlutterError errorWithCode:@"error" + message:@"No active stream to cancel" + details:nil]]); return; } currentSink = nil; FlutterError* error = [handler onCancelWithArguments:call.arguments]; if (error) - callback([_codec encodeErrorEnvelope:error]); + callback([blockSelf->_codec encodeErrorEnvelope:error]); else - callback([_codec encodeSuccessEnvelope:nil]); + callback([blockSelf->_codec encodeSuccessEnvelope:nil]); } else { callback(nil); } From 3a85a85ab3cbae0837f00bbbc3ae8a85d851d2be Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 27 Jun 2019 15:53:14 -0700 Subject: [PATCH 4/6] Made sure that the stream channels can still function beyond being deallocated to match behaviour of other channels. --- .../framework/Source/FlutterChannels.mm | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/shell/platform/darwin/common/framework/Source/FlutterChannels.mm b/shell/platform/darwin/common/framework/Source/FlutterChannels.mm index 612b4cfc7aaf2..38e1dba8927cc 100644 --- a/shell/platform/darwin/common/framework/Source/FlutterChannels.mm +++ b/shell/platform/darwin/common/framework/Source/FlutterChannels.mm @@ -267,17 +267,15 @@ - (void)dealloc { [super dealloc]; } -- (void)setStreamHandler:(NSObject*)handler { - if (!handler) { - [_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil]; - return; - } +static void SetStreamHandlerMessageHandlerOnChannel(NSObject* handler, + NSString* name, + NSObject* messenger, + NSObject* codec) { __block FlutterEventSink currentSink = nil; // self retains _messenger, _messenger retains this block, so it is safe to use a weak reference // to self. - __block FlutterEventChannel* blockSelf = self; FlutterBinaryMessageHandler messageHandler = ^(NSData* message, FlutterBinaryReply callback) { - FlutterMethodCall* call = [blockSelf->_codec decodeMethodCall:message]; + FlutterMethodCall* call = [codec decodeMethodCall:message]; if ([call.method isEqual:@"listen"]) { if (currentSink) { FlutterError* error = [handler onCancelWithArguments:nil]; @@ -287,38 +285,43 @@ - (void)setStreamHandler:(NSObject*)handler { } currentSink = ^(id event) { if (event == FlutterEndOfEventStream) - [blockSelf->_messenger sendOnChannel:blockSelf->_name message:nil]; + [messenger sendOnChannel:name message:nil]; else if ([event isKindOfClass:[FlutterError class]]) - [blockSelf->_messenger - sendOnChannel:blockSelf->_name - message:[blockSelf->_codec encodeErrorEnvelope:(FlutterError*)event]]; + [messenger sendOnChannel:name message:[codec encodeErrorEnvelope:(FlutterError*)event]]; else - [blockSelf->_messenger sendOnChannel:blockSelf->_name - message:[blockSelf->_codec encodeSuccessEnvelope:event]]; + [messenger sendOnChannel:name message:[codec encodeSuccessEnvelope:event]]; }; FlutterError* error = [handler onListenWithArguments:call.arguments eventSink:currentSink]; if (error) - callback([blockSelf->_codec encodeErrorEnvelope:error]); + callback([codec encodeErrorEnvelope:error]); else - callback([blockSelf->_codec encodeSuccessEnvelope:nil]); + callback([codec encodeSuccessEnvelope:nil]); } else if ([call.method isEqual:@"cancel"]) { if (!currentSink) { - callback([blockSelf->_codec - encodeErrorEnvelope:[FlutterError errorWithCode:@"error" - message:@"No active stream to cancel" - details:nil]]); + callback( + [codec encodeErrorEnvelope:[FlutterError errorWithCode:@"error" + message:@"No active stream to cancel" + details:nil]]); return; } currentSink = nil; FlutterError* error = [handler onCancelWithArguments:call.arguments]; if (error) - callback([blockSelf->_codec encodeErrorEnvelope:error]); + callback([codec encodeErrorEnvelope:error]); else - callback([blockSelf->_codec encodeSuccessEnvelope:nil]); + callback([codec encodeSuccessEnvelope:nil]); } else { callback(nil); } }; - [_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:messageHandler]; + [messenger setMessageHandlerOnChannel:name binaryMessageHandler:messageHandler]; +} + +- (void)setStreamHandler:(NSObject*)handler { + if (!handler) { + [_messenger setMessageHandlerOnChannel:_name binaryMessageHandler:nil]; + return; + } + SetStreamHandlerMessageHandlerOnChannel(handler, _name, _messenger, _codec); } @end From 9ab46875e9b14d9f9c3651e0c96ce25897a3a0b7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 27 Jun 2019 17:45:57 -0700 Subject: [PATCH 5/6] Switched preprocessor logic for exporting symbols for testing. (NDEBUG wasn't working for Luci builds). --- .../darwin/ios/framework/Source/FlutterBinaryMessengerRelay.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterBinaryMessengerRelay.h b/shell/platform/darwin/ios/framework/Source/FlutterBinaryMessengerRelay.h index df668dd2a341c..af5c82f2012a1 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterBinaryMessengerRelay.h +++ b/shell/platform/darwin/ios/framework/Source/FlutterBinaryMessengerRelay.h @@ -5,7 +5,7 @@ #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterBinaryMessenger.h" #import "flutter/shell/platform/darwin/common/framework/Headers/FlutterMacros.h" -#ifndef NDEBUG +#if FLUTTER_RUNTIME_MODE == FLUTTER_RUNTIME_MODE_DEBUG FLUTTER_EXPORT #endif @interface FlutterBinaryMessengerRelay : NSObject From 3f8addf652ded87d205010c5818ab5a6699faf97 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 28 Jun 2019 09:47:50 -0700 Subject: [PATCH 6/6] removed stray comment --- .../platform/darwin/common/framework/Source/FlutterChannels.mm | 2 -- 1 file changed, 2 deletions(-) diff --git a/shell/platform/darwin/common/framework/Source/FlutterChannels.mm b/shell/platform/darwin/common/framework/Source/FlutterChannels.mm index 38e1dba8927cc..33ff2da28dcc7 100644 --- a/shell/platform/darwin/common/framework/Source/FlutterChannels.mm +++ b/shell/platform/darwin/common/framework/Source/FlutterChannels.mm @@ -272,8 +272,6 @@ static void SetStreamHandlerMessageHandlerOnChannel(NSObject* messenger, NSObject* codec) { __block FlutterEventSink currentSink = nil; - // self retains _messenger, _messenger retains this block, so it is safe to use a weak reference - // to self. FlutterBinaryMessageHandler messageHandler = ^(NSData* message, FlutterBinaryReply callback) { FlutterMethodCall* call = [codec decodeMethodCall:message]; if ([call.method isEqual:@"listen"]) {