Skip to content
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

Deadlock in ThreadPool.GetMaxThreads() during app start. #93175

Closed
avparuch opened this issue Oct 7, 2023 · 8 comments
Closed

Deadlock in ThreadPool.GetMaxThreads() during app start. #93175

avparuch opened this issue Oct 7, 2023 · 8 comments

Comments

@avparuch
Copy link

avparuch commented Oct 7, 2023

Description

There is a deadlock that occurs transiently when our .NET 6.0 application is starting. From the call stack, the application is hanging on a call to Threadpool.GetMaxThreads(). Looking at the dump with repro with the 2 deadlocked threads, the deadlock involves locking around the ThreadPool type constructor and a lock taken in the EventListener class:

image

image

The dump has been shared with the .NET team, filing this issue at the request of the .NET team for tracking.

Reproduction Steps

It's a not trivial repro. It involves running a .NET 6.0 app that calls into Threadpool.GetMaxThreads() during application startup and an external entity (on the same machine) triggering an ETW session that, in turn instantiates EventSource inside the application at the same time.

From David Mason (@davmason):
image

Expected behavior

No deadlock.

Actual behavior

There is a deadlock.

Regression?

No response

Known Workarounds

@davmason suggested creating the EventListener explicitly before calling ThreadPool.GetMaxThreads() as a workaround. I can confirm this works.

class RuntimeEventListener : EventListener
{
    internal static int eventCount;

    protected override void OnEventSourceCreated(EventSource eventSource)
    {
        base.OnEventSourceCreated(eventSource);

        if (eventSource.Name.Equals("System.Runtime"))
        {
            Dictionary<string, string> refreshInterval = new Dictionary<string, string>();
            refreshInterval["EventCounterIntervalSec"] = "1";
            EnableEvents(source, EventLevel.Informational, (EventKeywords)(-1), refreshInterval);
        }
    }

    protected override void OnEventWritten(EventWrittenEventArgs eventData)
    {
        base.OnEventWritten(eventData);
    }
}

And then create one at the beginning of Main before calling ThreadPool.GetMaxThreads():
RuntimeEventListener listener = new RuntimeEventListener();

Configuration

.NET 6.0.20

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 7, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 7, 2023
@jkotas jkotas added area-System.Threading and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Oct 7, 2023
@ghost
Copy link

ghost commented Oct 7, 2023

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

We found a deadlock that occurs transiently when our application is starting. From the call stack, the application is hanging on a call to Threadpool.GetMaxThreads(). Looking at the dump with repro with the 2 deadlocked threads, the deadlock involves locking around the ThreadPool type constructor and a lock taken in the EventListener class:

image

image

The dump has been shared with the .NET team, filing this issue at the request of the .NET team for tracking.

Reproduction Steps

From David Mason (@davmason):
image

Expected behavior

No deadlock.

Actual behavior

There is a deadlock.

Regression?

No response

Known Workarounds

@davmason suggested creating the EventListener explicitly before calling ThreadPool.GetMaxThreads() as a workaround. I can confirm this works.

class RuntimeEventListener : EventListener
{
    internal static int eventCount;

    protected override void OnEventSourceCreated(EventSource eventSource)
    {
        base.OnEventSourceCreated(eventSource);

        if (eventSource.Name.Equals("System.Runtime"))
        {
            Dictionary<string, string> refreshInterval = new Dictionary<string, string>();
            refreshInterval["EventCounterIntervalSec"] = "1";
            EnableEvents(source, EventLevel.Informational, (EventKeywords)(-1), refreshInterval);
        }
    }

    protected override void OnEventWritten(EventWrittenEventArgs eventData)
    {
        base.OnEventWritten(eventData);
    }
}

And then create one at the beginning of Main:
RuntimeEventListener listener = new RuntimeEventListener();

Configuration

.NET 6.0.20

Other information

No response

Author: avparuch
Assignees: -
Labels:

area-System.Threading, untriaged, needs-area-label

Milestone: -

@mangod9 mangod9 removed the untriaged New issue has not been triaged by the area owner label Oct 9, 2023
@mangod9 mangod9 added this to the 9.0.0 milestone Oct 9, 2023
@mangod9
Copy link
Member

mangod9 commented Oct 9, 2023

@kouvel, have you seen this issue before?

@kouvel
Copy link
Member

kouvel commented Oct 9, 2023

I haven't seen the issue before, but it makes sense and it sounds like @davmason has looked into it a bit. @davmason, is this something that can be generically fixed from the EventSource side? It should be possible to fix from the ThreadPool side too, though it may not be the only case where this lock ordering occurs.

Copy link
Contributor

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

@davmason
Copy link
Member

The problem here is that we try to populate the counter values while holding the EventSource EventListenerLock. We should instead create the counters and not immediately populate the values, then let them populate when the timer fires on a different thread.

PJB3005 added a commit to space-wizards/RobustToolbox that referenced this issue Jun 20, 2024
For some reason we call ThreadPool.SetMinThreads on startup of the game server. Calling this function this early seems to put us at high risk of triggering the following deadlock bug in the .NET runtime: dotnet/runtime#93175

Given I have zero trust in whether this manual ThreadPool fuckery is even helpful, I'm just gonna nuke it and call it a day.
@tommcdon tommcdon modified the milestones: 9.0.0, 10.0.0 Jul 23, 2024
@PJB3005
Copy link
Contributor

PJB3005 commented Aug 3, 2024

I know this isn't the most productive but I would just like to say that Rider automatically attaches EventPipe on app start now, which makes any code that runs ThreadPool's cctor extremely likely to deadlock just on startup. This is now forcing me to disable Rider's monitoring feature as I cannot start my app 5 times in a row from deadlocks. And I've had this happen before even on bog-standard ASP.NET Core apps.

@ldy985
Copy link

ldy985 commented Aug 5, 2024

My team had the same problem with the rider, which led me here.

Note: I needed a way to +1. If anyone else has the problem, make a thumbs-up on @PJB3005's comment.

@mdh1418
Copy link
Member

mdh1418 commented Sep 23, 2024

I believe this is fixed by #105548. I was able to repro the issue on .NET Sdk 8.0.300, and using a local build of runtime with the PR I could not trigger the deadlock. This issue seemed to have the same underlying issue as #105682, which I based the repro on.

@mdh1418 mdh1418 closed this as completed Sep 23, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants