egametang / ET

Unity3D Client And C# Server Framework
Other
8.95k stars 3.05k forks source link

High CPU caused by HashSet infinite loop in StaticClassCircularDependencyAnalyzer.StaticClassDependencyAnalyze #452

Open AlexDelepine opened 1 year ago

AlexDelepine commented 1 year ago

Description

Hello, I am on the Visual Studio performance team and our telemetry has shown that there are high amounts of CPU being consumed by the two callstacks I pasted below. It appears that there are multiple threads accessing a HashSet at the same time, causing the HashSet to enter a corrupted state. This causes multiple threads to be stuck in an infinite loop within the HashSet. HashSet is not inherently thread-safe, so having concurrent writers into a HashSet or overlapping reader + a writer can cause issues such as infinite loops, exceptions, and incorrect values. In the case with the callstacks I pasted below, 4 threads were stuck within HashSet.Contains and 1 thread within HashSet.AddIfNotPresent, causing 5 full cores of CPU to be continuously used.

Solution

I would recommend making sure that reads and writes to the HashSet within StaticClassDependencyAnalyze are thread-safe. You can do this either by using a lock for the HashSet, or using a thread-safe collection such as a ConcurrentDictionary or ConcurrentBag.

CallStack 1

++microsoft.codeanalysis.ni!Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[System.ValueTuple`2[System.Canon,Microsoft.CodeAnalysis.Diagnostics.SyntaxNodeAnalysisContext]](Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer, System.Action`1>, System.ValueTuple`2, System.Nullable`1) ++ microsoft.codeanalysis.csharp.ni!Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor+<>c55`1[Microsoft.CodeAnalysis.CSharp.SyntaxKind].b55_0(System.ValueTuple`2,Microsoft.CodeAnalysis.Diagnostics.SyntaxNodeAnalysisContext>) ++++share.analyzer!ET.Analyzer.StaticClassCircularDependencyAnalyzer+<>c__DisplayClass7_0.b0(value class Microsoft.CodeAnalysis.Diagnostics.SyntaxNodeAnalysisContext) ++++ share.analyzer!StaticClassCircularDependencyAnalyzer.StaticClassDependencyAnalyze ++++++system.core.ni!System.Collections.Generic.HashSet`1[System.Canon].Contains(System.Canon)

CallStack 2

++microsoft.codeanalysis.ni!Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock[System.ValueTuple`2[System.Canon,Microsoft.CodeAnalysis.Diagnostics.SyntaxNodeAnalysisContext]](Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer, System.Action`1>, System.ValueTuple`2, System.Nullable`1) ++ microsoft.codeanalysis.csharp.ni!Microsoft.CodeAnalysis.Diagnostics.AnalyzerExecutor+<>c55`1[Microsoft.CodeAnalysis.CSharp.SyntaxKind].b55_0(System.ValueTuple`2,Microsoft.CodeAnalysis.Diagnostics.SyntaxNodeAnalysisContext>) ++++share.analyzer!ET.Analyzer.StaticClassCircularDependencyAnalyzer+<>c__DisplayClass7_0.b0(value class Microsoft.CodeAnalysis.Diagnostics.SyntaxNodeAnalysisContext) ++++ share.analyzer!StaticClassCircularDependencyAnalyzer.StaticClassDependencyAnalyze ++++++system.core.ni!System.Collections.Generic.HashSet`1[System.Canon].AddIfNotPresent(System.Canon)

egametang commented 1 year ago

OK, thx!

BoysheO commented 3 months ago

Is fixed?