ksidirop/MAN-318-fix-potential-memory-leaks-in-ios

Laerdal.McuMgr.Tests/FileDownloader/FileDownloaderTestbed.cs

@@ -66,6 +66,10 @@ public void FatalErrorOccurredAdvertisement(string resource, string errorMessage
 
             public void FileDownloadProgressPercentageAndDataThroughputChangedAdvertisement(int progressPercentage, float averageThroughput)
                 => _downloaderCallbacksProxy.FileDownloadProgressPercentageAndDataThroughputChangedAdvertisement(progressPercentage, averageThroughput); //raises the actual event
+
+            public void Dispose()
+            {
+            }
         }
     }
 }

Laerdal.McuMgr.Tests/FirmwareInstaller/FirmwareInstallerTestbed.cs

@@ -78,6 +78,10 @@ public void FirmwareUploadProgressPercentageAndDataThroughputChangedAdvertisemen
             public void CleanupResourcesOfLastInstallation()
             {
             }
+
+            public void Dispose()
+            {
+            }
         }
     }
 }

Laerdal.McuMgr/Droid/FileDownloader/FileDownloader.cs

@@ -57,7 +57,39 @@ internal AndroidFileDownloaderProxy(INativeFileDownloaderCallbacksProxy fileDown
                 _fileDownloaderCallbacksProxy = fileDownloaderCallbacksProxy ?? throw new ArgumentNullException(nameof(fileDownloaderCallbacksProxy));
             }
 
+            // public new void Dispose() { ... }    dont   there is no need to override the base implementation
+
+            private bool _alreadyDisposed;
+            protected override void Dispose(bool disposing)
+            {
+                if (_alreadyDisposed)
+                {
+                    base.Dispose(disposing); //vital
+                    return;
+                }
+
+                if (disposing)
+                {                   
+                    CleanupInfrastructure();
+                }
+
+                _alreadyDisposed = true;
+
+                base.Dispose(disposing);
+            }
 
+            private void CleanupInfrastructure()
+            {
+                try
+                {
+                    Disconnect();
+                }
+                catch
+                {
+                    // ignored
+                }
+            }
+
             #region commands 
 
             public new EFileDownloaderVerdict BeginDownload(string remoteFilePath)
@@ -66,7 +98,6 @@ internal AndroidFileDownloaderProxy(INativeFileDownloaderCallbacksProxy fileDown
             }
 
             #endregion commands
-
 
 
             #region android callbacks -> csharp event emitters

Laerdal.McuMgr/Droid/FileUploader/FileUploader.cs

@@ -59,6 +59,39 @@ internal AndroidFileUploaderProxy(INativeFileUploaderCallbacksProxy fileUploader
                 _fileUploaderCallbacksProxy = fileUploaderCallbacksProxy ?? throw new ArgumentNullException(nameof(fileUploaderCallbacksProxy));
             }
 
