Make GetConnectionForRoute more thread safe and awaitable.

WildernessLabs · Jul 30, 2024 · 5f1a88a · 5f1a88a
1 parent a295604
commit 5f1a88a
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 27 deletions.
diff --git a/Source/v2/Meadow.Cli/Commands/Current/BaseDeviceCommand.cs b/Source/v2/Meadow.Cli/Commands/Current/BaseDeviceCommand.cs
@@ -29,11 +29,11 @@ private async Task<IMeadowConnection> GetConnection(string? route, bool forceRec
 
         if (route != null)
         {
-            connection = ConnectionManager.GetConnectionForRoute(route, forceReconnect);
+            connection = await MeadowConnectionManager.GetConnectionForRoute(route, forceReconnect);
         }
         else
         {
-            connection = ConnectionManager.GetCurrentConnection(forceReconnect);
+            connection = await ConnectionManager.GetCurrentConnection(forceReconnect);
         }
 
         if (connection != null)

diff --git a/Source/v2/Meadow.Tooling.Core/Connection/MeadowConnectionManager.cs b/Source/v2/Meadow.Tooling.Core/Connection/MeadowConnectionManager.cs
@@ -19,14 +19,17 @@ public class MeadowConnectionManager
     private static readonly object _lockObject = new();
 
     private readonly ISettingsManager _settingsManager;
-    private IMeadowConnection? _currentConnection;
+    private static IMeadowConnection? _currentConnection;
+
+    private static readonly int RETRY_COUNT = 10;
+    private static readonly int RETRY_DELAY = 500;
 
     public MeadowConnectionManager(ISettingsManager settingsManager)
     {
         _settingsManager = settingsManager;
     }
 
-    public IMeadowConnection? GetCurrentConnection(bool forceReconnect = false)
+    public async Task<IMeadowConnection?> GetCurrentConnection(bool forceReconnect = false)
     {
         var route = _settingsManager.GetSetting(SettingsManager.PublicSettings.Route);
 
@@ -35,22 +38,36 @@ public MeadowConnectionManager(ISettingsManager settingsManager)
             throw new Exception($"No 'route' configuration set.{Environment.NewLine}Use the `meadow config route` command. For example:{Environment.NewLine}  > meadow config route COM5");
         }
 
-        return GetConnectionForRoute(route, forceReconnect);
+        return await GetConnectionForRoute(route, forceReconnect);
     }
 
-    public IMeadowConnection? GetConnectionForRoute(string route, bool forceReconnect = false)
+    public static async Task<IMeadowConnection?> GetConnectionForRoute(string route, bool forceReconnect = false)
     {
         // TODO: support connection changing (CLI does this rarely as it creates a new connection with each command)
-        if (_currentConnection != null && forceReconnect == false)
+        lock (_lockObject)
         {
-            return _currentConnection;
+            if (_currentConnection != null
+            && _currentConnection.Name == route
+            && forceReconnect == false)
+            {
+                return _currentConnection;
+            }
+            else if (_currentConnection != null)
+            {
+                _currentConnection.Detach();
+                _currentConnection.Dispose();
+                _currentConnection = null;
+            }
         }
-        _currentConnection?.Detach();
-        _currentConnection?.Dispose();
 
         if (route == "local")
         {
-            return new LocalConnection();
+            var newConnection = new LocalConnection();
+            lock (_lockObject)
+            {
+                _currentConnection = newConnection;
+            }
+            return _currentConnection;
         }
 
         // try to determine what the route is
@@ -74,30 +91,40 @@ public MeadowConnectionManager(ISettingsManager settingsManager)
 
         if (uri != null)
         {
-            _currentConnection = new TcpConnection(uri);
+            var newConnection = new TcpConnection(uri);
+            lock (_lockObject)
+            {
+                _currentConnection = newConnection;
+            }
+            return _currentConnection;
         }
         else
         {
-            var retryCount = 0;
-
-        get_serial_connection:
-            try
-            {
-                _currentConnection = new SerialConnection(route);
-            }
-            catch
+            for (int retryCount = 0; retryCount <= RETRY_COUNT; retryCount++)
             {
-                retryCount++;
-                if (retryCount > 10)
+                try
+                {
+                    var newConnection = new SerialConnection(route);
+                    lock (_lockObject)
+                    {
+                        _currentConnection = newConnection;
+                    }
+                    return _currentConnection;
+                }
+                catch
                 {
-                    throw new Exception($"Cannot find port {route}");
+                    if (retryCount == RETRY_COUNT)
+                    {
+                        throw new Exception($"Cannot create SerialConnection on route: {route}");
+                    }
+
+                    await Task.Delay(RETRY_DELAY);
                 }
-                Thread.Sleep(500);
-                goto get_serial_connection;
             }
-        }
 
-        return _currentConnection;
+            // This line should never be reached because the loop will either return or throw
+            throw new Exception("Unexpected error in GetCurrentConnection");
+        }
     }
 
     public static async Task<IList<string>> GetSerialPorts(string? serialNumber = null)