Extract static factory methods for scripting shells into a factory se…

…rvice.

Agent/Interfaces/IScriptingShell.cs

@@ -0,0 +1,11 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Remotely.Agent.Interfaces;
+public interface IScriptingShell
+{
+    bool IsDisposed { get; }
+}

Agent/Program.cs

@@ -23,9 +23,6 @@ namespace Remotely.Agent;
 
 public class Program
 {
-    [Obsolete("Remove this when all services are in DI behind interfaces.")]
-    public static IServiceProvider? Services { get; set; }
-
     public static async Task Main(string[] args)
     {
         try
@@ -39,8 +36,6 @@ public static async Task Main(string[] args)
 
             await host.StartAsync();
 
-            Services = host.Services;
-
             await Init(host.Services);
 
             await host.WaitForShutdownAsync();
@@ -105,6 +100,7 @@ private static void RegisterServices(IServiceCollection services)
         services.AddScoped<IProcessInvoker, ProcessInvoker>();
         services.AddScoped<IUpdateDownloader, UpdateDownloader>();
         services.AddSingleton<IFileLogsManager, FileLogsManager>();
+        services.AddSingleton<IScriptingShellFactory, ScriptingShellFactory>();
 
         if (OperatingSystem.IsWindows())
         {

Agent/Services/AgentHubConnection.cs

@@ -42,6 +42,7 @@ public class AgentHubConnection : IAgentHubConnection, IDisposable
     private readonly ILogger<AgentHubConnection> _logger;
     private readonly IFileLogsManager _fileLogsManager;
     private readonly IHostApplicationLifetime _appLifetime;
+    private readonly IScriptingShellFactory _scriptingShellFactory;
     private readonly IScriptExecutor _scriptExecutor;
     private readonly IUninstaller _uninstaller;
     private readonly IUpdater _updater;
@@ -63,6 +64,7 @@ public AgentHubConnection(
         IWakeOnLanService wakeOnLanService,
         IFileLogsManager fileLogsManager,
         IHostApplicationLifetime appLifetime,
+        IScriptingShellFactory scriptingShellFactory,
         ILogger<AgentHubConnection> logger)
     {
         _configService = configService;
@@ -77,6 +79,7 @@ public AgentHubConnection(
         _logger = logger;
         _fileLogsManager = fileLogsManager;
         _appLifetime = appLifetime;
+        _scriptingShellFactory = scriptingShellFactory;
     }
 
     public bool IsConnected => _hubConnection?.State == HubConnectionState.Connected;
@@ -395,7 +398,7 @@ private void RegisterMessageHandlers()
         {
             try
             {
-                var session = PsCoreShell.GetCurrent(senderConnectionId);
+                var session = _scriptingShellFactory.GetOrCreatePsCoreShell(senderConnectionId);
                 var completion = session.GetCompletions(inputText, currentIndex, forward);
                 var completionModel = completion.ToPwshCompletion();
                 await _hubConnection.InvokeAsync("ReturnPowerShellCompletions", completionModel, intent, senderConnectionId).ConfigureAwait(false);

Agent/Services/ExternalScriptingShell.cs

@@ -1,9 +1,7 @@
-using Immense.RemoteControl.Shared.Extensions;
-using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging;
+using Remotely.Agent.Interfaces;
 using Remotely.Shared.Dtos;
 using Remotely.Shared.Enums;
-using Remotely.Shared.Models;
 using System;
 using System.Collections.Concurrent;
 using System.Diagnostics;
@@ -14,7 +12,7 @@
 
 namespace Remotely.Agent.Services;
 
-public interface IExternalScriptingShell
+public interface IExternalScriptingShell : IDisposable, IScriptingShell
 {
     Process? ShellProcess { get; }
     Task Init(ScriptingShell shell, string shellProcessName, string lineEnding, string connectionId);
@@ -28,6 +26,7 @@ public class ExternalScriptingShell : IExternalScriptingShell
     private readonly ILogger<ExternalScriptingShell> _logger;
     private readonly ManualResetEvent _outputDone = new(false);
     private readonly SemaphoreSlim _writeLock = new(1, 1);
+    private bool _disposedValue;
     private string _errorOut = string.Empty;
     private string _lastInputID = string.Empty;
     private string _lineEnding = Environment.NewLine;
@@ -39,7 +38,6 @@ public class ExternalScriptingShell : IExternalScriptingShell
     private string? _senderConnectionId;
     private ScriptingShell _shell;
     private string _standardOut = string.Empty;
-
     public ExternalScriptingShell(
         IConfigService configService,
         ILogger<ExternalScriptingShell> logger)
@@ -48,38 +46,14 @@ public ExternalScriptingShell(
         _logger = logger;
     }
 
-    public Process? ShellProcess { get; private set; }
+    public bool IsDisposed => _disposedValue;
 
+    public Process? ShellProcess { get; private set; }
 
-    // TODO: Turn into cache and factory.
-    public static async Task<IExternalScriptingShell> GetCurrent(ScriptingShell shell, string senderConnectionId)
+    public void Dispose()
     {
-        if (_sessions.TryGetValue($"{shell}-{senderConnectionId}", out var session) &&
-            session.ShellProcess?.HasExited != true)
-        {
-            return session;
-        }
-        else
-        {
-            session = Program.Services.GetRequiredService<IExternalScriptingShell>();
-
-            switch (shell)
-            {
-                case ScriptingShell.WinPS:
-                    await session.Init(shell, "powershell.exe", "\r\n", senderConnectionId);
-                    break;
-                case ScriptingShell.Bash:
-                    await session.Init(shell, "bash", "\n", senderConnectionId);
-                    break;
-                case ScriptingShell.CMD:
-                    await session.Init(shell, "cmd.exe", "\r\n", senderConnectionId);
-                    break;
-                default:
-                    throw new ArgumentException($"Unknown external scripting shell type: {shell}");
-            }
-            _sessions.AddOrUpdate($"{shell}-{senderConnectionId}", session, (id, b) => session);
-            return session;
-        }
+        Dispose(disposing: true);
+        GC.SuppressFinalize(this);
     }
 
     public async Task Init(ScriptingShell shell, string shellProcessName, string lineEnding, string connectionId)
@@ -187,6 +161,19 @@ public async Task<ScriptResultDto> WriteInput(string input, TimeSpan timeout)
         return GeneratePartialResult(input, sw.Elapsed);
     }
 
+    protected virtual void Dispose(bool disposing)
+    {
+        if (!_disposedValue)
+        {
+            if (disposing)
+            {
+                ShellProcess?.Dispose();
+            }
+
+            _disposedValue = true;
+        }
+    }
+
     private ScriptResultDto GenerateCompletedResult(string input, TimeSpan runtime)
     {
         return new ScriptResultDto()

Agent/Services/PsCoreShell.cs

@@ -1,5 +1,6 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Remotely.Agent.Interfaces;
 using Remotely.Shared.Dtos;
 using Remotely.Shared.Models;
 using System;
@@ -11,7 +12,7 @@
 
 namespace Remotely.Agent.Services;
 
-public interface IPsCoreShell
+public interface IPsCoreShell : IDisposable, IScriptingShell
 {
     string? SenderConnectionId { get; set; }
 
@@ -21,11 +22,11 @@ public interface IPsCoreShell
 
 public class PsCoreShell : IPsCoreShell
 {
-    private static readonly ConcurrentDictionary<string, IPsCoreShell> _sessions = new();
     private readonly IConfigService _configService;
     private readonly ConnectionInfo _connectionInfo;
     private readonly ILogger<PsCoreShell> _logger;
     private readonly PowerShell _powershell;
+    private bool _disposedValue;
     private CommandCompletion? _lastCompletion;
     private string? _lastInputText;
 
@@ -49,22 +50,13 @@ public PsCoreShell(
         _powershell.Invoke();
     }
 
+    public bool IsDisposed => _disposedValue;
     public string? SenderConnectionId { get; set; }
 
-    // TODO: Turn into cache and factory.
-    public static IPsCoreShell GetCurrent(string senderConnectionId)
+    public void Dispose()
     {
-        if (_sessions.TryGetValue(senderConnectionId, out var session))
-        {
-            return session;
-        }
-        else
-        {
-            session = Program.Services.GetRequiredService<IPsCoreShell>();
-            session.SenderConnectionId = senderConnectionId;
-            _sessions.AddOrUpdate(senderConnectionId, session, (id, b) => session);
-            return session;
-        }
+        Dispose(disposing: true);
+        GC.SuppressFinalize(this);
     }
 
     public CommandCompletion GetCompletions(string inputText, int currentIndex, bool? forward)
@@ -150,4 +142,16 @@ public async Task<ScriptResultDto> WriteInput(string input)
             };
         }
     }
+
+    protected virtual void Dispose(bool disposing)
+    {
+        if (!_disposedValue)
+        {
+            if (disposing)
+            {
+                _powershell.Dispose();
+            }
+            _disposedValue = true;
+        }
+    }
 }

Agent/Services/ScriptExecutor.cs

@@ -25,11 +25,16 @@ public interface IScriptExecutor
 public class ScriptExecutor : IScriptExecutor
 {
     private readonly IConfigService _configService;
+    private readonly IScriptingShellFactory _scriptingShellFactory;
     private readonly ILogger<ScriptExecutor> _logger;
 
-    public ScriptExecutor(IConfigService configService, ILogger<ScriptExecutor> logger)
+    public ScriptExecutor(
+        IConfigService configService, 
+        IScriptingShellFactory scriptingShellFactory,
+        ILogger<ScriptExecutor> logger)
     {
         _configService = configService;
+        _scriptingShellFactory = scriptingShellFactory;
         _logger = logger;
     }
 
@@ -161,15 +166,15 @@ private async Task<ScriptResultDto> ExecuteScriptContent(
         switch (shell)
         {
             case ScriptingShell.PSCore:
-                return await PsCoreShell
-                    .GetCurrent(terminalSessionId)
+                return await _scriptingShellFactory
+                    .GetOrCreatePsCoreShell(terminalSessionId)
                     .WriteInput(command);
 
             case ScriptingShell.WinPS:
                 if (EnvironmentHelper.IsWindows)
                 {
-                    var instance = await ExternalScriptingShell
-                      .GetCurrent(ScriptingShell.WinPS, terminalSessionId);
+                    var instance = await _scriptingShellFactory
+                      .GetOrCreateExternalShell(ScriptingShell.WinPS, terminalSessionId);
 
                     return await instance.WriteInput(command, timeout);
                 }
@@ -178,17 +183,17 @@ private async Task<ScriptResultDto> ExecuteScriptContent(
             case ScriptingShell.CMD:
                 if (EnvironmentHelper.IsWindows)
                 {
-                    var instance = await ExternalScriptingShell
-                         .GetCurrent(ScriptingShell.CMD, terminalSessionId);
+                    var instance = await _scriptingShellFactory
+                         .GetOrCreateExternalShell(ScriptingShell.CMD, terminalSessionId);
 
                     return await instance.WriteInput(command, timeout);
                 }
                 break;
 
             case ScriptingShell.Bash:
                 {
-                    var instance = await ExternalScriptingShell
-                        .GetCurrent(ScriptingShell.Bash, terminalSessionId);
+                    var instance = await _scriptingShellFactory
+                        .GetOrCreateExternalShell(ScriptingShell.Bash, terminalSessionId);
 
                     return await instance.WriteInput(command, timeout);
                 }