diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs index 0ced88e5f8efb5e40f68035845e5f32b84859324..2924fcb534c35dbc586e5e13769eb47a6800b4c6 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs @@ -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; } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceParameterSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceParameterSymbol.cs index b88d04b888b7f071839f8076901ae99f985d8d59..5d97fe325ca01216282506eda0b56f6c33188c1c 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceParameterSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceParameterSymbol.cs @@ -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); } /// diff --git a/src/Compilers/CSharp/Portable/Symbols/SymbolCompletionState.cs b/src/Compilers/CSharp/Portable/Symbols/SymbolCompletionState.cs index cd7fdc67d772a7268c0483ec9fe10d939c2e065f..b6c4692bf5c771f1df0c2196627f17bdba03e138 100644 --- a/src/Compilers/CSharp/Portable/Symbols/SymbolCompletionState.cs +++ b/src/Compilers/CSharp/Portable/Symbols/SymbolCompletionState.cs @@ -34,16 +34,35 @@ internal int IncompleteParts } /// - /// 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 will set the part on + /// the thread that actually completes the loading of attributes. Failure to do so will potentially + /// result in a deadlock. /// /// The owning source symbol. - 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