Uh oh!
There was an error while loading. Please reload this page.
Update CoreclrTestWrapperLib to use diagnostics IPC for hang dump collection - #133253
Conversation
Request dumps for timed-out tests through diagnostics IPC while preserving verbose dump output and detailed failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Tagging subscribers to this area: @dotnet/runtime-infrastructure |
There was a problem hiding this comment.
🟡 Changes recommended
The new timeout path introduces unbounded waits/long IPC timeouts that can hang or significantly extend CI after a test timeout, and should be made best-effort and bounded.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the CoreCLR test wrapper’s timeout-time crash dump collection on non-Windows platforms to request dumps via the .NET diagnostics IPC socket instead of invoking createdump, and restructures timeout reporting so diagnostic output can be captured and appended to the test output.
Changes:
- Add diagnostics IPC message construction + Unix domain socket client to request
GenerateCoreDump3dumps on non-Windows. - Route dump-collection logs into an in-memory writer during timeout handling and append them to the test output after killing the timed-out process tree.
- Generalize dump collection helpers to accept
TextWriterinstead ofStreamWriter.
File summaries
| File | Description |
|---|---|
| src/tests/Common/CoreCLRTestLibrary/CoreclrTestWrapperLib.cs | Replace non-Windows dump collection with diagnostics IPC requests and adjust timeout handling/output to capture dump diagnostics. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Use dedicated IPC and output-copy timeouts so hang diagnostics remain best effort. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new diagnostics IPC socket probing and multi-socket handling have correctness issues that can cause dump collection to fail unnecessarily in some environments.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/tests/Common/CoreCLRTestLibrary/CoreclrTestWrapperLib.cs:318
- If TMPDIR is set but points to a missing/inaccessible directory, dump collection fails immediately even though the runtime’s diagnostic socket is typically under /tmp. Also, the failure message doesn’t identify which directory/pattern was searched, which makes diagnosing CI/environment issues harder.
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Request dumps for timed-out tests through diagnostics IPC while preserving verbose dump output and detailed failures.