+            // public new void Dispose() { ... }    dont   there is no need to override the base implementation
+
+            private bool _alreadyDisposed;
+            protected override void Dispose(bool disposing)
+            {
+                if (_alreadyDisposed)
+                {
+                    base.Dispose(disposing); //vital
+                    return;
+                }
+
+                if (disposing)
+                {                   
+                    CleanupInfrastructure();
+                }
+
+                _alreadyDisposed = true;
+
+                base.Dispose(disposing);
+            }
+
+            private void CleanupInfrastructure()
+            {
+                try
+                {
+                    Disconnect();
+                }
+                catch
+                {
+                    // ignored
+                }
+            }
+
             public void CleanupResourcesOfLastUpload()
             {
                 //nothing to do in android

Laerdal.McuMgr/Droid/FirmwareInstaller/FirmwareInstaller.cs

@@ -59,6 +59,39 @@ internal AndroidFirmwareInstallerProxy(INativeFirmwareInstallerCallbacksProxy fi
                 _firmwareInstallerCallbacksProxy = firmwareInstallerCallbacksProxy ?? throw new ArgumentNullException(nameof(firmwareInstallerCallbacksProxy));
             }
 
+            // public new void Dispose() { ... }    dont   there is no need to override the base implementation
+
+            private bool _alreadyDisposed;
+            protected override void Dispose(bool disposing)
+            {
+                if (_alreadyDisposed)
+                {
+                    base.Dispose(disposing); //vital
+                    return;
+                }
+
+                if (disposing)
+                {                   
+                    CleanupInfrastructure();
+                }
+
+                _alreadyDisposed = true;
+
+                base.Dispose(disposing);
+            }
+
+            private void CleanupInfrastructure()
+            {
+                try
+                {
+                    Disconnect();
+                }
+                catch
+                {
+                    // ignored
+                }
+            }
+
             public void CleanupResourcesOfLastInstallation()
             {
                 //nothing to do here for android   only ios needs this

Laerdal.McuMgr/Shared/FileDownloader/Contracts/IFileDownloader.cs

@@ -1,11 +1,17 @@
 // ReSharper disable UnusedMember.Global
 // ReSharper disable EventNeverSubscribedTo.Global
 
+using System;
+
 namespace Laerdal.McuMgr.FileDownloader.Contracts
 {
     /// <summary>Downloads a file on a specific Nordic-chip-based BLE device</summary>
     /// <remarks>For the file-downloading process to even commence you need to be authenticated with the AED device that's being targeted.</remarks>
-    public interface IFileDownloader : IFileDownloaderCommandable, IFileDownloaderQueryable, IFileDownloaderEventSubscribable
+    public interface IFileDownloader :
+        IFileDownloaderCommandable,
+        IFileDownloaderQueryable,
+        IFileDownloaderEventSubscribable,
+        IDisposable
     {
     }
 }

Laerdal.McuMgr/Shared/FileDownloader/Contracts/Native/INativeFileDownloaderProxy.cs

@@ -1,6 +1,12 @@
-namespace Laerdal.McuMgr.FileDownloader.Contracts.Native
+using System;
+
+namespace Laerdal.McuMgr.FileDownloader.Contracts.Native
 {
-    internal interface INativeFileDownloaderProxy : INativeFileDownloaderQueryableProxy, INativeFileDownloaderCommandableProxy, INativeFileDownloaderCallbacksProxy
+    internal interface INativeFileDownloaderProxy :
+        INativeFileDownloaderQueryableProxy,
+        INativeFileDownloaderCommandableProxy,
+        INativeFileDownloaderCallbacksProxy,
+        IDisposable
     {
     }
 }

Laerdal.McuMgr/Shared/FileDownloader/FileDownloader.cs

@@ -28,6 +28,13 @@ internal FileDownloader(INativeFileDownloaderProxy nativeFileDownloaderProxy)
             _nativeFileDownloaderProxy = nativeFileDownloaderProxy ?? throw new ArgumentNullException(nameof(nativeFileDownloaderProxy));
             _nativeFileDownloaderProxy.FileDownloader = this; //vital
         }
+
+        public void Dispose()
+        {
+            _nativeFileDownloaderProxy?.Dispose();
+
+            GC.SuppressFinalize(this);
+        }
 
         public string LastFatalErrorMessage => _nativeFileDownloaderProxy?.LastFatalErrorMessage;
 

Laerdal.McuMgr/Shared/FileUploader/Contracts/IFileUploader.cs

@@ -7,7 +7,12 @@ namespace Laerdal.McuMgr.FileUploader.Contracts
 {
     /// <summary>Uploads a file on a specific Nordic-chip-based BLE device</summary>
     /// <remarks>For the file-uploading process to even commence you need to be authenticated with the AED device that's being targeted.</remarks>
-    public interface IFileUploader : IFileUploaderCommandable, IFileUploaderQueryable, IFileUploaderEventSubscribable, IFileUploaderCleanupable, IDisposable
+    public interface IFileUploader :
+        IFileUploaderCommandable,
+        IFileUploaderQueryable,
+        IFileUploaderEventSubscribable,
+        IFileUploaderCleanupable,
+        IDisposable
     {
     }
 }

Laerdal.McuMgr/Shared/FileUploader/FileUploader.cs

@@ -31,6 +31,13 @@ internal FileUploader(INativeFileUploaderProxy nativeFileUploaderProxy)
             _nativeFileUploaderProxy.FileUploader = this; //vital
         }
 
+        public void Dispose()
+        {
+            _nativeFileUploaderProxy?.Dispose();
+
+            GC.SuppressFinalize(this);
+        }
+
         public bool TrySetContext(object context) => _nativeFileUploaderProxy?.TrySetContext(context) ?? false;
         public bool TrySetBluetoothDevice(object bluetoothDevice) => _nativeFileUploaderProxy?.TrySetContext(bluetoothDevice) ?? false;
         public bool TryInvalidateCachedTransport() => _nativeFileUploaderProxy?.TryInvalidateCachedTransport() ?? false;
@@ -438,7 +445,5 @@ public void FileUploadProgressPercentageAndDataThroughputChangedAdvertisement(in
                     progressPercentage: progressPercentage
                 ));
         }
-
-        public void Dispose() => _nativeFileUploaderProxy?.Dispose();
     }
 }

Laerdal.McuMgr/Shared/FirmwareInstaller/Contracts/IFirmwareInstaller.cs

@@ -1,10 +1,16 @@
 // ReSharper disable UnusedMember.Global
 // ReSharper disable EventNeverSubscribedTo.Global
 
