-
-
Notifications
You must be signed in to change notification settings - Fork 114
feat: timeout diagnostics with stack traces and deadlock detection #4947
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| using System.Text; | ||
|
|
||
| namespace TUnit.Engine.Helpers; | ||
|
|
||
| /// <summary> | ||
| /// Provides diagnostic information when a test or hook times out, | ||
| /// including stack trace capture and deadlock pattern detection. | ||
| /// </summary> | ||
| internal static class TimeoutDiagnostics | ||
| { | ||
| /// <summary> | ||
| /// Common synchronization patterns that may indicate a deadlock when found in a stack trace. | ||
| /// </summary> | ||
| private static readonly (string Pattern, string Hint)[] DeadlockPatterns = | ||
| [ | ||
| ("Monitor.Enter", "A lock (Monitor.Enter) was being acquired. This may indicate a deadlock if another thread holds the lock."), | ||
| ("Monitor.Wait", "Monitor.Wait was called. Ensure the corresponding Monitor.Pulse/PulseAll is reachable."), | ||
| ("SemaphoreSlim.Wait", "SemaphoreSlim.Wait (synchronous) was called. Consider using SemaphoreSlim.WaitAsync() instead."), | ||
| ("ManualResetEvent.WaitOne", "ManualResetEvent.WaitOne was called. The event may never be signaled."), | ||
| ("AutoResetEvent.WaitOne", "AutoResetEvent.WaitOne was called. The event may never be signaled."), | ||
| ("Task.Wait", "Task.Wait (synchronous) was called inside an async context. This can cause deadlocks. Use 'await' instead."), | ||
| ("get_Result", "Task.Result was accessed synchronously. This can cause deadlocks in async contexts. Use 'await' instead."), | ||
| ("TaskAwaiter", "GetAwaiter().GetResult() was called synchronously. This can cause deadlocks in async contexts. Use 'await' instead."), | ||
| ("SpinWait", "A SpinWait was active. The condition being waited on may never become true."), | ||
| ("Thread.Sleep", "Thread.Sleep was called. Consider using Task.Delay in async code."), | ||
| ("Mutex.WaitOne", "Mutex.WaitOne was called. The mutex may be held by another thread or process."), | ||
| ]; | ||
|
|
||
| /// <summary> | ||
| /// Builds an enhanced timeout message that includes diagnostic information. | ||
| /// </summary> | ||
| /// <param name="baseMessage">The original timeout message.</param> | ||
| /// <param name="executionTask">The task that was being executed when the timeout occurred.</param> | ||
| /// <returns>An enhanced message with diagnostics appended.</returns> | ||
| public static string BuildTimeoutDiagnosticsMessage(string baseMessage, Task? executionTask) | ||
| { | ||
| var sb = new StringBuilder(baseMessage); | ||
|
|
||
| AppendTaskStatus(sb, executionTask); | ||
| AppendStackTraceDiagnostics(sb); | ||
|
|
||
| return sb.ToString(); | ||
| } | ||
|
|
||
| private static void AppendTaskStatus(StringBuilder sb, Task? executionTask) | ||
| { | ||
| if (executionTask is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| sb.AppendLine(); | ||
| sb.AppendLine(); | ||
| sb.Append("--- Task Status: "); | ||
| sb.Append(executionTask.Status); | ||
| sb.Append(" ---"); | ||
|
|
||
| if (executionTask.IsFaulted && executionTask.Exception is { } aggregateException) | ||
| { | ||
| sb.AppendLine(); | ||
| sb.Append("Task exception: "); | ||
|
|
||
| foreach (var innerException in aggregateException.InnerExceptions) | ||
| { | ||
| sb.AppendLine(); | ||
| sb.Append(" "); | ||
| sb.Append(innerException.GetType().Name); | ||
| sb.Append(": "); | ||
| sb.Append(innerException.Message); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static void AppendStackTraceDiagnostics(StringBuilder sb) | ||
| { | ||
| string stackTrace; | ||
|
|
||
| try | ||
| { | ||
| stackTrace = Environment.StackTrace; | ||
| } | ||
| catch | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (string.IsNullOrEmpty(stackTrace)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| sb.AppendLine(); | ||
| sb.AppendLine(); | ||
| sb.Append("--- Timeout Handler Stack Trace ---"); | ||
| sb.AppendLine(); | ||
| sb.Append("Note: This is the timeout handler's stack trace, not the blocked test's stack."); | ||
| sb.AppendLine(); | ||
| sb.Append(stackTrace); | ||
|
|
||
| var hints = DetectDeadlockPatterns(stackTrace); | ||
|
|
||
| if (hints.Count > 0) | ||
| { | ||
| sb.AppendLine(); | ||
| sb.AppendLine(); | ||
| sb.Append("--- Potential Deadlock Detected ---"); | ||
|
|
||
| foreach (var hint in hints) | ||
| { | ||
| sb.AppendLine(); | ||
| sb.Append(" * "); | ||
| sb.Append(hint); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Scans a stack trace for common patterns that may indicate a deadlock. | ||
| /// </summary> | ||
| internal static List<string> DetectDeadlockPatterns(string stackTrace) | ||
| { | ||
| var hints = new List<string>(); | ||
|
|
||
| foreach (var (pattern, hint) in DeadlockPatterns) | ||
| { | ||
| #if NET | ||
| if (stackTrace.Contains(pattern, StringComparison.OrdinalIgnoreCase)) | ||
| #else | ||
| if (stackTrace.IndexOf(pattern, StringComparison.OrdinalIgnoreCase) >= 0) | ||
| #endif | ||
| { | ||
| hints.Add(hint); | ||
| } | ||
| } | ||
|
|
||
| return hints; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical:
Environment.StackTracecaptures the wrong call stackEnvironment.StackTraceonly captures the stack of the current thread — the timeout handler continuation — not the stack of theexecutionTaskthat actually hung.By the time
BuildTimeoutDiagnosticsMessageis called (afterTask.WhenAnyreturns + grace period expires), the current thread's stack looks roughly like:The blocking patterns you're scanning for (
Monitor.Enter,SemaphoreSlim.Wait, etc.) would appear on an entirely different thread — the one that's actually stuck — or may not appear on any active thread at all (if the task isawaiting aTaskCompletionSourcewith no active thread holding it).The diagnostic output will label this the "Timeout Stack Trace" and claim to detect deadlocks, but it's showing the timeout infrastructure's own call stack rather than the user's blocked code.
Better approach: Capture
Environment.StackTraceat the start of the task delegate (or useAsyncLocal<string>to propagate it), then include that in the timeout message. For example:Alternatively, for
IsFaultedtasks the existing code already captures the exception — that's the most useful diagnostic for those cases. For still-running tasks on another thread, there's no safe public API to capture their stack without a debugger.