Fix race in SourceParameterSymbol

This type, and it's children, had the same race condition around
attributes that `SourceEventSymbol` did. After some thought I decided to
move this code back into `SourceCompletionState` with a new name to make
it clear the completion is around attributes only.

This is a follow up to #29807
上级 6805b2e5
......@@ -67,30 +67,7 @@ internal sealed override bool HasComplete(CompletionPart part)
internal override void ForceComplete(SourceLocation locationOpt, CancellationToken cancellationToken)
{
if (!_state.HasComplete(CompletionPart.Attributes))
{
_ = GetAttributes();
// Consider the following items:
// 1. It is possible for parallel calls to GetAttributes to exist
// 2. GetAttributes will return when the attributes are available, not when the part is noted
// as complete.
// 3. The thread which actually completes the attributes is the one which must set the CompletionParts.Attributes
// value.
// 4. This call cannot correctly return until this part is set.
//
// That is why it is necessary to check this value again.
//
// Note: #2 above is common practice amongst all of the symbols.
//
// Note: #3 above is an invariant that has existed in the code for some time. It's not clear if this invariant
// is 100% correct. After inspection though it seems likely to be correct as the code is asserting that
// SymbolDeclaredEvent is raised before CompletionPart.Attributes is noted as completed. Also this is a common
// pattern amongst the GetAttributes implementations.
_state.SpinWaitComplete(CompletionPart.Attributes, cancellationToken);
}
_state.NotePartComplete(CompletionPart.All);
_state.ForceCompleteAttributesOnly(this, cancellationToken);
}
public override abstract string Name { get; }
......
......@@ -158,7 +158,7 @@ internal sealed override bool HasComplete(CompletionPart part)
internal override void ForceComplete(SourceLocation locationOpt, CancellationToken cancellationToken)
{
state.DefaultForceComplete(this);
state.ForceCompleteAttributesOnly(this, cancellationToken);
}
/// <summary>
......
......@@ -34,16 +34,35 @@ internal int IncompleteParts
}
/// <summary>
/// Used to force (source) symbols to a given state of completion.
/// Used to force (source) symbols to a given state of completion when the only potential remaining
/// part is attributes. This does force the invariant on the caller that the implementation of
/// of <see cref="Symbol.GetAttributes"/> will set the part <see cref="CompletionPart.Attributes"/> on
/// the thread that actually completes the loading of attributes. Failure to do so will potentially
/// result in a deadlock.
/// </summary>
/// <param name="symbol">The owning source symbol.</param>
internal void DefaultForceComplete(Symbol symbol)
internal void ForceCompleteAttributesOnly(Symbol symbol, CancellationToken cancellationToken)
{
Debug.Assert(symbol.RequiresCompletion);
if (!HasComplete(CompletionPart.Attributes))
{
symbol.GetAttributes();
_ = symbol.GetAttributes();
// Consider the following items:
// 1. It is possible for parallel calls to GetAttributes to exist
// 2. GetAttributes implementation can validly return when the attributes are available but before the
// CompletionParts.Attributes value is set.
// 3. GetAttributes implementation typically have the invariant that the thread which completes the
// loading of attributes is the one which sets CompletionParts.Attributes.
// 4. This call cannot correctly return until CompletionParts.Attributes is set.
//
// Note: #2 above is common practice amongst all of the symbols.
//
// Note: #3 above is an invariant that has existed in the code base for some time. It's not 100% clear
// whether this invariant is tied to correctness or not. The most compelling example though is
// SourceEventSymbol which raises SymbolDeclaredEvent before CompletionPart.Attributes is noted as completed.
// Many other implementations have this pattern but no apparent code which could dependd on it.
SpinWaitComplete(CompletionPart.Attributes, cancellationToken);
}
// any other values are completion parts intended for other kinds of symbols
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册