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

Fix ArgumentNullException under a race condition in analyzer driver #65083

Merged
merged 9 commits into from
Nov 12, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,24 @@ private static void FreeState_NoLock<TAnalyzerStateData>(TAnalyzerStateData? sta
{
if (state != null && !ReferenceEquals(state, AnalyzerStateData.FullyProcessedInstance))
{
// Do not free state if it is still being processed for analysis, i.e. StateKind is 'InProcess'.
// This can happen in rare cases when multiple threads are trying to analyze the same
// symbol/syntax/operation, such that one thread has finished all the analyzer callbacks,
// but before this thread marks the state as complete and invokes this FreeState call,
// second thread starts processing the same symbol/syntax/operation and sets the StateKind to
// 'InProcess' for the same state object. If we free the state here in the first thread,
// then this can lead to data corruption/exceptions in the second thread.
// For example, see https://github.com/dotnet/roslyn/issues/59988.
//
// CONSIDER: Note that our current approach of not freeing the state for StateKind.InProcess
// leads to a leak, as we will never return this state object to the pool. However, this should
// happen in extremely rare cases as described above, so it should be fine. In future, if we do
// see negative performance impact from this leak, we can do a more complex state tracking and
// ensure that we free this state object and return it to the pool once the StateKind is reset
// to StateKind.ReadyToProcess state.
if (state.StateKind == StateKind.InProcess)
return;

state.Free();
pool.Free(state);
}
Expand Down