fix: Add mounted checks and fix memory leaks across UI components

Address potential memory leaks and state-after-dispose issues: - AppProvider: Add dispose method to clean up listeners and callbacks - MapTab: Store and restore original location callback instead of nullifying - MessagesTab: Use Timer instead of Future.delayed for highlight cleanup - ConnectionDialog: Use named listener method for proper tab controller cleanup - RoomLoginSheet: Add _isDisposed flag to handle async callback race conditions - SettingsScreen: Clear location service callbacks in dispose - Various screens: Add mounted checks before setState after async operations (contacts_tab, device_config, home_screen, map_management, packet_log, sar_template_management, sar_update_sheet, permission_request_dialog)
2026-08-13 01:10:28 +00:00 · 2025-12-04 19:34:48 +01:00
parent 3e2e4aa4d9
commit 96824e455e
15 changed files with 119 additions and 57 deletions
--- a/lib/providers/app_provider.dart
+++ b/lib/providers/app_provider.dart
@@ -25,6 +25,8 @@ class AppProvider with ChangeNotifier {
  bool _isInitialized = false;
  bool get isInitialized => _isInitialized;

+  bool _isDisposed = false;
+
  bool _isSimpleMode = true;
  bool get isSimpleMode => _isSimpleMode;

@@ -683,4 +685,17 @@ class AppProvider with ChangeNotifier {
      'sarMarkers': messagesProvider.sarMarkerStats,
    };
  }
+
+  @override
+  void dispose() {
+    _isDisposed = true;
+    // Remove connection state listener
+    connectionProvider.removeListener(_handleConnectionStateChange);
+    // Clear location service callbacks
+    locationTrackingService.onPositionUpdate = null;
+    locationTrackingService.onBroadcastSent = null;
+    locationTrackingService.onError = null;
+    locationTrackingService.onTrackingStateChanged = null;
+    super.dispose();
+  }
 }
--- a/lib/screens/contacts_tab.dart
+++ b/lib/screens/contacts_tab.dart
@@ -51,9 +51,11 @@ class _ContactsTabState extends State<ContactsTab> {
  }

  Future<void> _handleRefresh() async {
+    if (!mounted) return;
    final appProvider = context.read<AppProvider>();
    await appProvider.refresh();
    // Also refresh location
+    if (!mounted) return;
    await _getCurrentLocation();
  }

--- a/lib/screens/device_config_screen.dart
+++ b/lib/screens/device_config_screen.dart
@@ -347,6 +347,8 @@ class _DeviceConfigScreenState extends State<DeviceConfigScreen> {
        ),
      );

+      if (!mounted) return;
+
      setState(() {
        _latController.text = position.latitude.toStringAsFixed(6);
        _lonController.text = position.longitude.toStringAsFixed(6);
--- a/lib/screens/home_screen.dart
+++ b/lib/screens/home_screen.dart
@@ -279,6 +279,7 @@ class _HomeScreenState extends State<HomeScreen>
    final appProvider = context.watch<AppProvider>();
    if (_isMapEnabled != appProvider.isMapEnabled) {
      WidgetsBinding.instance.addPostFrameCallback((_) {
+        if (!mounted) return;
        _updateTabController(appProvider.isMapEnabled);
      });
    }
--- a/lib/screens/map_management_screen.dart
+++ b/lib/screens/map_management_screen.dart
@@ -409,6 +409,7 @@ class _MapManagementScreenState extends State<MapManagementScreen> {
        );
      }
    } catch (e) {
+      if (!mounted) return;
      setState(() => _isLoading = false);
      _showError(AppLocalizations.of(context)!.clearCacheFailed(e.toString()));
    }