+using System;
+
 namespace Laerdal.McuMgr.FirmwareInstaller.Contracts
 {
     /// <summary>Upgrades the firmware on a specific Nordic-chip-based BLE device</summary>
-    public interface IFirmwareInstaller : IFirmwareInstallerQueryable, IFirmwareInstallerEventSubscribable, IFirmwareInstallerCommandable
+    public interface IFirmwareInstaller :
+        IFirmwareInstallerQueryable,
+        IFirmwareInstallerEventSubscribable,
+        IFirmwareInstallerCommandable,
+        IDisposable
     {
     }
 }

Laerdal.McuMgr/Shared/FirmwareInstaller/Contracts/Native/INativeFirmwareInstallerProxy.cs

@@ -1,10 +1,13 @@
+using System;
+
 namespace Laerdal.McuMgr.FirmwareInstaller.Contracts.Native
 {
     internal interface INativeFirmwareInstallerProxy :
         INativeFirmwareInstallerCommandableProxy,
         INativeFirmwareInstallerQueryableProxy,
         INativeFirmwareInstallerCleanupableProxy,
-        INativeFirmwareInstallerCallbacksProxy
+        INativeFirmwareInstallerCallbacksProxy,
+        IDisposable
     {
         string Nickname { get; set; }
     }

Laerdal.McuMgr/Shared/FirmwareInstaller/FirmwareInstaller.cs

@@ -29,6 +29,13 @@ internal FirmwareInstaller(INativeFirmwareInstallerProxy nativeFirmwareInstaller
             _nativeFirmwareInstallerProxy = nativeFirmwareInstallerProxy ?? throw new ArgumentNullException(nameof(nativeFirmwareInstallerProxy));
             _nativeFirmwareInstallerProxy.FirmwareInstaller = this; //vital
         }
+
+        public void Dispose()
+        {
+            _nativeFirmwareInstallerProxy?.Dispose();
+
+            GC.SuppressFinalize(this);
+        }
 
         public EFirmwareInstallationVerdict BeginInstallation(
             byte[] data,

Laerdal.McuMgr/iOS/FileDownloader/FileDownloader.cs

@@ -33,7 +33,7 @@ static private INativeFileDownloaderProxy ValidateArgumentsAndConstructProxy(CBP
         //ReSharper disable once InconsistentNaming
         private sealed class IOSNativeFileDownloaderProxy : IOSListenerForFileDownloader, INativeFileDownloaderProxy
         {
-            private readonly IOSFileDownloader _nativeFileDownloader;
+            private IOSFileDownloader _nativeFileDownloader;
             private readonly INativeFileDownloaderCallbacksProxy _nativeFileDownloaderCallbacksProxy;
 
             internal IOSNativeFileDownloaderProxy(CBPeripheral bluetoothDevice, INativeFileDownloaderCallbacksProxy nativeFileDownloaderCallbacksProxy)
@@ -45,6 +45,41 @@ internal IOSNativeFileDownloaderProxy(CBPeripheral bluetoothDevice, INativeFileD
                 _nativeFileDownloaderCallbacksProxy = nativeFileDownloaderCallbacksProxy; //composition-over-inheritance
             }
 
+            // public new void Dispose() { ... }    dont   there is no need to override the base implementation
+
+            private bool _alreadyDisposed;
+            protected override void Dispose(bool disposing)
+            {
+                if (_alreadyDisposed)
+                {
+                    base.Dispose(disposing); //vital
+                    return;
+                }
+
+                if (disposing)
+                {
+                    CleanupInfrastructure();
+                }
+
+                _alreadyDisposed = true;
+
+                base.Dispose(disposing);
+            }
+
+            private void CleanupInfrastructure()
+            {
+                try
+                {
+                    Disconnect();
+                }
+                catch
+                {
+                    // ignored
+                }
+
+                _nativeFileDownloader?.Dispose();
+                _nativeFileDownloader = null;
+            }
 
             #region commands
 

Laerdal.McuMgr/iOS/FileUploader/FileUploader.cs

@@ -71,13 +71,22 @@ protected override void Dispose(bool disposing)
                 }
 
                 _alreadyDisposed = true;
-                
+
                 base.Dispose(disposing);
             }
 
             private void CleanupInfrastructure()
             {
-                _nativeFileUploader?.Dispose();
+                try
+                {
+                    Disconnect();
+                }
+                catch
+                {
+                    // ignored
+                }
+
+                _nativeFileUploader?.Dispose(); //order
                 _nativeFileUploader = null;
             }
 

Laerdal.McuMgr/iOS/FirmwareInstaller/FirmwareInstaller.cs

@@ -72,6 +72,15 @@ protected override void Dispose(bool disposing)
 
             private void CleanupInfrastructure()
             {
+                try
+                {
+                    Disconnect();
+                }
+                catch
+                {
+                    // ignored
+                }
+
                 _nativeFirmwareInstaller?.Dispose();
                 _nativeFirmwareInstaller = null;
             }