Merge branch 'main' into 1616-allow-documenting-unnamed-arguments

opentap · Jun 20, 2024 · 7ec49c4 · 7ec49c4
2 parents eda4ffe + 449e501
commit 7ec49c4
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 23 deletions.
diff --git a/BasicSteps/ProcessStep.cs b/BasicSteps/ProcessStep.cs
@@ -110,7 +110,6 @@ public Int32 Timeout
         [EnabledIf(nameof(WaitForEnd), true, HideIfDisabled = true)]
         public int ExitCode { get; private set; }
 
-        ManualResetEvent outputWaitHandle, errorWaitHandle;
         StringBuilder output;
 
 
@@ -153,7 +152,7 @@ public override void Run()
                     // Set RunElevated = false so ProcessHelper doesn't infinitely loop
                     RunElevated = false;
                     var processRunner = new SubProcessHost { ForwardLogs = AddToLog, LogHeader = LogHeader};
-                    var verdict = processRunner.Run(this, true, CancellationToken.None);
+                    var verdict = processRunner.Run(this, true, TapThread.Current.AbortToken);
                     UpgradeVerdict(verdict);
                     return;
                 }
@@ -217,8 +216,6 @@ public override void Run()
             {
                 output = new StringBuilder();
 
-                using (outputWaitHandle = new ManualResetEvent(false))
-                using (errorWaitHandle = new ManualResetEvent(false))
                 using(process)
                 using(abortRegistration)
                 {
@@ -230,10 +227,29 @@ public override void Run()
 
                     process.BeginOutputReadLine();
                     process.BeginErrorReadLine();
+
+                    var elapsed = Stopwatch.StartNew();
+                    // Wait for the process to exit, allowing for cancellation every 100 ms
+                    while (elapsed.Elapsed.TotalSeconds < timeout && process.WaitForExit(100) == false)
+                        TapThread.ThrowIfAborted();
+
+                    // Ensure that all asynchronous processing is completed by calling process.WaitForExit()
+                    // Only the overload with no parameters has this guarantee. See remarks at
+                    // https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.process.waitforexit?view=netframework-4.8
+                    var done = new ManualResetEventSlim(false);
+                    TapThread.Start(() =>
+                    {
+                        // Wrap it in a thread because this can block indefinitely due to a bug in dotnet. See:
+                        // https://github.com/dotnet/runtime/issues/74677
+                        // https://github.com/dotnet/runtime/issues/28583
+                        process.WaitForExit();
+                        done.Set();
+                    });
+
+                    while (elapsed.Elapsed.TotalSeconds < timeout && done.WaitHandle.WaitOne(100) == false)
+                        TapThread.ThrowIfAborted();
 
-                    if (process.WaitForExit(timeout) &&
-                        outputWaitHandle.WaitOne(timeout) &&
-                        errorWaitHandle.WaitOne(timeout))
+                    if (elapsed.Elapsed.TotalSeconds < timeout)
                     {
                         var resultData = output.ToString();
 
@@ -273,7 +289,6 @@ public override void Run()
                     {
                         process.Start();
                         process.WaitForExit();
-                        abortRegistration.Dispose();
                     }
                 });
             }
@@ -283,11 +298,7 @@ void OutputDataRecv(object sender, DataReceivedEventArgs e)
         {
             try
             {
-                if (e.Data == null)
-                {
-                    outputWaitHandle.Set();
-                }
-                else
+                if (e.Data != null)
                 {
                     if(AddToLog)
                         Log.Info("{0}{1}", prepend, e.Data);
@@ -305,11 +316,7 @@ void ErrorDataRecv(object sender, DataReceivedEventArgs e)
         {
             try
             {
-                if (e.Data == null)
-                {
-                    errorWaitHandle.Set();
-                }
-                else
+                if (e.Data != null)
                 {
                     if(AddToLog)
                         Log.Error("{0}{1}", prepend, e.Data);

diff --git a/Engine/Cli/CliActionExecutor.cs b/Engine/Cli/CliActionExecutor.cs
@@ -285,10 +285,14 @@ string getVersion()
                 }
                 string tapCommand = OperatingSystem.Current == OperatingSystem.Windows ? "tap.exe" : "tap";
 
-                if (args.Length != 0)
+                var helpOptions = new string[] { "--help", "-help", "-h" };
+                bool isHelp = args.Length == 0 || args.Any(a => helpOptions.Contains(a.ToLower()));
+
+                if (!isHelp)
                 {
                     log.Error($"\"{tapCommand} {string.Join(" ", args)}\" is not a recognized command.");
                 }
+
                 log.Info("OpenTAP Command Line Interface ({0})", getVersion());
                 log.Info($"Usage: \"{tapCommand} <command> [<subcommand(s)>] [<args>]\"\n");
 
@@ -308,19 +312,22 @@ string getVersion()
 
                 log.Info($"\nRun \"{tapCommand} <command> [<subcommand(s)>] -h\" to get additional help for a specific command.\n");
 
-                if (args.Length == 0 || args.Any(s => s.ToLower() == "--help" || s.ToLower() == "-h"))
+                if (isHelp)
                     return (int)ExitCodes.Success;
                 else
                     return (int)ExitCodes.ArgumentParseError;
             }
 
             if (SelectedAction != TypeData.FromType(typeof(RunCliAction)) && UserInput.Interface == null) // RunCliAction has --non-interactive flag and custom platform interaction handling.          
                 CliUserInputInterface.Load();
-            
+
             ICliAction packageAction = null;
-            try{
+            try
+            {
                 packageAction = (ICliAction)SelectedAction.CreateInstance();
-            }catch(TargetInvocationException e1) when (e1.InnerException is System.ComponentModel.LicenseException e){
+            }
+            catch (TargetInvocationException e1) when (e1.InnerException is System.ComponentModel.LicenseException e)
+            {
                 log.Error("Unable to load CLI Action '{0}'", SelectedAction.GetDisplayAttribute().GetFullName());
                 log.Info("{0}", e.Message);
                 return (int)ExitCodes.UnknownCliAction;