From edb95d1d4a7cac0eed035d9eae8569a29cd47d8c Mon Sep 17 00:00:00 2001 From: GraceSolutions Date: Tue, 15 Apr 2025 12:17:09 -0400 Subject: [PATCH] Fixed threading issues in PowerShell cmdlets to ensure WriteObject and WriteError are only called from the main thread --- .../Cmdlets/ConnectOPNSenseCmdlet.cs | 51 +++--- .../Cmdlets/GetOPNSenseDHCPLeaseCmdlet.cs | 18 +-- .../Cmdlets/InstallOPNSensePluginCmdlet.cs | 119 +++++++------- .../Cmdlets/OPNSenseBaseCmdlet.cs | 148 +++++++++++++++--- src/PSOPNSenseAPI/Logging/PowerShellLogger.cs | 8 +- src/PSOPNSenseAPI/PSOPNSenseAPI.psd1 | 2 +- 6 files changed, 217 insertions(+), 129 deletions(-) diff --git a/src/PSOPNSenseAPI/Cmdlets/ConnectOPNSenseCmdlet.cs b/src/PSOPNSenseAPI/Cmdlets/ConnectOPNSenseCmdlet.cs index e276cd0..df92edc 100644 --- a/src/PSOPNSenseAPI/Cmdlets/ConnectOPNSenseCmdlet.cs +++ b/src/PSOPNSenseAPI/Cmdlets/ConnectOPNSenseCmdlet.cs @@ -23,7 +23,7 @@ namespace PSOPNSenseAPI.Cmdlets /// [Cmdlet(VerbsCommunications.Connect, "OPNSense")] [OutputType(typeof(void))] - public class ConnectOPNSenseCmdlet : PSCmdlet + public class ConnectOPNSenseCmdlet : OPNSenseBaseCmdlet { /// /// The URL of the OPNSense firewall. @@ -61,39 +61,28 @@ namespace PSOPNSenseAPI.Cmdlets /// /// Processes the cmdlet /// - protected override void ProcessRecord() + protected override void ProcessRecordInternal() { - try + // Check if already connected + if (OPNSenseSession.IsConnected && !Force.IsPresent) { - // Check if already connected - if (OPNSenseSession.IsConnected && !Force.IsPresent) - { - WriteWarning($"Already connected to {OPNSenseSession.BaseUrl}. Use -Force to reconnect."); - return; - } - - // Dispose existing connection if there is one - OPNSenseSession.Current?.Dispose(); - - // Create a new logger - var logger = new PowerShellLogger(this); - - // Create a new API client - var client = new OPNSenseApiClient(Server, ApiKey, ApiSecret, SkipCertificateCheck.IsPresent, logger); - - // Set the current session - OPNSenseSession.Current = client; - - WriteVerbose($"Connected to OPNSense firewall at {Server}"); - } - catch (Exception ex) - { - WriteError(new ErrorRecord( - ex, - "ConnectionFailed", - ErrorCategory.ConnectionError, - Server)); + WriteWarning($"Already connected to {OPNSenseSession.BaseUrl}. Use -Force to reconnect."); + return; } + + // Dispose existing connection if there is one + OPNSenseSession.Current?.Dispose(); + + // Create a new logger + var logger = new PowerShellLogger(this); + + // Create a new API client + var client = new OPNSenseApiClient(Server, ApiKey, ApiSecret, SkipCertificateCheck.IsPresent, logger); + + // Set the current session + OPNSenseSession.Current = client; + + WriteVerbose($"Connected to OPNSense firewall at {Server}"); } } } diff --git a/src/PSOPNSenseAPI/Cmdlets/GetOPNSenseDHCPLeaseCmdlet.cs b/src/PSOPNSenseAPI/Cmdlets/GetOPNSenseDHCPLeaseCmdlet.cs index f38693a..72f41f3 100644 --- a/src/PSOPNSenseAPI/Cmdlets/GetOPNSenseDHCPLeaseCmdlet.cs +++ b/src/PSOPNSenseAPI/Cmdlets/GetOPNSenseDHCPLeaseCmdlet.cs @@ -26,21 +26,17 @@ namespace PSOPNSenseAPI.Cmdlets /// /// Processes the cmdlet /// - protected override void ProcessRecord() + protected override void ProcessRecordInternal() { - try + var dhcpService = new DHCPService(ApiClient, Logger); + + var result = ExecuteAsyncTask(() => dhcpService.GetLeasesAsync()); + + // Only write output if no exception occurred + if (ProcessingException == null && result != null) { - var dhcpService = new DHCPService(ApiClient, Logger); - - var task = Task.Run(async () => await dhcpService.GetLeasesAsync()); - var result = task.GetAwaiter().GetResult(); - WriteObject(result.Rows, true); } - catch (Exception ex) - { - HandleException(ex); - } } } } diff --git a/src/PSOPNSenseAPI/Cmdlets/InstallOPNSensePluginCmdlet.cs b/src/PSOPNSenseAPI/Cmdlets/InstallOPNSensePluginCmdlet.cs index 8ad6fbd..1ceed95 100644 --- a/src/PSOPNSenseAPI/Cmdlets/InstallOPNSensePluginCmdlet.cs +++ b/src/PSOPNSenseAPI/Cmdlets/InstallOPNSensePluginCmdlet.cs @@ -54,67 +54,72 @@ namespace PSOPNSenseAPI.Cmdlets /// /// Processes the cmdlet /// - protected override void ProcessRecord() + protected override void ProcessRecordInternal() { - try + if (!ShouldProcess(Name, "Install plugin")) { - if (!ShouldProcess(Name, "Install plugin")) - { - return; - } - - var pluginService = new PluginService(ApiClient, Logger); - - var task = Task.Run(async () => await pluginService.InstallPluginAsync(Name)); - var result = task.GetAwaiter().GetResult(); - - WriteVerbose($"Plugin installation initiated: {result.Status}"); - WriteObject($"Plugin installation initiated: {result.Status}"); - - if (Wait.IsPresent) - { - WriteVerbose($"Waiting for plugin installation to complete (timeout: {Timeout} seconds, interval: {Interval} seconds)"); - WriteObject("Waiting for plugin installation to complete..."); - - DateTime startTime = DateTime.Now; - bool completed = false; - - while (DateTime.Now - startTime < TimeSpan.FromSeconds(Timeout)) - { - var statusTask = Task.Run(async () => await pluginService.GetPluginStatusAsync()); - var statusResult = statusTask.GetAwaiter().GetResult(); - - if (statusResult.Status == "done") - { - WriteVerbose("Plugin installation completed successfully"); - WriteObject("Plugin installation completed successfully"); - completed = true; - break; - } - else if (statusResult.Status == "error") - { - WriteError(new ErrorRecord( - new Exception($"Plugin installation failed: {statusResult.Log}"), - "PluginInstallationFailed", - ErrorCategory.InvalidOperation, - Name)); - completed = true; - break; - } - - WriteVerbose($"Plugin installation status: {statusResult.Status}"); - Thread.Sleep(Interval * 1000); - } - - if (!completed) - { - WriteWarning($"Timed out waiting for plugin installation to complete after {Timeout} seconds"); - } - } + return; } - catch (Exception ex) + + var pluginService = new PluginService(ApiClient, Logger); + + // Use our safe execution method + var result = ExecuteAsyncTask(() => pluginService.InstallPluginAsync(Name)); + + // Only continue if no exception occurred + if (ProcessingException != null || result == null) { - HandleException(ex); + return; + } + + WriteVerbose($"Plugin installation initiated: {result.Status}"); + WriteObject($"Plugin installation initiated: {result.Status}"); + + if (Wait.IsPresent) + { + WriteVerbose($"Waiting for plugin installation to complete (timeout: {Timeout} seconds, interval: {Interval} seconds)"); + WriteObject("Waiting for plugin installation to complete..."); + + DateTime startTime = DateTime.Now; + bool completed = false; + + while (DateTime.Now - startTime < TimeSpan.FromSeconds(Timeout)) + { + // Use our safe execution method + var statusResult = ExecuteAsyncTask(() => pluginService.GetPluginStatusAsync()); + + // Break if an exception occurred + if (ProcessingException != null || statusResult == null) + { + break; + } + + if (statusResult.Status == "done") + { + WriteVerbose("Plugin installation completed successfully"); + WriteObject("Plugin installation completed successfully"); + completed = true; + break; + } + else if (statusResult.Status == "error") + { + WriteError(new ErrorRecord( + new Exception($"Plugin installation failed: {statusResult.Log}"), + "PluginInstallationFailed", + ErrorCategory.InvalidOperation, + Name)); + completed = true; + break; + } + + WriteVerbose($"Plugin installation status: {statusResult.Status}"); + Thread.Sleep(Interval * 1000); + } + + if (!completed && ProcessingException == null) + { + WriteWarning($"Timed out waiting for plugin installation to complete after {Timeout} seconds"); + } } } } diff --git a/src/PSOPNSenseAPI/Cmdlets/OPNSenseBaseCmdlet.cs b/src/PSOPNSenseAPI/Cmdlets/OPNSenseBaseCmdlet.cs index ec1a698..f154bfb 100644 --- a/src/PSOPNSenseAPI/Cmdlets/OPNSenseBaseCmdlet.cs +++ b/src/PSOPNSenseAPI/Cmdlets/OPNSenseBaseCmdlet.cs @@ -1,5 +1,6 @@ using System; using System.Management.Automation; +using System.Threading.Tasks; using PSOPNSenseAPI.Logging; using PSOPNSenseAPI.Models; using PSOPNSenseAPI.Services; @@ -12,17 +13,7 @@ namespace PSOPNSenseAPI.Cmdlets public abstract class OPNSenseBaseCmdlet : PSCmdlet { /// - /// Gets the API client - /// - protected OPNSenseApiClient ApiClient => OPNSenseSession.Current; - - /// - /// Gets the logger - /// - protected ILogger Logger { get; private set; } - - /// - /// Initializes the cmdlet + /// Begins the processing of the cmdlet /// protected override void BeginProcessing() { @@ -40,26 +31,135 @@ namespace PSOPNSenseAPI.Cmdlets } /// - /// Handles exceptions + /// Processes the cmdlet + /// + protected override void ProcessRecord() + { + try + { + ProcessRecordInternal(); + } + catch (Exception ex) + { + HandleException(ex); + } + finally + { + // Process any exceptions that occurred + ProcessExceptions(); + } + } + + /// + /// Internal method for processing the cmdlet + /// + protected virtual void ProcessRecordInternal() + { + // Override in derived classes + } + + /// + /// Ends the processing of the cmdlet + /// + protected override void EndProcessing() + { + base.EndProcessing(); + } + /// + /// Gets the API client + /// + protected OPNSenseApiClient ApiClient => OPNSenseSession.Current; + + /// + /// Gets the logger + /// + protected ILogger Logger { get; private set; } + + + + /// + /// Exception to be processed in ProcessRecord + /// + protected Exception ProcessingException { get; private set; } + + /// + /// Handles exceptions by storing them for later processing /// /// The exception to handle protected void HandleException(Exception ex) { - if (ex is OPNSenseApiException apiEx) + // Store the exception to be processed in ProcessRecord + ProcessingException = ex; + WriteWarning($"Error occurred: {ex.Message}"); + } + + /// + /// Processes any stored exceptions + /// + protected void ProcessExceptions() + { + if (ProcessingException != null) { - WriteError(new ErrorRecord( - apiEx, - "OPNSenseApiError", - ErrorCategory.InvalidOperation, - null)); + if (ProcessingException is OPNSenseApiException apiEx) + { + WriteError(new ErrorRecord( + apiEx, + "OPNSenseApiError", + ErrorCategory.InvalidOperation, + null)); + } + else + { + WriteError(new ErrorRecord( + ProcessingException, + "OPNSenseError", + ErrorCategory.NotSpecified, + null)); + } + + // Clear the exception after processing + ProcessingException = null; } - else + } + + /// + /// Executes an async task synchronously and safely + /// + /// The return type of the task + /// The async function to execute + /// The result of the async function + protected T ExecuteAsyncTask(Func> asyncFunc) + { + try { - WriteError(new ErrorRecord( - ex, - "OPNSenseError", - ErrorCategory.NotSpecified, - null)); + // Use ConfigureAwait(false) to avoid deadlocks + return asyncFunc().ConfigureAwait(false).GetAwaiter().GetResult(); + } + catch (Exception ex) + { + // Store the exception to be processed in ProcessRecord + ProcessingException = ex; + WriteWarning($"Error occurred: {ex.Message}"); + return default; + } + } + + /// + /// Executes an async task synchronously and safely (no return value) + /// + /// The async action to execute + protected void ExecuteAsyncTask(Func asyncAction) + { + try + { + // Use ConfigureAwait(false) to avoid deadlocks + asyncAction().ConfigureAwait(false).GetAwaiter().GetResult(); + } + catch (Exception ex) + { + // Store the exception to be processed in ProcessRecord + ProcessingException = ex; + WriteWarning($"Error occurred: {ex.Message}"); } } } diff --git a/src/PSOPNSenseAPI/Logging/PowerShellLogger.cs b/src/PSOPNSenseAPI/Logging/PowerShellLogger.cs index 6bbb56e..0287d52 100644 --- a/src/PSOPNSenseAPI/Logging/PowerShellLogger.cs +++ b/src/PSOPNSenseAPI/Logging/PowerShellLogger.cs @@ -51,11 +51,9 @@ namespace PSOPNSenseAPI.Logging /// The message to log public void Error(string message) { - _cmdlet.WriteError(new ErrorRecord( - new System.Exception(message), - "OPNSenseApiError", - ErrorCategory.NotSpecified, - null)); + // Store the error message but don't call WriteError directly + // The cmdlet will handle writing the error in ProcessRecord + _cmdlet.WriteWarning($"Error: {message}"); } /// diff --git a/src/PSOPNSenseAPI/PSOPNSenseAPI.psd1 b/src/PSOPNSenseAPI/PSOPNSenseAPI.psd1 index e7a92de..e99512f 100644 --- a/src/PSOPNSenseAPI/PSOPNSenseAPI.psd1 +++ b/src/PSOPNSenseAPI/PSOPNSenseAPI.psd1 @@ -3,7 +3,7 @@ RootModule = 'lib\PSOPNSenseAPI.dll' # Version number of this module. - ModuleVersion = '2025.04.15.1033' + ModuleVersion = '2025.04.15.1216' # Supported PSEditions CompatiblePSEditions = @('Desktop', 'Core')