Skip to content

Fix CoreclrTestWrapperLib to be robust against exited processes#113937

Merged
jkotas merged 1 commit into
dotnet:mainfrom
jkotas:112846
Mar 27, 2025
Merged

Fix CoreclrTestWrapperLib to be robust against exited processes#113937
jkotas merged 1 commit into
dotnet:mainfrom
jkotas:112846

Conversation

@jkotas

@jkotas jkotas commented Mar 26, 2025

Copy link
Copy Markdown
Member

Fixes #112846

Copilot AI review requested due to automatic review settings March 26, 2025 18:41
@ghost ghost added the area-Infrastructure-coreclr Only use for closed issues label Mar 26, 2025
@jkotas jkotas requested review from hoyosjs and jkoritzinsky March 26, 2025 18:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request improves the robustness of process handling in CoreclrTestWrapperLib by adding new extension methods for safely retrieving process IDs and names, and refactoring the child process search logic.

  • Introduces TryGetProcessId and TryGetProcessName extension methods.
  • Updates the FindChildProcessesByName method to use the new extension methods.
  • Makes minor adjustments to exception handling in the symbolizer output code.


} catch (Exception e) {
}
catch (Exception e)

Copilot AI Mar 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The modified exception handling in the symbolizer output code introduces an additional catch block. Consider consolidating the error handling to streamline the code and ensure that no redundant catch blocks exist.

Copilot uses AI. Check for mistakes.
Process child = childrenToCheck.Dequeue();
if (seen.Contains(child.Id))

if (!child.TryGetProcessId(out int processId))

Copilot AI Mar 26, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While using the new TryGetProcessId extension method enhances robustness, consider adding logging of the failure cases within the extension method to aid in debugging when a process has already exited.

Copilot uses AI. Check for mistakes.
@jkotas jkotas changed the title Fix CoreclrTestWrapperLib to be robust against existed processes Fix CoreclrTestWrapperLib to be robust against exited processes Mar 26, 2025
@jkotas

jkotas commented Mar 27, 2025

Copy link
Copy Markdown
Member Author

/ba-g infrastructure timeout

@jkotas jkotas merged commit 0ee8cba into dotnet:main Mar 27, 2025
@jkotas jkotas deleted the 112846 branch March 27, 2025 00:14
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Infrastructure-coreclr Only use for closed issues

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Test failure: baseservices/exceptions/unhandled/unhandledTester/unhandledTester.cmd

4 participants