diff --git a/packages/google_maps_flutter/google_maps_flutter/AUTHORS b/packages/google_maps_flutter/google_maps_flutter/AUTHORS index 9f1b53ee2667..82923e29f668 100644 --- a/packages/google_maps_flutter/google_maps_flutter/AUTHORS +++ b/packages/google_maps_flutter/google_maps_flutter/AUTHORS @@ -65,3 +65,4 @@ Anton Borries Alex Li Rahul Raj <64.rahulraj@gmail.com> Taha Tesser +Patrick Wolf diff --git a/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md b/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md index 6edda3e08049..8ed643b9f543 100644 --- a/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md +++ b/packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md @@ -1,3 +1,9 @@ +## 2.3.2 + +* Fixes memory leak by implementing mechanism for disposing stream subscriptions in `GoogleMapController`. + Previously, subscriptions were not canceled upon disposal of the controller, which could lead to memory leaks or + unexpected behavior. The controller now tracks all stream subscriptions and cancels them when `dispose()` is called. + ## 2.3.1 * Fixes a regression from 2.2.8 that could cause incorrect handling of a diff --git a/packages/google_maps_flutter/google_maps_flutter/lib/src/controller.dart b/packages/google_maps_flutter/google_maps_flutter/lib/src/controller.dart index dedb8400a32e..41f6d9128820 100644 --- a/packages/google_maps_flutter/google_maps_flutter/lib/src/controller.dart +++ b/packages/google_maps_flutter/google_maps_flutter/lib/src/controller.dart @@ -18,6 +18,35 @@ class GoogleMapController { /// The mapId for this controller final int mapId; + /// State map of the stream subscriptions for this controller, used internally. + /// + /// The map stores a [StreamSubscription] as key and a [bool] as value. + /// Each [StreamSubscription] represents a subscription to an event from the native side. + /// The corresponding [bool] indicates whether the subscription has been canceled `true` or not `false`. + /// + /// This map allows for efficient management of subscriptions, facilitating operations + /// like mass cancellation during controller disposal. + /// + /// This field is private to prevent external mutation. Access is provided through + /// the [streamSubscriptionsState] getter, which returns an unmodifiable view of this map. + final Map, bool> _streamSubscriptionsState = + , bool>{}; + + /// Returns an unmodifiable view of the map that holds the active StreamSubscriptions + /// and their corresponding states. + /// + /// This getter provides a way to inspect the current state of active StreamSubscriptions + /// in the [GoogleMapController] class without allowing any outside mutations. + /// + /// The map contains [StreamSubscription] instances as keys, and [bool] as values. + /// The value indicates if the subscription has been canceled `true` or not `false`. + /// + /// This is useful in situations where the need arises to monitor or debug the + /// lifecycle of various StreamSubscriptions handled within this class. + Map, bool> get streamSubscriptionsState => + Map, bool>.unmodifiable( + _streamSubscriptionsState); + /// Initialize control of a [GoogleMap] with [id]. /// /// Mainly for internal use when instantiating a [GoogleMapController] passed @@ -34,51 +63,112 @@ class GoogleMapController { ); } + /// The state object of the [GoogleMap] widget. + /// + /// This instance holds important properties and methods that are essential for + /// the behavior and state of the [GoogleMap] widget. final _GoogleMapState _googleMapState; + /// Subscribes to the various event streams from the [GoogleMapsFlutterPlatform] for a specific map. + /// + /// This method takes the `mapId` as an argument and uses it to subscribe to the various streams available from the [GoogleMapsFlutterPlatform]. + /// It only subscribes to the streams if the corresponding callback from the widget is not null. This includes streams for camera movements, + /// marker taps, marker drag events, info window taps, polyline taps, polygon taps, circle taps, map taps, and map long presses. + /// + /// Each subscription is added to [_streamSubscriptionsState] for lifecycle management. + /// + /// - [mapId] is the identifier of the map for which to initiate the streams. void _connectStreams(int mapId) { if (_googleMapState.widget.onCameraMoveStarted != null) { - GoogleMapsFlutterPlatform.instance - .onCameraMoveStarted(mapId: mapId) - .listen((_) => _googleMapState.widget.onCameraMoveStarted!()); + _addSubscription( + GoogleMapsFlutterPlatform.instance + .onCameraMoveStarted(mapId: mapId) + .listen((_) => _googleMapState.widget.onCameraMoveStarted!()), + ); } if (_googleMapState.widget.onCameraMove != null) { - GoogleMapsFlutterPlatform.instance.onCameraMove(mapId: mapId).listen( - (CameraMoveEvent e) => _googleMapState.widget.onCameraMove!(e.value)); + _addSubscription( + GoogleMapsFlutterPlatform.instance.onCameraMove(mapId: mapId).listen( + (CameraMoveEvent e) => + _googleMapState.widget.onCameraMove!(e.value), + ), + ); } if (_googleMapState.widget.onCameraIdle != null) { - GoogleMapsFlutterPlatform.instance - .onCameraIdle(mapId: mapId) - .listen((_) => _googleMapState.widget.onCameraIdle!()); + _addSubscription( + GoogleMapsFlutterPlatform.instance + .onCameraIdle(mapId: mapId) + .listen((_) => _googleMapState.widget.onCameraIdle!()), + ); } - GoogleMapsFlutterPlatform.instance - .onMarkerTap(mapId: mapId) - .listen((MarkerTapEvent e) => _googleMapState.onMarkerTap(e.value)); - GoogleMapsFlutterPlatform.instance.onMarkerDragStart(mapId: mapId).listen( - (MarkerDragStartEvent e) => - _googleMapState.onMarkerDragStart(e.value, e.position)); - GoogleMapsFlutterPlatform.instance.onMarkerDrag(mapId: mapId).listen( - (MarkerDragEvent e) => - _googleMapState.onMarkerDrag(e.value, e.position)); - GoogleMapsFlutterPlatform.instance.onMarkerDragEnd(mapId: mapId).listen( - (MarkerDragEndEvent e) => - _googleMapState.onMarkerDragEnd(e.value, e.position)); - GoogleMapsFlutterPlatform.instance.onInfoWindowTap(mapId: mapId).listen( - (InfoWindowTapEvent e) => _googleMapState.onInfoWindowTap(e.value)); - GoogleMapsFlutterPlatform.instance - .onPolylineTap(mapId: mapId) - .listen((PolylineTapEvent e) => _googleMapState.onPolylineTap(e.value)); - GoogleMapsFlutterPlatform.instance - .onPolygonTap(mapId: mapId) - .listen((PolygonTapEvent e) => _googleMapState.onPolygonTap(e.value)); - GoogleMapsFlutterPlatform.instance - .onCircleTap(mapId: mapId) - .listen((CircleTapEvent e) => _googleMapState.onCircleTap(e.value)); - GoogleMapsFlutterPlatform.instance - .onTap(mapId: mapId) - .listen((MapTapEvent e) => _googleMapState.onTap(e.position)); - GoogleMapsFlutterPlatform.instance.onLongPress(mapId: mapId).listen( - (MapLongPressEvent e) => _googleMapState.onLongPress(e.position)); + _addSubscription( + GoogleMapsFlutterPlatform.instance + .onMarkerTap(mapId: mapId) + .listen((MarkerTapEvent e) => _googleMapState.onMarkerTap(e.value)), + ); + _addSubscription( + GoogleMapsFlutterPlatform.instance.onMarkerDragStart(mapId: mapId).listen( + (MarkerDragStartEvent e) => + _googleMapState.onMarkerDragStart(e.value, e.position), + ), + ); + _addSubscription( + GoogleMapsFlutterPlatform.instance.onMarkerDrag(mapId: mapId).listen( + (MarkerDragEvent e) => + _googleMapState.onMarkerDrag(e.value, e.position), + ), + ); + _addSubscription( + GoogleMapsFlutterPlatform.instance.onMarkerDragEnd(mapId: mapId).listen( + (MarkerDragEndEvent e) => + _googleMapState.onMarkerDragEnd(e.value, e.position), + ), + ); + _addSubscription( + GoogleMapsFlutterPlatform.instance.onInfoWindowTap(mapId: mapId).listen( + (InfoWindowTapEvent e) => _googleMapState.onInfoWindowTap(e.value), + ), + ); + _addSubscription( + GoogleMapsFlutterPlatform.instance.onPolylineTap(mapId: mapId).listen( + (PolylineTapEvent e) => _googleMapState.onPolylineTap(e.value), + ), + ); + _addSubscription( + GoogleMapsFlutterPlatform.instance.onPolygonTap(mapId: mapId).listen( + (PolygonTapEvent e) => _googleMapState.onPolygonTap(e.value), + ), + ); + _addSubscription( + GoogleMapsFlutterPlatform.instance + .onCircleTap(mapId: mapId) + .listen((CircleTapEvent e) => _googleMapState.onCircleTap(e.value)), + ); + _addSubscription( + GoogleMapsFlutterPlatform.instance + .onTap(mapId: mapId) + .listen((MapTapEvent e) => _googleMapState.onTap(e.position)), + ); + _addSubscription( + GoogleMapsFlutterPlatform.instance.onLongPress(mapId: mapId).listen( + (MapLongPressEvent e) => _googleMapState.onLongPress(e.position), + ), + ); + } + + /// Adds a new stream subscription to the [_streamSubscriptionsState] map. + /// + /// The method takes a [StreamSubscription] and sets its status to `false` + /// in the [_streamSubscriptionsState] map. The `false` status indicates that the subscription + /// is currently active and has not been canceled yet. + /// + /// The [_streamSubscriptionsState] map manages all active stream subscriptions + /// associated with this instance, which facilitates their lifecycle management + /// and ensures that they are properly canceled and disposed when necessary. + /// + /// [subscription]: A [StreamSubscription] that will be managed by the [_streamSubscriptionsState] map. + void _addSubscription(StreamSubscription subscription) { + _streamSubscriptionsState[subscription] = false; } /// Updates configuration options of the map user interface. @@ -270,6 +360,13 @@ class GoogleMapController { /// Disposes of the platform resources void dispose() { + for (final StreamSubscription subscription + in _streamSubscriptionsState.keys) { + subscription.cancel().then((void value) { + _streamSubscriptionsState[subscription] = true; + }); + } + _streamSubscriptionsState.clear(); GoogleMapsFlutterPlatform.instance.dispose(mapId: mapId); } } diff --git a/packages/google_maps_flutter/google_maps_flutter/lib/src/google_map.dart b/packages/google_maps_flutter/google_maps_flutter/lib/src/google_map.dart index 08c2286527fb..1e8483b0d666 100644 --- a/packages/google_maps_flutter/google_maps_flutter/lib/src/google_map.dart +++ b/packages/google_maps_flutter/google_maps_flutter/lib/src/google_map.dart @@ -398,6 +398,9 @@ class _GoogleMapState extends State { } Future onPlatformViewCreated(int id) async { + if (!mounted) { + return; + } final GoogleMapController controller = await GoogleMapController.init( id, widget.initialCameraPosition, diff --git a/packages/google_maps_flutter/google_maps_flutter/pubspec.yaml b/packages/google_maps_flutter/google_maps_flutter/pubspec.yaml index e56ff513e7a4..26b4dfd930c6 100644 --- a/packages/google_maps_flutter/google_maps_flutter/pubspec.yaml +++ b/packages/google_maps_flutter/google_maps_flutter/pubspec.yaml @@ -2,7 +2,7 @@ name: google_maps_flutter description: A Flutter plugin for integrating Google Maps in iOS and Android applications. repository: https://github.com/flutter/packages/tree/main/packages/google_maps_flutter/google_maps_flutter issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+maps%22 -version: 2.3.1 +version: 2.3.2 environment: sdk: ">=3.0.0 <4.0.0" diff --git a/packages/google_maps_flutter/google_maps_flutter/test/stream_subscription_test.dart b/packages/google_maps_flutter/google_maps_flutter/test/stream_subscription_test.dart new file mode 100644 index 000000000000..1ca7bc1de649 --- /dev/null +++ b/packages/google_maps_flutter/google_maps_flutter/test/stream_subscription_test.dart @@ -0,0 +1,85 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:google_maps_flutter/google_maps_flutter.dart'; +import 'package:google_maps_flutter_platform_interface/google_maps_flutter_platform_interface.dart'; + +import 'fake_google_maps_flutter_platform.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + group('GoogleMap widget', () { + testWidgets('Subscriptions are canceled on dispose', + (WidgetTester tester) async { + GoogleMapsFlutterPlatform.instance = FakeGoogleMapsFlutterPlatform(); + + final FakePlatformViewsController fakePlatformViewsController = + FakePlatformViewsController(); + + tester.binding.defaultBinaryMessenger.setMockMethodCallHandler( + SystemChannels.platform_views, + fakePlatformViewsController.fakePlatformViewsMethodHandler, + ); + + final ValueNotifier controllerNotifier = + ValueNotifier(null); + + final GoogleMap googleMap = GoogleMap( + onMapCreated: (GoogleMapController controller) { + controllerNotifier.value = controller; + }, + initialCameraPosition: const CameraPosition( + target: LatLng(0, 0), + ), + ); + + await tester.pumpWidget(Directionality( + textDirection: TextDirection.ltr, + child: googleMap, + )); + + await tester.pump(); + + final GoogleMapController? controller = controllerNotifier.value; + + if (controller != null) { + expect(controller.streamSubscriptionsState, isNotEmpty); + + controller.streamSubscriptionsState.forEach((_, bool isCanceled) { + expect(isCanceled, isFalse); + }); + + await tester.pumpWidget(const Directionality( + textDirection: TextDirection.ltr, + child: SizedBox(), + )); + + await tester.binding.runAsync(() async { + await tester.pump(); + }); + + controller.streamSubscriptionsState.forEach((_, bool isCanceled) { + expect(isCanceled, isTrue); + }); + } else { + fail('GoogleMapController not created'); + } + }); + }); +} + +class FakePlatformViewsController { + Future fakePlatformViewsMethodHandler(MethodCall call) { + switch (call.method) { + case 'create': + return Future.value(1); + default: + return Future.value(); + } + } +}