Fix orphaned requirement and bound clang subprocess timeout

docs/reqstream/api-mark-dot-net.yaml

@@ -40,6 +40,7 @@ sections:
           - ApiMarkDotNet-DotNetEmitter-EmitConversionOperatorsWithConversionSyntax
           - ApiMarkDotNet-DotNetEmitter-RenderClassModifiersInTypeSignature
           - ApiMarkDotNet-DotNetEmitter-RenderInitOnlyPropertyAccessor
+          - ApiMarkDotNet-DotNetEmitter-RenderConstantFieldValue
           - ApiMarkDotNet-DotNetEmitterGradualDisclosure-EmitApiIndexPage
           - ApiMarkDotNet-DotNetEmitterGradualDisclosure-EmitNamespacePages
           - ApiMarkDotNet-DotNetEmitterGradualDisclosure-EmitNamespaceDocRemarksAndExamples

src/ApiMark.Cpp/CppAst/ClangAstParser.cs

@@ -445,7 +445,7 @@
                 // clang the user has installed; callers can override via APIMARK_CLANG_PATH or
                 // an absolute ClangPath option. xcrun is only used here to probe for macOS
                 // Command Line Tools availability and is always a fixed system utility on PATH.
                 FileName = "xcrun",
                 RedirectStandardOutput = true,
                 RedirectStandardError = true,
                 UseShellExecute = false,
@@ -702,11 +702,22 @@
     ///     Starts a process, reads stdout and stderr concurrently to avoid pipe deadlock,
     ///     and returns the combined output with the exit code.
     /// </summary>
+    /// <remarks>
+    ///     The wait is bounded by <see cref="ClangTimeoutMilliseconds"/> so that a clang (or
+    ///     <c>xcrun</c>) invocation that never exits — e.g. a first-run Xcode license prompt
+    ///     blocked on stdin, or a process starved by heavy CI-runner contention — fails this
+    ///     single call instead of hanging the calling test (and, transitively, the whole CI
+    ///     job) indefinitely. The process (and any children it spawned) is killed on timeout.
+    ///     The resulting exception carries the OS, PID, elapsed time, full command line, and
+    ///     any partial stdout/stderr the process had already produced, so a hang can be
+    ///     diagnosed from CI output alone without needing to reproduce it locally.
+    /// </remarks>
     /// <param name="fileName">Executable to launch.</param>
     /// <param name="arguments">Arguments to pass to the executable.</param>
     /// <returns>A tuple of (stdout, stderr, exitCode).</returns>
     /// <exception cref="InvalidOperationException">
-    ///     Thrown when the process cannot be started.
+    ///     Thrown when the process cannot be started, or when it does not exit within
+    ///     <see cref="ClangTimeoutMilliseconds"/>.
     /// </exception>
     private static (string Stdout, string Stderr, int ExitCode) RunProcess(
         string fileName,
@@ -726,19 +737,122 @@
             psi.ArgumentList.Add(arg);
         }
 
+        var commandLine = BuildDiagnosticCommandLine(fileName, arguments);
+        var stopwatch = Stopwatch.StartNew();
+
         using var process = Process.Start(psi)
-            ?? throw new InvalidOperationException($"Failed to start process '{fileName}'.");
+            ?? throw new InvalidOperationException(
+                $"Failed to start process '{fileName}'. Command: {commandLine}");
+
+        // Capture the PID up front — it is still readable after Kill(), but not reliably
+        // so once the process has fully exited and been reaped on some platforms
+        var processId = process.Id;
 
         // Read stdout and stderr concurrently — reading them sequentially can deadlock
         // when one pipe's buffer fills while the other is not being drained
         var stdoutTask = process.StandardOutput.ReadToEndAsync();
         var stderrTask = process.StandardError.ReadToEndAsync();
 
-        process.WaitForExit();
+        var timeoutMilliseconds = TimeoutOverrideMillisecondsForTests ?? ClangTimeoutMilliseconds;
+        if (!process.WaitForExit(timeoutMilliseconds))
+        {
+            TryKillProcess(process);
+
+            // Best-effort: give the now-killed process's pipes a short grace period to flush
+            // so partial output (e.g. a diagnostic clang had already printed) can be reported.
+            // This wait is bounded independently of timeoutMilliseconds so a pipe that
+            // never closes cannot extend the hang.
+            var partialStdout = TryGetPartialOutput(stdoutTask);
+            var partialStderr = TryGetPartialOutput(stderrTask);
+
+            throw new InvalidOperationException(
+                $"Process did not exit within {timeoutMilliseconds}ms and was killed.\n" +
+                $"  OS: {RuntimeInformation.OSDescription} ({RuntimeInformation.OSArchitecture})\n" +
+                $"  PID: {processId}\n" +
+                $"  Elapsed: {stopwatch.Elapsed}\n" +
+                $"  Command: {commandLine}\n" +
+                $"  Partial stdout ({partialStdout.Length} chars): {Truncate(partialStdout)}\n" +
+                $"  Partial stderr ({partialStderr.Length} chars): {Truncate(partialStderr)}");
+        }
 
         return (stdoutTask.Result, stderrTask.Result, process.ExitCode);
     }
 