--- a/lib/screens/map_tab.dart
+++ b/lib/screens/map_tab.dart
@@ -78,6 +78,9 @@ class _MapTabState extends State<MapTab> with AutomaticKeepAliveClientMixin {
  // MBTiles layers
  List<MapLayer> _mbtilesLayers = [];

+  // Store original location callback to restore in dispose
+  void Function(Position)? _originalLocationCallback;
+
  // WMS layers (Slovenian)
  late final MapLayer _slovenianAerialLayer;
  late final MapLayer _dtk25Layer;
@@ -128,6 +131,7 @@ class _MapTabState extends State<MapTab> with AutomaticKeepAliveClientMixin {

    // Listen to map provider for navigation requests
    WidgetsBinding.instance.addPostFrameCallback((_) {
+      if (!mounted) return; // Check if widget is still mounted
      final mapProvider = context.read<MapProvider>();
      mapProvider.addListener(_handleMapNavigation);
      // Load WMS overlay state
@@ -146,13 +150,13 @@ class _MapTabState extends State<MapTab> with AutomaticKeepAliveClientMixin {
  /// Note: LocationTrackingService is initialized and started by AppProvider
  /// This method only adds map-specific callbacks for rotation and UI updates
  void _setupLocationCallbacks() {
-    // Store the original callback from AppProvider
-    final originalCallback = _locationService.onPositionUpdate;
+    // Store the original callback from AppProvider to restore in dispose
+    _originalLocationCallback = _locationService.onPositionUpdate;

    // Add map-specific callback that chains with the original
    _locationService.onPositionUpdate = (position) {
      // Call original callback first (AppProvider's logging)
-      originalCallback?.call(position);
+      _originalLocationCallback?.call(position);

      // Then handle map-specific logic - early exit if not mounted or disposing
      if (!mounted || _isDisposing) {
@@ -437,8 +441,8 @@ class _MapTabState extends State<MapTab> with AutomaticKeepAliveClientMixin {
    mapProvider.removeListener(_handleMapNavigation);

    // DO NOT stop location tracking - it's managed by AppProvider
-    // Just clear the map-specific callback
-    _locationService.onPositionUpdate = null;
+    // Restore the original callback instead of setting to null
+    _locationService.onPositionUpdate = _originalLocationCallback;
    _mapController.dispose();
    _tileCache.dispose();
    super.dispose();
--- a/lib/screens/messages_tab.dart
+++ b/lib/screens/messages_tab.dart
@@ -1,3 +1,4 @@
+import 'dart:async';
 import 'package:flutter/material.dart';
 import 'package:flutter/services.dart';
 import 'package:provider/provider.dart';
@@ -33,6 +34,7 @@ class _MessagesTabState extends State<MessagesTab> {
  int _characterCount = 0;
  static const int _maxCharacters = 160;
  String? _highlightedMessageId;
+  Timer? _highlightTimer; // Timer for clearing message highlight

  // Message destination state
  String _destinationType =
@@ -73,6 +75,7 @@ class _MessagesTabState extends State<MessagesTab> {

  @override
  void dispose() {
+    _highlightTimer?.cancel();
    _textController.dispose();
    _focusNode.dispose();
    _scrollController.dispose();
@@ -112,8 +115,9 @@ class _MessagesTabState extends State<MessagesTab> {
        _highlightedMessageId = messageId;
      });

-      // Clear highlight after 2 seconds
-      Future.delayed(const Duration(seconds: 2), () {
+      // Clear highlight after 2 seconds using a properly managed Timer
+      _highlightTimer?.cancel();
+      _highlightTimer = Timer(const Duration(seconds: 2), () {
        if (mounted) {
          setState(() {
            _highlightedMessageId = null;
--- a/lib/screens/packet_log_screen.dart
+++ b/lib/screens/packet_log_screen.dart
@@ -73,10 +73,12 @@ class _PacketLogScreenState extends State<PacketLogScreen> {

      // Save to temporary file
      final tempDir = await getTemporaryDirectory();
+      if (!context.mounted) return;
      final file = File('${tempDir.path}/ble_packets_${DateTime.now().millisecondsSinceEpoch}.csv');
      await file.writeAsString(buffer.toString());

      // Share the file
+      if (!context.mounted) return;
      await Share.shareXFiles(
        [XFile(file.path)],
        subject: 'MeshCore BLE Packet Logs',
@@ -118,10 +120,12 @@ class _PacketLogScreenState extends State<PacketLogScreen> {

      // Save to temporary file
      final tempDir = await getTemporaryDirectory();
+      if (!context.mounted) return;
      final file = File('${tempDir.path}/ble_packets_${DateTime.now().millisecondsSinceEpoch}.txt');
      await file.writeAsString(buffer.toString());

      // Share the file
+      if (!context.mounted) return;
      await Share.shareXFiles(
        [XFile(file.path)],
        subject: 'MeshCore BLE Packet Logs',
@@ -147,27 +151,31 @@ class _PacketLogScreenState extends State<PacketLogScreen> {
  }

  void _clearLogs(BuildContext context) {
+    final parentContext = context; // Store parent context for setState
    showDialog(
      context: context,
-      builder: (context) => AlertDialog(
-        title: Text(AppLocalizations.of(context)!.clearAllData),
+      builder: (dialogContext) => AlertDialog(
+        title: Text(AppLocalizations.of(dialogContext)!.clearAllData),
        content: const Text('Are you sure you want to clear all packet logs? This cannot be undone.'),
        actions: [
          TextButton(
-            onPressed: () => Navigator.pop(context),
-            child: Text(AppLocalizations.of(context)!.cancel),
+            onPressed: () => Navigator.pop(dialogContext),
+            child: Text(AppLocalizations.of(dialogContext)!.cancel),
          ),
          TextButton(
            onPressed: () {
              widget.bleService.clearPacketLogs();
-              Navigator.pop(context);
+              Navigator.pop(dialogContext);
+              if (!mounted) return;
              setState(() {});
-              ScaffoldMessenger.of(context).showSnackBar(
-                const SnackBar(content: Text('Packet logs cleared')),
-              );
+              if (parentContext.mounted) {
+                ScaffoldMessenger.of(parentContext).showSnackBar(
+                  const SnackBar(content: Text('Packet logs cleared')),
+                );
+              }
            },
            style: TextButton.styleFrom(foregroundColor: Colors.red),
-            child: Text(AppLocalizations.of(context)!.clear),
+            child: Text(AppLocalizations.of(dialogContext)!.clear),
          ),
        ],
      ),
@@ -381,6 +389,7 @@ class _PacketLogScreenState extends State<PacketLogScreen> {
                      // Auto-scroll to bottom
                      if (_autoScroll && index == logs.length - 1) {
                        WidgetsBinding.instance.addPostFrameCallback((_) {
+                          if (!mounted) return;
                          if (_scrollController.hasClients) {
                            _scrollController.animateTo(
                              _scrollController.position.maxScrollExtent,
--- a/lib/screens/sar_template_management_screen.dart
+++ b/lib/screens/sar_template_management_screen.dart
@@ -24,6 +24,7 @@ class _SarTemplateManagementScreenState extends State<SarTemplateManagementScree

  Future<void> _initializeService() async {
    if (!_templateService.isInitialized) {
+      if (!mounted) return;
      setState(() => _isLoading = true);
      await _templateService.initialize();
      if (mounted) {
@@ -111,6 +112,7 @@ class _SarTemplateManagementScreenState extends State<SarTemplateManagementScree
  }

  Future<void> _importFromClipboard() async {
+    if (!mounted) return;
    setState(() => _isLoading = true);

    try {
--- a/lib/screens/settings_screen.dart
+++ b/lib/screens/settings_screen.dart
@@ -57,6 +57,15 @@ class _SettingsScreenState extends State<SettingsScreen> {
    _loadRxTxPreference();
  }

+  @override
+  void dispose() {
+    // Clear location service callbacks to prevent memory leaks
+    _locationService.onError = null;
+    _locationService.onBroadcastSent = null;
+    _locationService.onTrackingStateChanged = null;
+    super.dispose();
+  }
+
  Future<void> _loadPackageInfo() async {
    final info = await PackageInfo.fromPlatform();
    if (mounted) {
--- a/lib/widgets/connection_dialog.dart
+++ b/lib/widgets/connection_dialog.dart
@@ -22,6 +22,26 @@ class _ConnectionDialogState extends State<ConnectionDialog>
  int _totalToScan = 0;
  String? _connectingToServerUrl; // Track which server is being connected to

+  // Named listener method for proper cleanup
+  void _onTabChanged() {
+    if (_tabController.index == 1) {
+      // Switched to network tab
+      if (_networkScanner.hasCachedResults && _discoveredServers.isEmpty) {
+        // Load cached results
+        setState(() {
+          _discoveredServers.addAll(_networkScanner.cachedServers);
+        });
+        debugPrint(
+          '📦 [NetworkScanner] Loaded ${_discoveredServers.length} servers from cache',
+        );
+      } else if (!_networkScanner.isScanning &&
+          !_networkScanner.hasCachedResults) {
+        // No cache, start initial scan
+        _startNetworkScan();
+      }
+    }
+  }
+
  @override
  void initState() {
    super.initState();
@@ -55,25 +75,8 @@ class _ConnectionDialogState extends State<ConnectionDialog>
      }
    };

-    // Listen to tab changes
-    _tabController.addListener(() {
-      if (_tabController.index == 1) {
-        // Switched to network tab
-        if (_networkScanner.hasCachedResults && _discoveredServers.isEmpty) {
-          // Load cached results
-          setState(() {
-            _discoveredServers.addAll(_networkScanner.cachedServers);
-          });
-          debugPrint(
-            '📦 [NetworkScanner] Loaded ${_discoveredServers.length} servers from cache',
-          );
-        } else if (!_networkScanner.isScanning &&
-            !_networkScanner.hasCachedResults) {
-          // No cache, start initial scan
-          _startNetworkScan();
-        }
-      }
-    });
+    // Listen to tab changes using named method for proper cleanup
+    _tabController.addListener(_onTabChanged);
  }

  @override
@@ -84,6 +87,8 @@ class _ConnectionDialogState extends State<ConnectionDialog>
    );
    connectionProvider.stopScan();
    _networkScanner.stopScan();
+    // Remove listener before disposing to prevent memory leaks
+    _tabController.removeListener(_onTabChanged);
    _tabController.dispose();
    super.dispose();
  }
--- a/lib/widgets/contacts/direct_message_sheet.dart
+++ b/lib/widgets/contacts/direct_message_sheet.dart
@@ -41,6 +41,7 @@ class _DirectMessageSheetState extends State<DirectMessageSheet> {
  }

  void _updateCharacterCount() {
+    if (!mounted) return;
    setState(() {
      _characterCount = _textController.text.length;
    });
--- a/lib/widgets/contacts/room_login_sheet.dart
+++ b/lib/widgets/contacts/room_login_sheet.dart
@@ -21,6 +21,7 @@ class _RoomLoginSheetState extends State<RoomLoginSheet> {
  final FocusNode _focusNode = FocusNode();
  bool _isLoggingIn = false;
  bool _obscurePassword = true;
+  bool _isDisposed = false; // Track disposal state for async callbacks

  @override
  void initState() {
@@ -30,6 +31,7 @@ class _RoomLoginSheetState extends State<RoomLoginSheet> {

  @override
  void dispose() {
+    _isDisposed = true;
    _passwordController.dispose();
    _focusNode.dispose();
    super.dispose();
@@ -245,15 +247,15 @@ class _RoomLoginSheetState extends State<RoomLoginSheet> {
        '   Messages will be fetched when onMessageWaiting callback is triggered',
      );

-      if (mounted) {
-        ScaffoldMessenger.of(context).showSnackBar(
-          SnackBar(
-            content: Text(AppLocalizations.of(context)!.loggedInSuccessfully),
-            backgroundColor: Theme.of(context).colorScheme.primary,
-            duration: const Duration(seconds: 3),
-          ),
-        );
-      }
+      // Check both _isDisposed flag and mounted to handle race conditions
+      if (_isDisposed || !mounted) return;
+      ScaffoldMessenger.of(context).showSnackBar(
+        SnackBar(
+          content: Text(AppLocalizations.of(context)!.loggedInSuccessfully),
+          backgroundColor: Theme.of(context).colorScheme.primary,
+          duration: const Duration(seconds: 3),
+        ),
+      );
    };

    connectionProvider.onLoginFail = (publicKeyPrefix) {
@@ -263,15 +265,15 @@ class _RoomLoginSheetState extends State<RoomLoginSheet> {

      debugPrint('❌ [RoomLogin] Login failed - incorrect password');

-      if (mounted) {
-        ScaffoldMessenger.of(context).showSnackBar(
-          SnackBar(
-            content: Text(AppLocalizations.of(context)!.loginFailed),
-            backgroundColor: Theme.of(context).colorScheme.error,
-            duration: const Duration(seconds: 3),
-          ),
-        );
-      }
+      // Check both _isDisposed flag and mounted to handle race conditions
+      if (_isDisposed || !mounted) return;
+      ScaffoldMessenger.of(context).showSnackBar(
+        SnackBar(
+          content: Text(AppLocalizations.of(context)!.loginFailed),
+          backgroundColor: Theme.of(context).colorScheme.error,
+          duration: const Duration(seconds: 3),
+        ),
+      );
    };

    try {
--- a/lib/widgets/messages/sar_update_sheet.dart
+++ b/lib/widgets/messages/sar_update_sheet.dart
@@ -231,6 +231,7 @@ class _SarUpdateSheetState extends State<SarUpdateSheet> {
    try {
      // Check if location services are enabled
      bool serviceEnabled = await Geolocator.isLocationServiceEnabled();
+      if (!mounted) return;
      if (!serviceEnabled) {
        setState(() {
          _locationError = 'Location services are disabled';
@@ -241,8 +242,10 @@ class _SarUpdateSheetState extends State<SarUpdateSheet> {

      // Check permissions
      LocationPermission permission = await Geolocator.checkPermission();
+      if (!mounted) return;
      if (permission == LocationPermission.denied) {
        permission = await Geolocator.requestPermission();
+        if (!mounted) return;
        if (permission == LocationPermission.denied) {
          setState(() {
            _locationError = 'Location permission denied';
--- a/lib/widgets/permission_request_dialog.dart
+++ b/lib/widgets/permission_request_dialog.dart
@@ -38,6 +38,7 @@ class _PermissionRequestDialogState extends State<PermissionRequestDialog> {
    try {
      // Check if location service is enabled
      final serviceEnabled = await Geolocator.isLocationServiceEnabled();
+      if (!mounted) return;
      if (!serviceEnabled) {
        setState(() {
          _errorMessage = 'Location services are disabled. Please enable location services in your device settings.';
@@ -48,10 +49,12 @@ class _PermissionRequestDialogState extends State<PermissionRequestDialog> {

      // Check current permission
      LocationPermission permission = await Geolocator.checkPermission();
+      if (!mounted) return;

      if (permission == LocationPermission.denied) {
        // Request permission
        permission = await Geolocator.requestPermission();
+        if (!mounted) return;
      }

      if (permission == LocationPermission.denied) {
@@ -78,11 +81,10 @@ class _PermissionRequestDialogState extends State<PermissionRequestDialog> {
      });

      // Close dialog and notify parent
-      if (mounted) {
-        Navigator.of(context).pop();
-        widget.onPermissionsGranted();
-      }
+      Navigator.of(context).pop();
+      widget.onPermissionsGranted();
    } catch (e) {
+      if (!mounted) return;
      setState(() {
        _errorMessage = 'Error requesting permissions: $e';
        _isRequesting = false;