Skip to content

Make stdout/stderr redirection thread timeout configurable #49942

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

Conversation

adityamandaleeka
Copy link
Member

Contributes to #45066

The StandardStreamRedirection logic in ANCM uses CancelSynchronousIo to kick the redirection thread out of its blocking ReadFile call and waits a hardcoded 2 seconds to allow the thread to exit before it calls TerminateThread on the thread. This works reasonably well in the vast majority of cases (most of the time, the 2 seconds is enough), but occasionally, we have to resort to the TerminateThread and in a small number of those cases, the thread might be doing something dangerous like holding the OS loader lock.

I'd like to eventually rewrite this whole mechanism to eliminate TerminateThread entirely (as I did in the file watcher in #49696) but given where we are in the .NET 8 schedule, it's important to minimize the risk involved. This change just makes the timeout for thread termination configurable, so that people who are running into issues with this can give the thread more than 2 seconds to exit.

@BrennanConroy

Copy link
Member

@BrennanConroy BrennanConroy left a comment

Choose a reason for hiding this comment

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

Sad we couldn't restructure it. Let's make sure we have an issue for 9 😃

@adityamandaleeka adityamandaleeka merged commit 34a3d4a into dotnet:main Aug 9, 2023
@ghost ghost added this to the 8.0-rc1 milestone Aug 9, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants