diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs
index d8b41d831de6a..a9406c77825ff 100644
--- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs
+++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs
@@ -271,8 +271,27 @@ public bool InitializeIsUnscopedRef(bool value)
}
///
- /// Holds infrequently accessed fields. See for an explanation.
+ /// This type is used to hold lazily-initialized fields that many methods will not need. We avoid creating it unless one of the fields is needed;
+ /// unfortunately, this means that we need to be careful of data races. The general pattern that we use is to check for a flag in .
+ /// If the flag for that field is set, and there was a positive result (ie, there are indeed custom attributes, or there is obsolete data), then it
+ /// is safe to rely on the data in the field. If the flag for a field is set but the result is empty (ie, there is no obsolete data), then we can be in
+ /// one of 3 scenarios:
+ ///
+ /// - is itself null. In this case, no race has occurred, and the consuming code can safely handle the lack of
+ /// however it chooses.
+ /// - is not null, and the backing field has been initialized to some empty value, such as
+ /// . In this case, again, no race has occurred, and the consuming code can simply trust the empty value.
+ /// - is not null, and the backing field is uninitialized, either being , or is some
+ /// kind of sentinel value. In this case, a data race has occurred, and the consuming code must initialize the field to empty to bring it back
+ /// into scenario 2.
+ ///
///
+ ///
+ /// The initialization pattern for this type must follow the following pattern to make the safety guarantees above:
+ /// If the field initialization code determines that the backing field needs to be set to some non-empty value, it must first call ,
+ /// set the backing field using an atomic operation, and then set the flag in . This ensures that the field is always set before the flag is set.
+ /// If this order is reversed, the consuming code may see the flag set, but the field not initialized, and incorrectly assume that there is no data.
+ ///
private sealed class UncommonFields
{
public ParameterSymbol _lazyThisParameter;
@@ -289,64 +308,64 @@ private sealed class UncommonFields
public MethodSymbol _lazyExplicitClassOverride;
}
- private UncommonFields CreateUncommonFields()
+ private UncommonFields AccessUncommonFields()
{
- var retVal = new UncommonFields();
- if (!_packedFlags.IsObsoleteAttributePopulated)
- {
- retVal._lazyObsoleteAttributeData = ObsoleteAttributeData.Uninitialized;
- }
+ var retVal = _uncommonFields;
+ return retVal ?? InterlockedOperations.Initialize(ref _uncommonFields, createUncommonFields());
- if (!_packedFlags.IsUnmanagedCallersOnlyAttributePopulated)
+ UncommonFields createUncommonFields()
{
- retVal._lazyUnmanagedCallersOnlyAttributeData = UnmanagedCallersOnlyAttributeData.Uninitialized;
- }
-
- //
- // Do not set _lazyUseSiteDiagnostic !!!!
- //
- // "null" Indicates "no errors" or "unknown state",
- // and we know which one of the states we have from IsUseSiteDiagnosticPopulated
- //
- // Setting _lazyUseSiteDiagnostic to a sentinel value here would introduce
- // a number of extra states for various permutations of IsUseSiteDiagnosticPopulated, UncommonFields and _lazyUseSiteDiagnostic
- // Some of them, in tight races, may lead to returning the sentinel as the diagnostics.
- //
+ var retVal = new UncommonFields();
+ if (!_packedFlags.IsObsoleteAttributePopulated)
+ {
+ retVal._lazyObsoleteAttributeData = ObsoleteAttributeData.Uninitialized;
+ }
- if (_packedFlags.IsCustomAttributesPopulated)
- {
- retVal._lazyCustomAttributes = ImmutableArray.Empty;
- }
+ if (!_packedFlags.IsUnmanagedCallersOnlyAttributePopulated)
+ {
+ retVal._lazyUnmanagedCallersOnlyAttributeData = UnmanagedCallersOnlyAttributeData.Uninitialized;
+ }
- if (_packedFlags.IsConditionalPopulated)
- {
- retVal._lazyConditionalAttributeSymbols = ImmutableArray.Empty;
- }
+ //
+ // Do not set _lazyUseSiteDiagnostic !!!!
+ //
+ // "null" Indicates "no errors" or "unknown state",
+ // and we know which one of the states we have from IsUseSiteDiagnosticPopulated
+ //
+ // Setting _lazyUseSiteDiagnostic to a sentinel value here would introduce
+ // a number of extra states for various permutations of IsUseSiteDiagnosticPopulated, UncommonFields and _lazyUseSiteDiagnostic
+ // Some of them, in tight races, may lead to returning the sentinel as the diagnostics.
+ //
+
+ if (_packedFlags.IsCustomAttributesPopulated)
+ {
+ retVal._lazyCustomAttributes = ImmutableArray.Empty;
+ }
- if (_packedFlags.IsOverriddenOrHiddenMembersPopulated)
- {
- retVal._lazyOverriddenOrHiddenMembersResult = OverriddenOrHiddenMembersResult.Empty;
- }
+ if (_packedFlags.IsConditionalPopulated)
+ {
+ retVal._lazyConditionalAttributeSymbols = ImmutableArray.Empty;
+ }
- if (_packedFlags.IsMemberNotNullPopulated)
- {
- retVal._lazyNotNullMembers = ImmutableArray.Empty;
- retVal._lazyNotNullMembersWhenTrue = ImmutableArray.Empty;
- retVal._lazyNotNullMembersWhenFalse = ImmutableArray.Empty;
- }
+ if (_packedFlags.IsOverriddenOrHiddenMembersPopulated)
+ {
+ retVal._lazyOverriddenOrHiddenMembersResult = OverriddenOrHiddenMembersResult.Empty;
+ }
- if (_packedFlags.IsExplicitOverrideIsPopulated)
- {
- retVal._lazyExplicitClassOverride = null;
- }
+ if (_packedFlags.IsMemberNotNullPopulated)
+ {
+ retVal._lazyNotNullMembers = ImmutableArray.Empty;
+ retVal._lazyNotNullMembersWhenTrue = ImmutableArray.Empty;
+ retVal._lazyNotNullMembersWhenFalse = ImmutableArray.Empty;
+ }
- return retVal;
- }
+ if (_packedFlags.IsExplicitOverrideIsPopulated)
+ {
+ retVal._lazyExplicitClassOverride = null;
+ }
- private UncommonFields AccessUncommonFields()
- {
- var retVal = _uncommonFields;
- return retVal ?? InterlockedOperations.Initialize(ref _uncommonFields, CreateUncommonFields());
+ return retVal;
+ }
}
private readonly MethodDefinitionHandle _handle;
diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs
index 9151ef8c181a6..e278301f2a4c1 100644
--- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs
+++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs
@@ -143,6 +143,28 @@ public void SetCustomAttributesPopulated()
public readonly bool IsCustomAttributesPopulated => (_bits & IsCustomAttributesPopulatedBit) != 0;
}
+ ///
+ /// This type is used to hold lazily-initialized fields that many properties will not need. We avoid creating it unless one of the fields is needed;
+ /// unfortunately, this means that we need to be careful of data races. The general pattern that we use is to check for a flag in .
+ /// If the flag for that field is set, and there was a positive result (ie, there are indeed custom attributes, or there is obsolete data), then it
+ /// is safe to rely on the data in the field. If the flag for a field is set but the result is empty (ie, there is no obsolete data), then we can be in
+ /// one of 3 scenarios:
+ ///
+ /// - is itself null. In this case, no race has occurred, and the consuming code can safely handle the lack of
+ /// however it chooses.
+ /// - is not null, and the backing field has been initialized to some empty value, such as
+ /// . In this case, again, no race has occurred, and the consuming code can simply trust the empty value.
+ /// - is not null, and the backing field is uninitialized, either being , or is some
+ /// kind of sentinel value. In this case, a data race has occurred, and the consuming code must initialize the field to empty to bring it back
+ /// into scenario 2.
+ ///
+ ///
+ ///
+ /// The initialization pattern for this type must follow the following pattern to make the safety guarantees above:
+ /// If the field initialization code determines that the backing field needs to be set to some non-empty value, it must first call ,
+ /// set the backing field using an atomic operation, and then set the flag in . This ensures that the field is always set before the flag is set.
+ /// If this order is reversed, the consuming code may see the flag set, but the field not initialized, and incorrectly assume that there is no data.
+ ///
private sealed class UncommonFields
{
public ImmutableArray _lazyCustomAttributes;
@@ -312,31 +334,31 @@ static bool anyUnexpectedRequiredModifiers(ParamInfo[] propertyParam
}
}
- private UncommonFields CreateUncommonFields()
+ private UncommonFields AccessUncommonFields()
{
- var retVal = new UncommonFields();
- if (!_flags.IsObsoleteAttributePopulated)
- {
- retVal._lazyObsoleteAttributeData = ObsoleteAttributeData.Uninitialized;
- }
+ var retVal = _uncommonFields;
+ return retVal ?? InterlockedOperations.Initialize(ref _uncommonFields, createUncommonFields());
- if (!_flags.IsUseSiteDiagnosticPopulated)
+ UncommonFields createUncommonFields()
{
- retVal._lazyCachedUseSiteInfo = CachedUseSiteInfo.Uninitialized;
- }
+ var retVal = new UncommonFields();
+ if (!_flags.IsObsoleteAttributePopulated)
+ {
+ retVal._lazyObsoleteAttributeData = ObsoleteAttributeData.Uninitialized;
+ }
- if (_flags.IsCustomAttributesPopulated)
- {
- retVal._lazyCustomAttributes = ImmutableArray.Empty;
- }
+ if (!_flags.IsUseSiteDiagnosticPopulated)
+ {
+ retVal._lazyCachedUseSiteInfo = CachedUseSiteInfo.Uninitialized;
+ }
- return retVal;
- }
+ if (_flags.IsCustomAttributesPopulated)
+ {
+ retVal._lazyCustomAttributes = ImmutableArray.Empty;
+ }
- private UncommonFields AccessUncommonFields()
- {
- var retVal = _uncommonFields;
- return retVal ?? InterlockedOperations.Initialize(ref _uncommonFields, CreateUncommonFields());
+ return retVal;
+ }
}
private bool MustCallMethodsDirectlyCore()