dotnet / dotnet/runtime

Debugger DbgTransportSession has shutdown safety issues

Open
#126,393 1 comment 0 reactions 0 assignees View on GitHub
area-Diagnostics-coreclr
Dominant language
C#
Stars
18.3k
Forks
5.6k
PR merge metrics
PR metrics pending

Description

See https://github.com/dotnet/runtime/pull/126372#issuecomment-4167384494 for more background. Copying my post from there here:

---

The delayed thread visibility in copilot's 2nd explanation does sound plausible if the process was very short-lived or OS thread-scheduling delayed the transport worker to start close to the same time the main thread was shutting down. I see some odd behavior where the channel is initially created on the main thread inside of DbgTransportSession::Init() but then it starts the worker thread and the worker thread immediately disconnects that channel, NULLs out the field, and creates a new one. That would open a brief window where the field transitions non-NULL -> NULL -> non-NULL without any debugger interaction.

It looks like there are multiple issues lurking here:

Disconnecting the initial channel might ignore a valid debugger connection
The potential for the NULL deref in AbortConnection
The potential that AbortConnection runs CloseConnection() concurrently with the worker when its not clear the method supports concurrency
AbortConnection doesn't change the worker's state to SS_Closed so the worker will race process exit to re-create the connection.
I'm fine with the NULL check but agreed with @jkotas, I think its a partial mitigation at best.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.