+    /// <summary>
+    ///     Maximum time to wait for the clang (or <c>xcrun</c>-wrapped clang) AST-dump
+    ///     invocation to exit before treating it as hung and killing it.
+    /// </summary>
+    private const int ClangTimeoutMilliseconds = 120_000;
+
+    /// <summary>
+    ///     Test-only override for the timeout applied in <see cref="RunProcess"/>, scoped to
+    ///     the calling thread via <see cref="ThreadStaticAttribute"/> so a test exercising the
+    ///     timeout/kill path with an artificially short wait cannot affect the (ordinarily
+    ///     much faster) real clang invocations made concurrently by other tests. Tests must
+    ///     reset this to <see langword="null"/> in a <c>finally</c> block — thread-pool thread
+    ///     reuse would otherwise leak the override into an unrelated later test.
+    /// </summary>
+    [ThreadStatic]
+    internal static int? TimeoutOverrideMillisecondsForTests;
+
+    /// <summary>
+    ///     Maximum time to wait, after a hung process has been killed, for its redirected
+    ///     stdout/stderr pipes to report as complete so partial output can be captured.
+    /// </summary>
+    private const int PartialOutputGraceMilliseconds = 2000;
+
+    /// <summary>
+    ///     Maximum number of characters of partial stdout/stderr to include in a timeout
+    ///     exception message, so a chatty hung process does not produce an unreadable error.
+    /// </summary>
+    private const int PartialOutputMaxChars = 2000;
+
+    /// <summary>
+    ///     Builds a human-readable, shell-quoted rendering of a command line for diagnostic
+    ///     messages. Not used to actually invoke the process — <see cref="ProcessStartInfo.ArgumentList"/>
+    ///     already handles that safely.
+    /// </summary>
+    /// <param name="fileName">Executable name or path.</param>
+    /// <param name="arguments">Arguments passed to the executable.</param>
+    /// <returns>A single-line, space-separated, quoted command line for logging.</returns>
+    private static string BuildDiagnosticCommandLine(string fileName, IReadOnlyList<string> arguments)
+    {
+        var parts = new[] { fileName }.Concat(arguments)
+            .Select(a => a.Contains(' ', StringComparison.Ordinal) ? $"\"{a}\"" : a);
+        return string.Join(' ', parts);
+    }
+
+    /// <summary>
+    ///     Best-effort retrieval of a stdout/stderr read task's result after the owning
+    ///     process has been killed, bounded by <see cref="PartialOutputGraceMilliseconds"/> so
+    ///     a pipe that never reports complete cannot extend the timeout exception path.
+    /// </summary>
+    /// <param name="readTask">The <see cref="StreamReader.ReadToEndAsync()"/> task to probe.</param>
+    /// <returns>The captured text, or an empty string if it did not complete in time.</returns>
+    private static string TryGetPartialOutput(Task<string> readTask)
+    {
+        try
+        {
+            return readTask.Wait(PartialOutputGraceMilliseconds) ? readTask.Result : string.Empty;
+        }
+        catch (AggregateException)
+        {
+            // The read itself faulted (e.g. pipe torn down mid-read) — no partial output available
+            return string.Empty;
+        }
+    }
+
+    /// <summary>
+    ///     Truncates diagnostic text to <see cref="PartialOutputMaxChars"/> so a chatty hung
+    ///     process cannot produce an unreadable exception message.
+    /// </summary>
+    /// <param name="text">The text to truncate.</param>
+    /// <returns>The original text, or a truncated copy with a trailing marker.</returns>
+    private static string Truncate(string text) =>
+        text.Length <= PartialOutputMaxChars
+            ? text
+            : string.Concat(text.AsSpan(0, PartialOutputMaxChars), "… [truncated]");
+
     /// <summary>
     ///     Extracts error-class lines from clang's stderr output, used to populate
     ///     <see cref="CppCompilationResult.Errors"/>.
@@ -1151,7 +1265,7 @@
     /// <param name="isDeprecated">Whether the class is deprecated, set in place by a <c>DeprecatedAttr</c> child.</param>
     /// <param name="isFinal">Whether the class is final, set in place by a <c>FinalAttr</c> child.</param>
     /// <param name="doc">The class documentation comment, set in place by a <c>FullComment</c> child.</param>
     private void ProcessClassBodyChild(
         JsonElement child,
         string nsQualName,
         string className,
@@ -1789,7 +1903,7 @@
     /// <param name="node">The field JSON node.</param>
     /// <param name="accessibility">The current access level at the point of declaration.</param>
     /// <returns>A populated <see cref="CppField"/>, or <see langword="null"/> for implicit members.</returns>
     private static CppField? ParseField(JsonElement node, CppAccessibility accessibility)
     {
         // Skip compiler-synthesized fields
         if (node.TryGetProperty(IsImplicitProperty, out var impl) && impl.GetBoolean())
@@ -1885,7 +1999,7 @@
             // Numeric and string literals carry their value as a JSON string, already formatted
             // the way clang would print them (strings already include surrounding quotes)
             "IntegerLiteral" or "FloatingLiteral" or "StringLiteral" =>
                 node.TryGetProperty("value", out var v) ? v.GetString() : null,
 
             // Boolean literals carry their value as a JSON boolean, not a string
             "CXXBoolLiteralExpr" => ExtractBoolLiteralDefaultValue(node),

test/ApiMark.Cpp.Tests/ClangAstParserTests.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using System.Diagnostics;
+using System.Runtime.InteropServices;
 using ApiMark.Cpp;
 using ApiMark.Cpp.CppAst;
 using Xunit;
@@ -514,4 +515,87 @@ public void ClangAstParser_Parse_CalledDirectlyWithDifferentlyCasedIncludeRoot_R
             Directory.Delete(tempParent, recursive: true);
         }
     }
+
+    /// <summary>
+    ///     Writes a tiny script that ignores every argument it is given and sleeps far longer
+    ///     than any test timeout, so it can stand in for a hung clang invocation without
+    ///     depending on clang (or any OS-specific hang trigger such as an Xcode license prompt)
+    ///     being reproducible in this environment.
+    /// </summary>
+    /// <param name="directory">Directory to create the script file in.</param>
+    /// <returns>Absolute path to the created, executable script.</returns>
+    private static string CreateHangingScript(string directory)
+    {
+        if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+        {
+            // .NET's Process.Start transparently runs .cmd/.bat files through cmd.exe on
+            // Windows, so this can be used directly as CppGeneratorOptions.ClangPath.
+            var scriptPath = Path.Combine(directory, "hang.cmd");
+            File.WriteAllText(scriptPath, "@echo off\r\nping -n 1000 127.0.0.1 >nul\r\n");
+            return scriptPath;
+        }
+
+        var shPath = Path.Combine(directory, "hang.sh");
+        File.WriteAllText(shPath, "#!/bin/sh\nsleep 1000\n");
+        File.SetUnixFileMode(
+            shPath,
+            UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute |
+            UnixFileMode.GroupRead | UnixFileMode.GroupExecute |
+            UnixFileMode.OtherRead | UnixFileMode.OtherExecute);
+        return shPath;
+    }
+
+    /// <summary>
+    ///     Validates that a clang invocation which never exits is killed and reported as an
+    ///     <see cref="InvalidOperationException"/> bearing diagnostic detail, instead of hanging
+    ///     the test (and, in CI, the whole job) indefinitely.
+    /// </summary>
+    [Fact]
+    public void ClangAstParser_Parse_ClangHangs_ThrowsWithDiagnosticsAndKillsProcess()
+    {
+        // Arrange: a fake "clang" that ignores its arguments and sleeps indefinitely, combined
+        // with a short thread-local timeout override so this test does not actually wait for
+        // the production 120-second timeout
+        var tempDir = Path.Combine(Path.GetTempPath(), "ApiMarkClangHangTest_" + Guid.NewGuid().ToString("N"));
+        Directory.CreateDirectory(tempDir);
+        try
+        {
+            var hangingScript = CreateHangingScript(tempDir);
+            var options = new CppGeneratorOptions
+            {
+                LibraryName = "Fixtures",
+                PublicIncludeRoots = [],
+                ClangPath = hangingScript,
+            };
+
+            ClangAstParser.TimeoutOverrideMillisecondsForTests = 300;
+            try
+            {
+                // Act
+                var ex = Assert.Throws<InvalidOperationException>(
+                    () => ClangAstParser.Parse(["dummy.h"], options));
+
+                // Assert: the exception carries enough diagnostic detail to triage a real hang
+                // from CI output alone, without needing to reproduce it locally
+                Assert.Contains("did not exit within 300ms", ex.Message, StringComparison.Ordinal);
+                Assert.Contains("OS:", ex.Message, StringComparison.Ordinal);
+                Assert.Contains("PID:", ex.Message, StringComparison.Ordinal);
+                Assert.Contains("Elapsed:", ex.Message, StringComparison.Ordinal);
+                Assert.Contains("Command:", ex.Message, StringComparison.Ordinal);
+                Assert.Contains(hangingScript, ex.Message, StringComparison.Ordinal);
+                Assert.Contains("Partial stdout", ex.Message, StringComparison.Ordinal);
+                Assert.Contains("Partial stderr", ex.Message, StringComparison.Ordinal);
+            }
+            finally
+            {
+                // Reset the override — thread-pool thread reuse would otherwise leak a
+                // 300ms timeout into an unrelated later test running on the same thread
+                ClangAstParser.TimeoutOverrideMillisecondsForTests = null;
+            }
+        }
+        finally
+        {
+            Directory.Delete(tempDir, recursive: true);
+        }
+    }
 }