From 32d0360b73bd77256cc9a9314a3c4280a61ea9bc Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Mon, 4 Jul 2022 12:42:45 +0100 Subject: [PATCH 1/5] Refactor JsonPropertyInfo initialization infrastructure and implement JsonPropertyInfo.AttributeProvider (#71514) * Implement JsonPropertyInfo.AttributeProvider and refactor initialization infrastructure * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs * _shouldSerializeIsExplicitlySet set to false when setting IgnoreCondition * Reinstante _converterIsExternalAndPolymorphic * Move unique attribute resolution logic to reflection extensions * address feedback --- .../src/System/ReflectionExtensions.cs | 19 ++ .../Text/Json/Serialization/JsonConverter.cs | 2 +- .../Serialization/JsonConverterFactory.cs | 2 +- .../Json/Serialization/JsonConverterOfT.cs | 4 +- .../JsonSerializerOptions.Converters.cs | 69 ++---- .../Metadata/CustomJsonTypeInfoOfT.cs | 6 +- .../Metadata/DefaultJsonTypeInfoResolver.cs | 2 +- .../Metadata/JsonMetadataServices.cs | 4 +- .../Metadata/JsonPropertyInfo.cs | 215 ++++++++++++------ .../Metadata/JsonPropertyInfoOfT.cs | 155 ++++--------- .../Metadata/JsonTypeInfo.Cache.cs | 51 +---- .../Serialization/Metadata/JsonTypeInfo.cs | 125 ++++------ .../Metadata/ReflectionJsonTypeInfoOfT.cs | 121 +++------- .../Metadata/SourceGenJsonTypeInfoOfT.cs | 15 +- .../Text/Json/Serialization/WriteStack.cs | 2 +- .../Text/Json/ThrowHelper.Serialization.cs | 38 ++-- 16 files changed, 337 insertions(+), 493 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs b/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs index 006c9c4e0e3abb..6cc6dbbf91e70c 100644 --- a/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs +++ b/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs @@ -43,5 +43,24 @@ public static bool IsAssignableFromInternal(this Type type, Type from) private static bool HasJsonConstructorAttribute(ConstructorInfo constructorInfo) => constructorInfo.GetCustomAttribute() != null; + + public static TAttribute? GetUniqueCustomAttribute(this MemberInfo memberInfo, bool inherit) + where TAttribute : Attribute + { + object[] attributes = memberInfo.GetCustomAttributes(typeof(TAttribute), inherit); + + if (attributes.Length == 0) + { + return null; + } + + if (attributes.Length == 1) + { + return (TAttribute)attributes[0]; + } + + ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateAttribute(typeof(TAttribute), memberInfo); + return null; + } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs index bfff00aa3e39a4..571eb1459d8d09 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs @@ -67,7 +67,7 @@ internal virtual void ReadElementAndSetProperty( throw new InvalidOperationException(SR.NodeJsonObjectCustomConverterNotAllowedOnExtensionProperty); } - internal abstract JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo parentTypeInfo); + internal abstract JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, Type propertyType, JsonSerializerOptions options); internal abstract JsonParameterInfo CreateJsonParameterInfo(); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs index dcb8c950111089..0db82dbb610292 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs @@ -32,7 +32,7 @@ protected JsonConverterFactory() { } /// public abstract JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options); - internal override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo parentTypeInfo) + internal override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, Type propertyType, JsonSerializerOptions options) { Debug.Fail("We should never get here."); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs index a4dee4e3ff90a8..a6c7c5baae034d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs @@ -69,9 +69,9 @@ public override bool CanConvert(Type typeToConvert) internal override ConverterStrategy ConverterStrategy => ConverterStrategy.Value; - internal sealed override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo parentTypeInfo) + internal sealed override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, Type propertyType, JsonSerializerOptions options) { - return new JsonPropertyInfo(parentTypeInfo); + return new JsonPropertyInfo(declaringTypeInfo.Type, propertyType, declaringTypeInfo, options); } internal sealed override JsonParameterInfo CreateJsonParameterInfo() diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index 2dd8adf20952d3..bdf26c36dfb9cd 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -27,21 +27,15 @@ public sealed partial class JsonSerializerOptions public IList Converters => _converters; // This may return factory converter - internal JsonConverter? GetCustomConverterFromMember(Type? parentClassType, Type typeToConvert, MemberInfo? memberInfo) + internal JsonConverter? GetCustomConverterFromMember(Type typeToConvert, MemberInfo memberInfo) { + Debug.Assert(memberInfo.DeclaringType != null, "Properties and fields always have a declaring type."); JsonConverter? converter = null; - if (memberInfo != null) + JsonConverterAttribute? converterAttribute = memberInfo.GetUniqueCustomAttribute(inherit: false); + if (converterAttribute != null) { - Debug.Assert(parentClassType != null); - - JsonConverterAttribute? converterAttribute = (JsonConverterAttribute?) - GetAttributeThatCanHaveMultiple(parentClassType!, typeof(JsonConverterAttribute), memberInfo); - - if (converterAttribute != null) - { - converter = GetConverterFromAttribute(converterAttribute, typeToConvert, classTypeAttributeIsOn: parentClassType!, memberInfo); - } + converter = GetConverterFromAttribute(converterAttribute, typeToConvert, memberInfo); } return converter; @@ -180,12 +174,10 @@ private JsonConverter GetConverterFromOptionsOrReflectionConverter(Type typeToCo // Priority 2: Attempt to get converter from [JsonConverter] on the type being converted. if (converter == null) { - JsonConverterAttribute? converterAttribute = (JsonConverterAttribute?) - GetAttributeThatCanHaveMultiple(typeToConvert, typeof(JsonConverterAttribute)); - + JsonConverterAttribute? converterAttribute = typeToConvert.GetUniqueCustomAttribute(inherit: false); if (converterAttribute != null) { - converter = GetConverterFromAttribute(converterAttribute, typeToConvert: typeToConvert, classTypeAttributeIsOn: typeToConvert, memberInfo: null); + converter = GetConverterFromAttribute(converterAttribute, typeToConvert: typeToConvert, memberInfo: null); } } @@ -215,29 +207,30 @@ private JsonConverter GetConverterFromOptionsOrReflectionConverter(Type typeToCo // This suppression needs to be removed. https://github.com/dotnet/runtime/issues/68878 [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode", Justification = "The factory constructors are only invoked in the context of reflection serialization code paths " + "and are marked RequiresDynamicCode")] - private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, Type classTypeAttributeIsOn, MemberInfo? memberInfo) + private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, MemberInfo? memberInfo) { JsonConverter? converter; - Type? type = converterAttribute.ConverterType; - if (type == null) + Type declaringType = memberInfo?.DeclaringType ?? typeToConvert; + Type? converterType = converterAttribute.ConverterType; + if (converterType == null) { // Allow the attribute to create the converter. converter = converterAttribute.CreateConverter(typeToConvert); if (converter == null) { - ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, memberInfo, typeToConvert); + ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(declaringType, memberInfo, typeToConvert); } } else { - ConstructorInfo? ctor = type.GetConstructor(Type.EmptyTypes); - if (!typeof(JsonConverter).IsAssignableFrom(type) || ctor == null || !ctor.IsPublic) + ConstructorInfo? ctor = converterType.GetConstructor(Type.EmptyTypes); + if (!typeof(JsonConverter).IsAssignableFrom(converterType) || ctor == null || !ctor.IsPublic) { - ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeInvalid(classTypeAttributeIsOn, memberInfo); + ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeInvalid(declaringType, memberInfo); } - converter = (JsonConverter)Activator.CreateInstance(type)!; + converter = (JsonConverter)Activator.CreateInstance(converterType)!; } Debug.Assert(converter != null); @@ -255,38 +248,10 @@ private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converter return NullableConverterFactory.CreateValueConverter(underlyingType, converter); } - ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, memberInfo, typeToConvert); + ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(declaringType, memberInfo, typeToConvert); } return converter; } - - private static Attribute? GetAttributeThatCanHaveMultiple(Type classType, Type attributeType, MemberInfo memberInfo) - { - object[] attributes = memberInfo.GetCustomAttributes(attributeType, inherit: false); - return GetAttributeThatCanHaveMultiple(attributeType, classType, memberInfo, attributes); - } - - internal static Attribute? GetAttributeThatCanHaveMultiple(Type classType, Type attributeType) - { - object[] attributes = classType.GetCustomAttributes(attributeType, inherit: false); - return GetAttributeThatCanHaveMultiple(attributeType, classType, null, attributes); - } - - private static Attribute? GetAttributeThatCanHaveMultiple(Type attributeType, Type classType, MemberInfo? memberInfo, object[] attributes) - { - if (attributes.Length == 0) - { - return null; - } - - if (attributes.Length == 1) - { - return (Attribute)attributes[0]; - } - - ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateAttribute(attributeType, classType, memberInfo); - return default; - } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/CustomJsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/CustomJsonTypeInfoOfT.cs index 874815923eacb2..b8cc6c1661be3d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/CustomJsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/CustomJsonTypeInfoOfT.cs @@ -31,11 +31,7 @@ internal CustomJsonTypeInfo(JsonSerializerOptions options) private static JsonConverter GetConverter(JsonSerializerOptions options) { DefaultJsonTypeInfoResolver.RootDefaultInstance(); - return GetEffectiveConverter( - typeof(T), - parentClassType: null, // A TypeInfo never has a "parent" class. - memberInfo: null, // A TypeInfo never has a "parent" property. - options); + return options.GetConverterForType(typeof(T)); } internal override JsonParameterInfoValues[] GetParameterInfoValues() diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs index d0baa8e2e18dc9..817a6f4feb6102 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs @@ -50,7 +50,7 @@ public virtual JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options _mutable = false; - JsonTypeInfo.ValidateType(type, null, null, options); + JsonTypeInfo.ValidateType(type); JsonTypeInfo typeInfo = CreateJsonTypeInfo(type, options); if (_modifiers != null) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs index a60ec2fe2d5084..308e1841a0a59e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs @@ -48,9 +48,7 @@ public static JsonPropertyInfo CreatePropertyInfo(JsonSerializerOptions optio throw new InvalidOperationException(SR.Format(SR.FieldCannotBeVirtual, nameof(propertyInfo.IsProperty), nameof(propertyInfo.IsVirtual))); } - JsonPropertyInfo jsonPropertyInfo = new JsonPropertyInfo(parentTypeInfo: null); - jsonPropertyInfo.InitializeForSourceGen(options, propertyInfo); - return jsonPropertyInfo; + return new JsonPropertyInfo(options, propertyInfo); } /// diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs index c9a77e5db5a89e..00c59fe5ea4f69 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; -using System.ComponentModel; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Reflection; @@ -100,24 +99,95 @@ public JsonConverter? CustomConverter _shouldSerialize = value; // By default we will go through faster path (not using delegate) and use IgnoreCondition // If users sets it explicitly we always go through delegate - IgnoreCondition = null; + _ignoreCondition = null; IsIgnored = false; _shouldSerializeIsExplicitlySet = true; } } - private protected Func? _shouldSerialize; + internal JsonIgnoreCondition? IgnoreCondition + { + get => _ignoreCondition; + set + { + Debug.Assert(!_isConfigured); + + _ignoreCondition = value; + IsIgnored = value == JsonIgnoreCondition.Always; + _shouldSerialize = value != null ? GetShouldSerializeForIgnoreCondition(value.Value) : null; + _shouldSerializeIsExplicitlySet = false; + } + } + + private Func? _shouldSerialize; private bool _shouldSerializeIsExplicitlySet; + private JsonIgnoreCondition? _ignoreCondition; + + private protected abstract Func GetShouldSerializeForIgnoreCondition(JsonIgnoreCondition condition); + + /// + /// Gets or sets a custom attribute provider for the current property. + /// + /// + /// When resolving metadata via this + /// will be populated with the underlying of the serialized property or field. + /// + /// Setting a custom attribute provider will have no impact on the contract model, + /// but serves as metadata for downstream contract modifiers. + /// + internal ICustomAttributeProvider? AttributeProvider + { + get => _attributeProvider; + set + { + VerifyMutable(); + + _attributeProvider = value; + } + } + + private ICustomAttributeProvider? _attributeProvider; + internal string? MemberName { get; private protected set; } + internal MemberTypes MemberType { get; private protected set; } + internal bool IsVirtual { get; private set; } + + /// + /// Specifies whether the current property is a special extension data property. + /// + /// + /// Properties annotated with + /// will appear here when using or . + /// + internal bool IsExtensionData + { + get => _isExtensionDataProperty; + set + { + VerifyMutable(); + + if (value && !JsonTypeInfo.IsValidExtensionDataProperty(this)) + { + ThrowHelper.ThrowInvalidOperationException_SerializationDataExtensionPropertyInvalid(this); + } + + _isExtensionDataProperty = value; + } + } + + private bool _isExtensionDataProperty; - internal JsonPropertyInfo(JsonTypeInfo? parentTypeInfo) + internal JsonPropertyInfo(Type declaringType, Type propertyType, JsonTypeInfo? parentTypeInfo, JsonSerializerOptions options) { - // null parentTypeInfo means it's not tied yet - ParentTypeInfo = parentTypeInfo; + DeclaringType = declaringType; + PropertyType = propertyType; + Options = options; + ParentTypeInfo = parentTypeInfo; // null parentTypeInfo means it's not tied yet + PropertyTypeCanBeNull = PropertyType.CanBeNull(); } internal static JsonPropertyInfo GetPropertyPlaceholder() { - JsonPropertyInfo info = new JsonPropertyInfo(parentTypeInfo: null); + JsonPropertyInfo info = new JsonPropertyInfo(typeof(object), typeof(object), parentTypeInfo: null, options: null!); Debug.Assert(!info.IsForTypeInfo); Debug.Assert(!info.CanDeserialize); @@ -131,7 +201,7 @@ internal static JsonPropertyInfo GetPropertyPlaceholder() /// /// Gets the type of the current property metadata. /// - public Type PropertyType { get; private protected set; } = null!; + public Type PropertyType { get; } private protected void VerifyMutable() { @@ -190,54 +260,41 @@ internal virtual void Configure() } } - internal abstract void DetermineEffectiveConverter(); + private protected abstract void DetermineEffectiveConverter(); + private protected abstract void DetermineMemberAccessors(MemberInfo memberInfo); - internal void GetPolicies() + private void DeterminePoliciesFromMember(MemberInfo memberInfo) { - Debug.Assert(MemberInfo != null); - DeterminePropertyName(); + JsonPropertyOrderAttribute? orderAttr = memberInfo.GetCustomAttribute(inherit: false); + Order = orderAttr?.Order ?? 0; - JsonPropertyOrderAttribute? orderAttr = MemberInfo.GetCustomAttribute(inherit: false); - if (orderAttr != null) - { - Order = orderAttr.Order; - } - - JsonNumberHandlingAttribute? attribute = MemberInfo.GetCustomAttribute(inherit: false); - NumberHandling = attribute?.Handling; + JsonNumberHandlingAttribute? numberHandlingAttr = memberInfo.GetCustomAttribute(inherit: false); + NumberHandling = numberHandlingAttr?.Handling; } - private void DeterminePropertyName() + private void DeterminePropertyNameFromMember(MemberInfo memberInfo) { - Debug.Assert(MemberInfo != null); - - ClrName = MemberInfo.Name; - - JsonPropertyNameAttribute? nameAttribute = MemberInfo.GetCustomAttribute(inherit: false); + JsonPropertyNameAttribute? nameAttribute = memberInfo.GetCustomAttribute(inherit: false); + string? name; if (nameAttribute != null) { - string name = nameAttribute.Name; - if (name == null) - { - ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameNull(DeclaringType, this); - } - - Name = name; + name = nameAttribute.Name; } else if (Options.PropertyNamingPolicy != null) { - string name = Options.PropertyNamingPolicy.ConvertName(MemberInfo.Name); - if (name == null) - { - ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameNull(DeclaringType, this); - } - - Name = name; + name = Options.PropertyNamingPolicy.ConvertName(memberInfo.Name); } else { - Name = MemberInfo.Name; + name = memberInfo.Name; } + + if (name == null) + { + ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameNull(this); + } + + Name = name; } internal void CacheNameAsUtf8BytesAndEscapedNameSection() @@ -330,7 +387,7 @@ internal void DetermineIgnoreCondition(JsonIgnoreCondition? ignoreCondition) } else { - ThrowHelper.ThrowInvalidOperationException_IgnoreConditionOnValueTypeInvalid(ClrName!, DeclaringType); + ThrowHelper.ThrowInvalidOperationException_IgnoreConditionOnValueTypeInvalid(MemberName!, DeclaringType); } } } @@ -468,17 +525,40 @@ internal string GetDebugInfo(int indent = 0) internal bool HasGetter => _untypedGet is not null; internal bool HasSetter => _untypedSet is not null; - internal abstract void Initialize( - Type parentClassType, - Type declaredPropertyType, - ConverterStrategy converterStrategy, - MemberInfo? memberInfo, - bool isVirtual, - JsonConverter converter, - JsonIgnoreCondition? ignoreCondition, - JsonSerializerOptions options, - JsonTypeInfo? jsonTypeInfo = null, - bool isUserDefinedProperty = false); + internal void InitializeUsingMemberReflection(MemberInfo memberInfo) + { + Debug.Assert(AttributeProvider == null); + + switch (AttributeProvider = memberInfo) + { + case PropertyInfo propertyInfo: + { + MemberName = propertyInfo.Name; + IsVirtual = propertyInfo.IsVirtual(); + MemberType = MemberTypes.Property; + break; + } + case FieldInfo fieldInfo: + { + MemberName = fieldInfo.Name; + MemberType = MemberTypes.Field; + break; + } + default: + Debug.Fail("Only FieldInfo and PropertyInfo members are supported."); + break; + } + + DeterminePoliciesFromMember(memberInfo); + DeterminePropertyNameFromMember(memberInfo); + + if (!IsIgnored) + { + DetermineMemberAccessors(memberInfo); + } + + IsExtensionData = memberInfo.GetCustomAttribute(inherit: false) != null; + } internal bool IgnoreDefaultValuesOnRead { get; private set; } internal bool IgnoreDefaultValuesOnWrite { get; private set; } @@ -532,7 +612,7 @@ public string Name /// /// Options associated with JsonPropertyInfo /// - public JsonSerializerOptions Options { get; internal set; } = null!; // initialized in Init method + public JsonSerializerOptions Options { get; } /// /// Gets or sets the serialization order for the current property. @@ -650,9 +730,7 @@ internal void EnsureChildOf(JsonTypeInfo parent) } } - internal Type DeclaringType { get; set; } = null!; - - internal MemberInfo? MemberInfo { get; set; } + internal Type DeclaringType { get; } [AllowNull] internal JsonTypeInfo JsonTypeInfo @@ -685,22 +763,17 @@ internal JsonTypeInfo JsonTypeInfo internal abstract void SetExtensionDictionaryAsObject(object obj, object? extensionDict); - internal bool CanSerialize { get; set; } + internal bool CanSerialize { get; private set; } - internal bool CanDeserialize { get; set; } + internal bool CanDeserialize { get; private set; } - internal bool IsIgnored { get; set; } + internal bool IsIgnored { get; private set; } /// /// Relevant to source generated metadata: did the property have the ? /// internal bool SrcGen_HasJsonInclude { get; set; } - /// - /// Relevant to source generated metadata: did the property have the ? - /// - internal bool SrcGen_IsExtensionData { get; set; } - /// /// Relevant to source generated metadata: is the property public? /// @@ -732,15 +805,7 @@ public JsonNumberHandling? NumberHandling internal JsonNumberHandling? EffectiveNumberHandling { get; set; } // Whether the property type can be null. - internal bool PropertyTypeCanBeNull { get; set; } - - internal JsonIgnoreCondition? IgnoreCondition { get; set; } - - internal MemberTypes MemberType { get; set; } // TODO: with some refactoring, we should be able to remove this. - - internal string? ClrName { get; set; } - - internal bool IsVirtual { get; set; } + internal bool PropertyTypeCanBeNull { get; } /// /// Default value used for parameterized ctor invocation. @@ -748,6 +813,6 @@ public JsonNumberHandling? NumberHandling internal abstract object? DefaultValue { get; } [DebuggerBrowsable(DebuggerBrowsableState.Never)] - private string DebuggerDisplay => $"MemberInfo={MemberInfo}"; + private string DebuggerDisplay => $"MemberInfo={AttributeProvider as MemberInfo}"; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs index 31c9ca3aa28f29..8176763ce7bd33 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs @@ -25,13 +25,15 @@ internal sealed class JsonPropertyInfo : JsonPropertyInfo // Since a converter's TypeToConvert (which is the T value in this type) can be different than // the property's type, we track that and whether the property type can be null. - private bool _propertyTypeEqualsTypeToConvert; + private readonly bool _propertyTypeEqualsTypeToConvert; private Func? _typedGet; private Action? _typedSet; - internal JsonPropertyInfo(JsonTypeInfo? parentTypeInfo) : base(parentTypeInfo) + internal JsonPropertyInfo(Type declaringType, Type propertyType, JsonTypeInfo? parentTypeInfo, JsonSerializerOptions options) + : base(declaringType, propertyType, parentTypeInfo, options) { + _propertyTypeEqualsTypeToConvert = propertyType == typeof(T); } internal new Func? Get @@ -98,107 +100,57 @@ private protected override void SetSetter(Delegate? setter) internal JsonConverter TypedEffectiveConverter { get; private set; } = null!; - internal override void Initialize( - Type declaringType, - Type declaredPropertyType, - ConverterStrategy converterStrategy, - MemberInfo? memberInfo, - bool isVirtual, - JsonConverter converter, - JsonIgnoreCondition? ignoreCondition, - JsonSerializerOptions options, - JsonTypeInfo? jsonTypeInfo = null, - bool isUserDefinedProperty = false) + private protected override void DetermineMemberAccessors(MemberInfo memberInfo) { - Debug.Assert(converter != null); + Debug.Assert(memberInfo is FieldInfo or PropertyInfo); - PropertyType = declaredPropertyType; - _propertyTypeEqualsTypeToConvert = typeof(T) == declaredPropertyType; - PropertyTypeCanBeNull = PropertyType.CanBeNull(); - ConverterStrategy = converterStrategy; - if (jsonTypeInfo != null) + switch (memberInfo) { - JsonTypeInfo = jsonTypeInfo; - } - - DefaultConverterForType = converter; - Options = options; - DeclaringType = declaringType; - MemberInfo = memberInfo; - IsVirtual = isVirtual; - IgnoreCondition = ignoreCondition; - IsIgnored = ignoreCondition == JsonIgnoreCondition.Always; - - if (memberInfo != null) - { - if (!IsIgnored) - { - switch (memberInfo) + case PropertyInfo propertyInfo: { - case PropertyInfo propertyInfo: - { - bool useNonPublicAccessors = propertyInfo.GetCustomAttribute(inherit: false) != null; - - MethodInfo? getMethod = propertyInfo.GetMethod; - if (getMethod != null && (getMethod.IsPublic || useNonPublicAccessors)) - { - Get = options.MemberAccessorStrategy.CreatePropertyGetter(propertyInfo); - } - - MethodInfo? setMethod = propertyInfo.SetMethod; - if (setMethod != null && (setMethod.IsPublic || useNonPublicAccessors)) - { - Set = options.MemberAccessorStrategy.CreatePropertySetter(propertyInfo); - } + bool useNonPublicAccessors = propertyInfo.GetCustomAttribute(inherit: false) != null; - MemberType = MemberTypes.Property; - - break; - } + MethodInfo? getMethod = propertyInfo.GetMethod; + if (getMethod != null && (getMethod.IsPublic || useNonPublicAccessors)) + { + Get = Options.MemberAccessorStrategy.CreatePropertyGetter(propertyInfo); + } - case FieldInfo fieldInfo: - { - Debug.Assert(fieldInfo.IsPublic); + MethodInfo? setMethod = propertyInfo.SetMethod; + if (setMethod != null && (setMethod.IsPublic || useNonPublicAccessors)) + { + Set = Options.MemberAccessorStrategy.CreatePropertySetter(propertyInfo); + } - Get = options.MemberAccessorStrategy.CreateFieldGetter(fieldInfo); + break; + } - if (!fieldInfo.IsInitOnly) - { - Set = options.MemberAccessorStrategy.CreateFieldSetter(fieldInfo); - } + case FieldInfo fieldInfo: + { + Debug.Assert(fieldInfo.IsPublic); - MemberType = MemberTypes.Field; + Get = Options.MemberAccessorStrategy.CreateFieldGetter(fieldInfo); - break; - } + if (!fieldInfo.IsInitOnly) + { + Set = Options.MemberAccessorStrategy.CreateFieldSetter(fieldInfo); + } - default: - { - Debug.Fail($"Invalid memberInfo type: {memberInfo.GetType().FullName}"); - break; - } + break; } - } - GetPolicies(); - } - else if (!isUserDefinedProperty) - { - IsForTypeInfo = true; - } - - if (IgnoreCondition != null) - { - _shouldSerialize = GetShouldSerializeForIgnoreCondition(IgnoreCondition.Value); + default: + { + Debug.Fail($"Invalid MemberInfo type: {memberInfo.MemberType}"); + break; + } } } - internal void InitializeForSourceGen(JsonSerializerOptions options, JsonPropertyInfoValues propertyInfo) + internal JsonPropertyInfo(JsonSerializerOptions options, JsonPropertyInfoValues propertyInfo) + : this(propertyInfo.DeclaringType, propertyType: typeof(T), parentTypeInfo: null, options) { - Options = options; - ClrName = propertyInfo.PropertyName; - - string name; + string? name; // Property name settings. if (propertyInfo.JsonPropertyName != null) @@ -207,30 +159,27 @@ internal void InitializeForSourceGen(JsonSerializerOptions options, JsonProperty } else if (options.PropertyNamingPolicy == null) { - name = ClrName; + name = propertyInfo.PropertyName; } else { - name = options.PropertyNamingPolicy.ConvertName(ClrName); + name = options.PropertyNamingPolicy.ConvertName(propertyInfo.PropertyName); } // Compat: We need to do validation before we assign Name so that we get InvalidOperationException rather than ArgumentNullException if (name == null) { - ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameNull(DeclaringType, this); + ThrowHelper.ThrowInvalidOperationException_SerializerPropertyNameNull(this); } Name = name; - + MemberName = propertyInfo.PropertyName; + MemberType = propertyInfo.IsProperty ? MemberTypes.Property : MemberTypes.Field; SrcGen_IsPublic = propertyInfo.IsPublic; SrcGen_HasJsonInclude = propertyInfo.HasJsonInclude; - SrcGen_IsExtensionData = propertyInfo.IsExtensionData; - PropertyType = typeof(T); - _propertyTypeEqualsTypeToConvert = true; - PropertyTypeCanBeNull = PropertyType.CanBeNull(); + IsExtensionData = propertyInfo.IsExtensionData; JsonTypeInfo? propertyTypeInfo = propertyInfo.PropertyTypeInfo; - Type declaringType = propertyInfo.DeclaringType; JsonConverter? typedCustomConverter = propertyInfo.Converter; CustomConverter = typedCustomConverter; @@ -238,23 +187,15 @@ internal void InitializeForSourceGen(JsonSerializerOptions options, JsonProperty JsonConverter? typedNonCustomConverter = propertyTypeInfo?.Converter as JsonConverter; DefaultConverterForType = typedNonCustomConverter; - IsIgnored = propertyInfo.IgnoreCondition == JsonIgnoreCondition.Always; - if (!IsIgnored) + IgnoreCondition = propertyInfo.IgnoreCondition; + if (IgnoreCondition != JsonIgnoreCondition.Always) { Get = propertyInfo.Getter!; Set = propertyInfo.Setter; } JsonTypeInfo = propertyTypeInfo; - DeclaringType = declaringType; - IgnoreCondition = propertyInfo.IgnoreCondition; - MemberType = propertyInfo.IsProperty ? MemberTypes.Property : MemberTypes.Field; NumberHandling = propertyInfo.NumberHandling; - - if (IgnoreCondition != null) - { - _shouldSerialize = GetShouldSerializeForIgnoreCondition(IgnoreCondition.Value); - } } internal override void Configure() @@ -267,7 +208,7 @@ internal override void Configure() } } - internal override void DetermineEffectiveConverter() + private protected override void DetermineEffectiveConverter() { JsonConverter? customConverter = CustomConverter; if (customConverter != null) @@ -544,7 +485,7 @@ internal override void SetExtensionDictionaryAsObject(object obj, object? extens Set!(obj, typedValue); } - private Func GetShouldSerializeForIgnoreCondition(JsonIgnoreCondition ignoreCondition) + private protected override Func GetShouldSerializeForIgnoreCondition(JsonIgnoreCondition ignoreCondition) { switch (ignoreCondition) { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs index 3d99324b5e0aee..32703dc5c8f3c6 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs @@ -51,35 +51,12 @@ public abstract partial class JsonTypeInfo internal Func? CtorParamInitFunc; - internal JsonPropertyInfo CreateProperty( - Type declaredPropertyType, - MemberInfo? memberInfo, - Type parentClassType, - bool isVirtual, - JsonConverter converter, - JsonSerializerOptions options, - JsonIgnoreCondition? ignoreCondition = null, - JsonTypeInfo? jsonTypeInfo = null, - JsonConverter? customConverter = null, - bool isUserDefinedProperty = false) + internal JsonPropertyInfo CreateProperty(Type propertyType, JsonConverter converter) { // Create the JsonPropertyInfo instance. - JsonPropertyInfo jsonPropertyInfo = converter.CreateJsonPropertyInfo(parentTypeInfo: this); - - jsonPropertyInfo.Initialize( - parentClassType, - declaredPropertyType, - converterStrategy: converter.ConverterStrategy, - memberInfo, - isVirtual, - converter, - ignoreCondition, - options, - jsonTypeInfo, - isUserDefinedProperty: isUserDefinedProperty); - - jsonPropertyInfo.CustomConverter = customConverter; - + JsonPropertyInfo jsonPropertyInfo = converter.CreateJsonPropertyInfo(declaringTypeInfo: this, propertyType, Options); + jsonPropertyInfo.DefaultConverterForType = converter; + jsonPropertyInfo.ConverterStrategy = converter.ConverterStrategy; return jsonPropertyInfo; } @@ -87,23 +64,11 @@ internal JsonPropertyInfo CreateProperty( /// Create a for a given Type. /// See . /// - private JsonPropertyInfo CreatePropertyInfoForTypeInfo( - Type declaredPropertyType, - JsonConverter converter, - JsonSerializerOptions options, - JsonTypeInfo? jsonTypeInfo = null) + private JsonPropertyInfo CreatePropertyInfoForTypeInfo(JsonTypeInfo jsonTypeInfo, JsonConverter converter) { - JsonPropertyInfo jsonPropertyInfo = CreateProperty( - declaredPropertyType: declaredPropertyType, - memberInfo: null, // Not a real property so this is null. - parentClassType: ObjectType, // a dummy value (not used) - isVirtual: false, - converter: converter, - options: options, - jsonTypeInfo: jsonTypeInfo); - - Debug.Assert(jsonPropertyInfo.IsForTypeInfo); - + JsonPropertyInfo jsonPropertyInfo = CreateProperty(jsonTypeInfo.Type, converter: converter); + jsonPropertyInfo.JsonTypeInfo = jsonTypeInfo; + jsonPropertyInfo.IsForTypeInfo = true; return jsonPropertyInfo; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs index db5982be7c15f3..5defed1620aafa 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs @@ -194,30 +194,7 @@ public JsonPolymorphismOptions? PolymorphismOptions // Add method delegate for non-generic Stack and Queue; and types that derive from them. internal object? AddMethodDelegate { get; set; } - /// - /// Gets or sets an extension data property for the current type. - /// - /// - /// Properties annotated with - /// will appears here when using or . - /// - internal JsonPropertyInfo? ExtensionDataProperty - { - get => _extensionDataProperty; - set - { - VerifyMutable(); - - if (value != null && !IsValidDataExtensionProperty(value)) - { - ThrowHelper.ThrowInvalidOperationException_SerializationDataExtensionPropertyInvalid(Type, value); - } - - _extensionDataProperty = value; - } - } - - private JsonPropertyInfo? _extensionDataProperty; + internal JsonPropertyInfo? ExtensionDataProperty { get; private set; } internal PolymorphicTypeResolver? PolymorphicTypeResolver { get; private set; } @@ -225,11 +202,11 @@ internal JsonPropertyInfo? ExtensionDataProperty private JsonTypeInfo? _elementTypeInfo; // Avoids having to perform an expensive cast to JsonTypeInfo to check if there is a Serialize method. - internal bool HasSerialize { get; set; } + internal bool HasSerialize { get; private protected set; } // Configure would normally have thrown why initializing properties for source gen but type had SerializeHandler // so it is allowed to be used for serialization but it will throw if used for deserialization - internal bool ThrowOnDeserialize { get; set; } + internal bool ThrowOnDeserialize { get; private protected set; } [MethodImpl(MethodImplOptions.AggressiveInlining)] internal void ValidateCanBeUsedForDeserialization() @@ -275,7 +252,7 @@ internal JsonTypeInfo? ElementTypeInfo } } - internal Type? ElementType { get; set; } + internal Type? ElementType { get; } // If dictionary, the JsonTypeInfo for the key type. private JsonTypeInfo? _keyTypeInfo; @@ -317,17 +294,17 @@ internal JsonTypeInfo? KeyTypeInfo } } - internal Type? KeyType { get; set; } + internal Type? KeyType { get; } /// /// Gets the instance associated with . /// - public JsonSerializerOptions Options { get; private set; } + public JsonSerializerOptions Options { get; } /// /// Type associated with JsonTypeInfo /// - public Type Type { get; private set; } + public Type Type { get; } /// /// Converter associated with the type for the given options instance @@ -390,7 +367,7 @@ internal JsonTypeInfo(Type type, JsonConverter converter, JsonSerializerOptions { Type = type; Options = options; - PropertyInfoForTypeInfo = CreatePropertyInfoForTypeInfo(Type, converter, Options, this); + PropertyInfoForTypeInfo = CreatePropertyInfoForTypeInfo(this, converter); ElementType = converter.ElementType; switch (PropertyInfoForTypeInfo.ConverterStrategy) @@ -494,7 +471,7 @@ internal virtual void Configure() { PropertyCache ??= CreatePropertyCache(capacity: 0); - foreach (var jsonPropertyInfoKv in PropertyCache.List) + foreach (KeyValuePair jsonPropertyInfoKv in PropertyCache.List) { JsonPropertyInfo jsonPropertyInfo = jsonPropertyInfoKv.Value; @@ -629,21 +606,8 @@ public JsonPropertyInfo CreateJsonPropertyInfo(Type propertyType, string name) ThrowHelper.ThrowArgumentException_CannotSerializeInvalidType(nameof(propertyType), propertyType, Type, name); } - JsonConverter converter = GetConverter(propertyType, - parentClassType: null, - memberInfo: null, - options: Options, - out _); - - JsonPropertyInfo propertyInfo = CreateProperty( - declaredPropertyType: propertyType, - memberInfo: null, - parentClassType: Type, - isVirtual: false, - converter: converter, - options: Options, - isUserDefinedProperty: true); - + JsonConverter converter = Options.GetConverterForType(propertyType); + JsonPropertyInfo propertyInfo = CreateProperty(propertyType, converter); propertyInfo.Name = name; return propertyInfo; @@ -651,13 +615,24 @@ public JsonPropertyInfo CreateJsonPropertyInfo(Type propertyType, string name) internal abstract JsonParameterInfoValues[] GetParameterInfoValues(); - internal void CacheMember(JsonPropertyInfo jsonPropertyInfo, JsonPropertyDictionary? propertyCache, ref Dictionary? ignoredMembers) + internal void CacheMember(JsonPropertyInfo jsonPropertyInfo, JsonPropertyDictionary propertyCache, ref Dictionary? ignoredMembers) { - Debug.Assert(jsonPropertyInfo.ClrName != null, "ClrName can be null in custom JsonPropertyInfo instances and should never be passed in this method"); - string memberName = jsonPropertyInfo.ClrName; + Debug.Assert(jsonPropertyInfo.MemberName != null, "MemberName can be null in custom JsonPropertyInfo instances and should never be passed in this method"); + string memberName = jsonPropertyInfo.MemberName; + + if (jsonPropertyInfo.IsExtensionData) + { + if (ExtensionDataProperty != null) + { + ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateTypeAttribute(Type, typeof(JsonExtensionDataAttribute)); + } + + ExtensionDataProperty = jsonPropertyInfo; + return; + } // The JsonPropertyNameAttribute or naming policy resulted in a collision. - if (!propertyCache!.TryAdd(jsonPropertyInfo.Name, jsonPropertyInfo)) + if (!propertyCache.TryAdd(jsonPropertyInfo.Name, jsonPropertyInfo)) { JsonPropertyInfo other = propertyCache[jsonPropertyInfo.Name]!; @@ -671,7 +646,7 @@ internal void CacheMember(JsonPropertyInfo jsonPropertyInfo, JsonPropertyDiction !jsonPropertyInfo.IsIgnored && // Is the current property hidden by the previously cached property // (with `new` keyword, or by overriding)? - other.ClrName != memberName && + other.MemberName != memberName && // Was a property with the same CLR name was ignored? That property hid the current property, // thus, if it was ignored, the current property should be ignored too. ignoredMembers?.ContainsKey(memberName) != true) @@ -738,7 +713,7 @@ internal void InitializeConstructorParameters(JsonParameterInfoValues[] jsonPara foreach (KeyValuePair kvp in PropertyCache.List) { JsonPropertyInfo jsonProperty = kvp.Value; - string propertyName = jsonProperty.ClrName ?? jsonProperty.Name; + string propertyName = jsonProperty.MemberName ?? jsonProperty.Name; ParameterLookupKey key = new(propertyName, jsonProperty.PropertyType); ParameterLookupValue value = new(jsonProperty); @@ -777,8 +752,8 @@ internal void InitializeConstructorParameters(JsonParameterInfoValues[] jsonPara else if (ExtensionDataProperty != null && StringComparer.OrdinalIgnoreCase.Equals(paramToCheck.Name, ExtensionDataProperty.Name)) { - Debug.Assert(ExtensionDataProperty.ClrName != null, "Custom property info cannot be data extension property"); - ThrowHelper.ThrowInvalidOperationException_ExtensionDataCannotBindToCtorParam(ExtensionDataProperty.ClrName, ExtensionDataProperty); + Debug.Assert(ExtensionDataProperty.MemberName != null, "Custom property info cannot be data extension property"); + ThrowHelper.ThrowInvalidOperationException_ExtensionDataCannotBindToCtorParam(ExtensionDataProperty.MemberName, ExtensionDataProperty); } } @@ -786,11 +761,11 @@ internal void InitializeConstructorParameters(JsonParameterInfoValues[] jsonPara Volatile.Write(ref ParameterCache, parameterCache); } - internal static void ValidateType(Type type, Type? parentClassType, MemberInfo? memberInfo, JsonSerializerOptions options) + internal static void ValidateType(Type type) { if (IsInvalidForSerialization(type)) { - ThrowHelper.ThrowInvalidOperationException_CannotSerializeInvalidType(type, parentClassType, memberInfo); + ThrowHelper.ThrowInvalidOperationException_CannotSerializeInvalidType(type, declaringType: null, memberInfo: null); } } @@ -823,16 +798,16 @@ private static bool IsByRefLike(Type type) #endif } - internal bool IsValidDataExtensionProperty(JsonPropertyInfo jsonPropertyInfo) + internal static bool IsValidExtensionDataProperty(JsonPropertyInfo jsonPropertyInfo) { Type memberType = jsonPropertyInfo.PropertyType; bool typeIsValid = typeof(IDictionary).IsAssignableFrom(memberType) || typeof(IDictionary).IsAssignableFrom(memberType) || // Avoid a reference to typeof(JsonNode) to support trimming. - (memberType.FullName == JsonObjectTypeName && ReferenceEquals(memberType.Assembly, GetType().Assembly)); + (memberType.FullName == JsonObjectTypeName && ReferenceEquals(memberType.Assembly, typeof(JsonTypeInfo).Assembly)); - return typeIsValid && Options.GetConverterFromTypeInfo(memberType) != null; + return typeIsValid && jsonPropertyInfo.Options.GetConverterFromTypeInfo(memberType) != null; } internal JsonPropertyDictionary CreatePropertyCache(int capacity) @@ -845,30 +820,16 @@ internal JsonPropertyDictionary CreatePropertyCache(int capaci // - class type, // - element type (if the type is a collection), // - the converter (either native or custom), if one exists. - internal static JsonConverter GetConverter( - Type type, - Type? parentClassType, - MemberInfo? memberInfo, + private protected static JsonConverter GetConverterFromMember( + Type typeToConvert, + MemberInfo memberInfo, JsonSerializerOptions options, out JsonConverter? customConverter) { - Debug.Assert(type != null); - Debug.Assert(!IsInvalidForSerialization(type), $"Type `{type.FullName}` should already be validated."); - customConverter = parentClassType != null ? options.GetCustomConverterFromMember(parentClassType, type, memberInfo) : null; - return options.GetConverterForType(type); - } - - internal static JsonConverter GetEffectiveConverter( - Type type, - Type? parentClassType, - MemberInfo? memberInfo, - JsonSerializerOptions options) - { - JsonConverter converter = GetConverter(type, parentClassType, memberInfo, options, out JsonConverter? customConverter); - - customConverter = options.ExpandFactoryConverter(customConverter, type); - - return customConverter ?? converter; + Debug.Assert(typeToConvert != null); + Debug.Assert(!IsInvalidForSerialization(typeToConvert), $"Type `{typeToConvert.FullName}` should already be validated."); + customConverter = options.GetCustomConverterFromMember(typeToConvert, memberInfo); + return options.GetConverterForType(typeToConvert); } private static JsonParameterInfo CreateConstructorParameter( @@ -908,6 +869,6 @@ private static JsonTypeInfoKind GetTypeInfoKind(Type type, ConverterStrategy con } [DebuggerBrowsable(DebuggerBrowsableState.Never)] - private string DebuggerDisplay => $"ConverterStrategy.{PropertyInfoForTypeInfo.ConverterStrategy}, {Type.Name}"; + private string DebuggerDisplay => $"Type:{Type.Name}, TypeInfoKind:{Kind}"; } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs index 2a0d33f6bfea25..22f322478ec604 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; -using System.ComponentModel; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Reflection; @@ -18,13 +17,7 @@ internal sealed class ReflectionJsonTypeInfo : JsonTypeInfo [RequiresUnreferencedCode(JsonSerializer.SerializationUnreferencedCodeMessage)] [RequiresDynamicCode(JsonSerializer.SerializationRequiresDynamicCodeMessage)] internal ReflectionJsonTypeInfo(JsonSerializerOptions options) - : this( - GetEffectiveConverter( - typeof(T), - parentClassType: null, // A TypeInfo never has a "parent" class. - memberInfo: null, // A TypeInfo never has a "parent" property. - options), - options) + : this(options.GetConverterForType(typeof(T)), options) { } @@ -67,34 +60,32 @@ private void AddPropertiesAndParametersUsingReflection() { Debug.Assert(PropertyInfoForTypeInfo.ConverterStrategy == ConverterStrategy.Object); - const BindingFlags bindingFlags = + const BindingFlags BindingFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly; Dictionary? ignoredMembers = null; - PropertyInfo[] properties = Type.GetProperties(bindingFlags); bool propertyOrderSpecified = false; - // PropertyCache is not accessed by other threads until the current JsonTypeInfo instance - // is finished initializing and added to the cache on JsonSerializerOptions. - // Default 'capacity' to the common non-polymorphic + property case. - PropertyCache = CreatePropertyCache(capacity: properties.Length); + // Walk through the inheritance hierarchy, starting from the most derived type upward. + for (Type? currentType = Type; currentType != null; currentType = currentType.BaseType) + { + PropertyInfo[] properties = currentType.GetProperties(BindingFlags); - // We start from the most derived type. - Type? currentType = Type; + // PropertyCache is not accessed by other threads until the current JsonTypeInfo instance + // is finished initializing and added to the cache in JsonSerializerOptions. + // Default 'capacity' to the common non-polymorphic + property case. + PropertyCache ??= CreatePropertyCache(capacity: properties.Length); - while (true) - { foreach (PropertyInfo propertyInfo in properties) { - bool isVirtual = propertyInfo.IsVirtual(); string propertyName = propertyInfo.Name; // Ignore indexers and virtual properties that have overrides that were [JsonIgnore]d. if (propertyInfo.GetIndexParameters().Length > 0 || - PropertyIsOverridenAndIgnored(propertyName, propertyInfo.PropertyType, isVirtual, ignoredMembers)) + PropertyIsOverridenAndIgnored(propertyName, propertyInfo.PropertyType, propertyInfo.IsVirtual(), ignoredMembers)) { continue; } @@ -104,11 +95,8 @@ private void AddPropertiesAndParametersUsingReflection() propertyInfo.SetMethod?.IsPublic == true) { CacheMember( - currentType, - propertyInfo.PropertyType, - propertyInfo, - isVirtual, - NumberHandling, + typeToConvert: propertyInfo.PropertyType, + memberInfo: propertyInfo, ref propertyOrderSpecified, ref ignoredMembers); } @@ -123,7 +111,7 @@ private void AddPropertiesAndParametersUsingReflection() } } - foreach (FieldInfo fieldInfo in currentType.GetFields(bindingFlags)) + foreach (FieldInfo fieldInfo in currentType.GetFields(BindingFlags)) { string fieldName = fieldInfo.Name; @@ -139,11 +127,8 @@ private void AddPropertiesAndParametersUsingReflection() if (hasJsonInclude || Options.IncludeFields) { CacheMember( - currentType, - fieldInfo.FieldType, - fieldInfo, - isVirtual: false, - NumberHandling, + typeToConvert: fieldInfo.FieldType, + memberInfo: fieldInfo, ref propertyOrderSpecified, ref ignoredMembers); } @@ -158,15 +143,9 @@ private void AddPropertiesAndParametersUsingReflection() // Non-public fields should not be included for (de)serialization. } } + } - currentType = currentType.BaseType; - if (currentType == null) - { - break; - } - - properties = currentType.GetProperties(bindingFlags); - }; + Debug.Assert(PropertyCache != null); if (propertyOrderSpecified) { @@ -175,21 +154,12 @@ private void AddPropertiesAndParametersUsingReflection() } private void CacheMember( - Type declaringType, - Type memberType, + Type typeToConvert, MemberInfo memberInfo, - bool isVirtual, - JsonNumberHandling? typeNumberHandling, ref bool propertyOrderSpecified, ref Dictionary? ignoredMembers) { - bool hasExtensionAttribute = memberInfo.GetCustomAttribute(inherit: false) != null; - if (hasExtensionAttribute && ExtensionDataProperty != null) - { - ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateTypeAttribute(Type, typeof(JsonExtensionDataAttribute)); - } - - JsonPropertyInfo? jsonPropertyInfo = AddProperty(memberInfo, memberType, declaringType, isVirtual, Options); + JsonPropertyInfo? jsonPropertyInfo = AddProperty(typeToConvert, memberInfo, Options); if (jsonPropertyInfo == null) { // ignored invalid property @@ -197,34 +167,24 @@ private void CacheMember( } Debug.Assert(jsonPropertyInfo.Name != null); - - if (hasExtensionAttribute) - { - Debug.Assert(ExtensionDataProperty == null); - ExtensionDataProperty = jsonPropertyInfo; - } - else - { - CacheMember(jsonPropertyInfo, PropertyCache, ref ignoredMembers); - propertyOrderSpecified |= jsonPropertyInfo.Order != 0; - } + Debug.Assert(PropertyCache != null); + CacheMember(jsonPropertyInfo, PropertyCache, ref ignoredMembers); + propertyOrderSpecified |= jsonPropertyInfo.Order != 0; } private JsonPropertyInfo? AddProperty( + Type typeToConvert, MemberInfo memberInfo, - Type memberType, - Type parentClassType, - bool isVirtual, JsonSerializerOptions options) { JsonIgnoreCondition? ignoreCondition = memberInfo.GetCustomAttribute(inherit: false)?.Condition; - if (IsInvalidForSerialization(memberType)) + if (IsInvalidForSerialization(typeToConvert)) { if (ignoreCondition == JsonIgnoreCondition.Always) return null; - ThrowHelper.ThrowInvalidOperationException_CannotSerializeInvalidType(memberType, parentClassType, memberInfo); + ThrowHelper.ThrowInvalidOperationException_CannotSerializeInvalidType(typeToConvert, memberInfo.DeclaringType, memberInfo); } JsonConverter? customConverter; @@ -232,34 +192,27 @@ private void CacheMember( try { - converter = GetConverter( - memberType, - parentClassType, - memberInfo, - options, - out customConverter); + converter = GetConverterFromMember( + typeToConvert, + memberInfo, + options, + out customConverter); } catch (InvalidOperationException) when (ignoreCondition == JsonIgnoreCondition.Always) { return null; } - return CreateProperty( - declaredPropertyType: memberType, - memberInfo, - parentClassType, - isVirtual, - converter, - options, - ignoreCondition, - customConverter: customConverter); + JsonPropertyInfo jsonPropertyInfo = CreateProperty(typeToConvert, converter); + jsonPropertyInfo.IgnoreCondition = ignoreCondition; + jsonPropertyInfo.CustomConverter = customConverter; + jsonPropertyInfo.InitializeUsingMemberReflection(memberInfo); + return jsonPropertyInfo; } private static JsonNumberHandling? GetNumberHandlingForType(Type type) { - var numberHandlingAttribute = - (JsonNumberHandlingAttribute?)JsonSerializerOptions.GetAttributeThatCanHaveMultiple(type, typeof(JsonNumberHandlingAttribute)); - + JsonNumberHandlingAttribute? numberHandlingAttribute = type.GetUniqueCustomAttribute(inherit: false); return numberHandlingAttribute?.Handling; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs index f50063c670318b..c981e0bc8e2d08 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs @@ -151,8 +151,8 @@ internal void AddPropertiesUsingSourceGenInfo() { if (hasJsonInclude) { - Debug.Assert(jsonPropertyInfo.ClrName != null, "ClrName is not set by source gen"); - ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(jsonPropertyInfo.ClrName, jsonPropertyInfo.DeclaringType); + Debug.Assert(jsonPropertyInfo.MemberName != null, "MemberName is not set by source gen"); + ThrowHelper.ThrowInvalidOperationException_JsonIncludeOnNonPublicInvalid(jsonPropertyInfo.MemberName, jsonPropertyInfo.DeclaringType); } continue; @@ -163,17 +163,6 @@ internal void AddPropertiesUsingSourceGenInfo() continue; } - if (jsonPropertyInfo.SrcGen_IsExtensionData) - { - if (ExtensionDataProperty != null) - { - ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateTypeAttribute(Type, typeof(JsonExtensionDataAttribute)); - } - - ExtensionDataProperty = jsonPropertyInfo; - continue; - } - CacheMember(jsonPropertyInfo, propertyCache, ref ignoredMembers); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs index 1366bda342f97b..b6f448100d07b6 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs @@ -407,7 +407,7 @@ public string PropertyPath() static void AppendStackFrame(StringBuilder sb, ref WriteStackFrame frame) { // Append the property name. - string? propertyName = frame.JsonPropertyInfo?.ClrName; + string? propertyName = frame.JsonPropertyInfo?.MemberName; if (propertyName == null) { // Attempt to get the JSON property name from the property name specified in re-entry. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs index 025605f098e15c..b65ab991d88979 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs @@ -86,29 +86,29 @@ public static void ThrowJsonException(string? message = null) } [DoesNotReturn] - public static void ThrowArgumentException_CannotSerializeInvalidType(string paramName, Type type, Type? parentClassType, string? propertyName) + public static void ThrowArgumentException_CannotSerializeInvalidType(string paramName, Type typeToConvert, Type? declaringType, string? propertyName) { - if (parentClassType == null) + if (declaringType == null) { Debug.Assert(propertyName == null); - throw new ArgumentException(SR.Format(SR.CannotSerializeInvalidType, type), paramName); + throw new ArgumentException(SR.Format(SR.CannotSerializeInvalidType, typeToConvert), paramName); } Debug.Assert(propertyName != null); - throw new ArgumentException(SR.Format(SR.CannotSerializeInvalidMember, type, propertyName, parentClassType), paramName); + throw new ArgumentException(SR.Format(SR.CannotSerializeInvalidMember, typeToConvert, propertyName, declaringType), paramName); } [DoesNotReturn] - public static void ThrowInvalidOperationException_CannotSerializeInvalidType(Type type, Type? parentClassType, MemberInfo? memberInfo) + public static void ThrowInvalidOperationException_CannotSerializeInvalidType(Type typeToConvert, Type? declaringType, MemberInfo? memberInfo) { - if (parentClassType == null) + if (declaringType == null) { Debug.Assert(memberInfo == null); - throw new InvalidOperationException(SR.Format(SR.CannotSerializeInvalidType, type)); + throw new InvalidOperationException(SR.Format(SR.CannotSerializeInvalidType, typeToConvert)); } Debug.Assert(memberInfo != null); - throw new InvalidOperationException(SR.Format(SR.CannotSerializeInvalidMember, type, memberInfo.Name, parentClassType)); + throw new InvalidOperationException(SR.Format(SR.CannotSerializeInvalidMember, typeToConvert, memberInfo.Name, declaringType)); } [DoesNotReturn] @@ -201,9 +201,9 @@ public static void ThrowInvalidOperationException_SerializerPropertyNameConflict } [DoesNotReturn] - public static void ThrowInvalidOperationException_SerializerPropertyNameNull(Type parentType, JsonPropertyInfo jsonPropertyInfo) + public static void ThrowInvalidOperationException_SerializerPropertyNameNull(JsonPropertyInfo jsonPropertyInfo) { - throw new InvalidOperationException(SR.Format(SR.SerializerPropertyNameNull, parentType, jsonPropertyInfo.MemberInfo?.Name)); + throw new InvalidOperationException(SR.Format(SR.SerializerPropertyNameNull, jsonPropertyInfo.DeclaringType, jsonPropertyInfo.MemberName)); } [DoesNotReturn] @@ -267,11 +267,8 @@ public static void ThrowInvalidOperationException_IgnoreConditionOnValueTypeInva [DoesNotReturn] public static void ThrowInvalidOperationException_NumberHandlingOnPropertyInvalid(JsonPropertyInfo jsonPropertyInfo) { - MemberInfo? memberInfo = jsonPropertyInfo.MemberInfo; - Debug.Assert(memberInfo != null); Debug.Assert(!jsonPropertyInfo.IsForTypeInfo); - - throw new InvalidOperationException(SR.Format(SR.NumberHandlingOnPropertyInvalid, memberInfo.Name, memberInfo.DeclaringType)); + throw new InvalidOperationException(SR.Format(SR.NumberHandlingOnPropertyInvalid, jsonPropertyInfo.MemberName, jsonPropertyInfo.DeclaringType)); } [DoesNotReturn] @@ -401,14 +398,9 @@ public static void AddJsonExceptionInformation(ref WriteStack state, JsonExcepti } [DoesNotReturn] - public static void ThrowInvalidOperationException_SerializationDuplicateAttribute(Type attribute, Type classType, MemberInfo? memberInfo) + public static void ThrowInvalidOperationException_SerializationDuplicateAttribute(Type attribute, MemberInfo memberInfo) { - string location = classType.ToString(); - if (memberInfo != null) - { - location += $".{memberInfo.Name}"; - } - + string location = memberInfo is Type type ? type.ToString() : $"{memberInfo.DeclaringType}.{memberInfo.Name}"; throw new InvalidOperationException(SR.Format(SR.SerializationDuplicateAttribute, attribute, location)); } @@ -425,9 +417,9 @@ public static void ThrowInvalidOperationException_SerializationDuplicateTypeAttr } [DoesNotReturn] - public static void ThrowInvalidOperationException_SerializationDataExtensionPropertyInvalid(Type type, JsonPropertyInfo jsonPropertyInfo) + public static void ThrowInvalidOperationException_SerializationDataExtensionPropertyInvalid(JsonPropertyInfo jsonPropertyInfo) { - throw new InvalidOperationException(SR.Format(SR.SerializationDataExtensionPropertyInvalid, type, jsonPropertyInfo.MemberInfo?.Name)); + throw new InvalidOperationException(SR.Format(SR.SerializationDataExtensionPropertyInvalid, jsonPropertyInfo.PropertyType, jsonPropertyInfo.MemberName)); } [DoesNotReturn] From 210a5072bdf860e0f4b67df5624a6d1c82d0ec18 Mon Sep 17 00:00:00 2001 From: Omair Majid Date: Mon, 4 Jul 2022 14:29:12 -0400 Subject: [PATCH 2/5] Add Ubuntu 22.04 to libraries CI run (#67345) This is to help catch OpenSSL 3.0 issues. --- eng/pipelines/libraries/helix-queues-setup.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/eng/pipelines/libraries/helix-queues-setup.yml b/eng/pipelines/libraries/helix-queues-setup.yml index 2c7897748380f8..6e7c8c386ad2a0 100644 --- a/eng/pipelines/libraries/helix-queues-setup.yml +++ b/eng/pipelines/libraries/helix-queues-setup.yml @@ -39,7 +39,7 @@ jobs: # Linux arm64 - ${{ if eq(parameters.platform, 'Linux_arm64') }}: - ${{ if or(eq(parameters.jobParameters.isExtraPlatforms, true), eq(parameters.jobParameters.includeAllPlatforms, true)) }}: - - (Ubuntu.2110.Arm64.Open)Ubuntu.1804.ArmArch.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-21.10-helix-arm64v8-20211116135000-0f8d97e + - (Ubuntu.2204.Arm64.Open)Ubuntu.1804.ArmArch.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-22.04-helix-arm64v8-20220504035342-1b9461f - ${{ if or(ne(parameters.jobParameters.isExtraPlatforms, true), eq(parameters.jobParameters.includeAllPlatforms, true)) }}: - (Ubuntu.1804.ArmArch.Open)Ubuntu.1804.ArmArch.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-18.04-helix-arm64v8-20220427172132-97d8652 @@ -63,14 +63,14 @@ jobs: - RedHat.7.Amd64.Open - SLES.15.Amd64.Open - (Fedora.34.Amd64.Open)Ubuntu.1804.Amd64.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:fedora-34-helix-20220523142223-4f64125 - - (Ubuntu.2110.Amd64.Open)Ubuntu.1804.Amd64.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-21.10-helix-amd64-20211116135132-0f8d97e + - (Ubuntu.2204.Amd64.Open)Ubuntu.1804.Amd64.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-22.04-helix-amd64-20220504035722-1b9461f - (Debian.10.Amd64.Open)Ubuntu.1804.Amd64.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:debian-10-helix-amd64-bfcd90a-20200121150006 - ${{ if or(ne(parameters.jobParameters.testScope, 'outerloop'), ne(parameters.jobParameters.runtimeFlavor, 'mono')) }}: - ${{ if or(eq(parameters.jobParameters.isExtraPlatforms, true), eq(parameters.jobParameters.includeAllPlatforms, true)) }}: - (Centos.8.Amd64.Open)Ubuntu.1604.Amd64.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:centos-8-helix-20201229003624-c1bf759 - SLES.15.Amd64.Open - (Fedora.34.Amd64.Open)ubuntu.1604.amd64.open@mcr.microsoft.com/dotnet-buildtools/prereqs:fedora-34-helix-20220523142223-4f64125 - - (Ubuntu.2110.Amd64.Open)ubuntu.1604.amd64.open@mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-21.04-helix-amd64-20210922170909-34a2d72 + - (Ubuntu.2204.Amd64.Open)Ubuntu.1804.Amd64.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-22.04-helix-amd64-20220504035722-1b9461f - (Debian.11.Amd64.Open)Ubuntu.1804.Amd64.Open@mcr.microsoft.com/dotnet-buildtools/prereqs:debian-11-helix-amd64-20210304164428-5a7c380 - (Mariner.1.0.Amd64.Open)ubuntu.1604.amd64.open@mcr.microsoft.com/dotnet-buildtools/prereqs:cbl-mariner-1.0-helix-20210528192219-92bf620 - (openSUSE.15.2.Amd64.Open)ubuntu.1604.amd64.open@mcr.microsoft.com/dotnet-buildtools/prereqs:opensuse-15.2-helix-amd64-20211018152525-9cc02fe From 15f67a2ed84e6a0ee7f388c8ce77654f892f3c0f Mon Sep 17 00:00:00 2001 From: Johan Lorensson Date: Mon, 4 Jul 2022 20:52:31 +0200 Subject: [PATCH 3/5] Fix EventPipe profiler recursion in class_loading under full AOT. (#71610) Happens in full AOT mode where methods have been compiled as direct calls (happens within an assembly). In that case, the method is not known by the runtime (not present in the JIT info table). When a new class is loading, the class_loading profiler callback is called. The EventPipe implementation of that callback will take a stackwalk (inline with CoreCLR behavior), but taking a stackwalk will look up each managed function on the callstack against the JIT info table, if its not there it will load it, that in turn could case additional recursive class_loading event to fire, that will again take the same callstack, ending up in an infinite recursion. The issue is even a little more problematic, since it turns out you cannot do anything inside the class_loading callback that might end up triggering a class load, since that might lead up to recursively loading the same type, that in turn will assert when outer class_loading call returns and tries to add it into the loaded class cache. Fix will harden the stackwalking logic inside EventPipe to detect recursive stackwalks (doing a stackwalk triggering another stackwalk) and if that is the case, the stackwalk will be done using the async_context stackwalking capabilities since that won't trigger any additional profiler events, breaking recursion. Fix also adds logic to the class_loading profiler callback in EventPipe to prevent recursion making sure it won't trigger any additional profiler events, leading up to potential issues loading the same class twice, that will cause an assert inside the class loader logic. For 1:1 mapping of old Mono profiler events issued into EventPipe, they have been aligned with old Mono log profiler events callstack behaviours, meaning it will only emit callstacks on a few limited profiler events, mitigating the issues seen in the runtime profiler provider. --- src/coreclr/vm/ClrEtwAllMeta.lst | 34 ++++++++++- src/mono/mono/eventpipe/ep-rt-mono.c | 87 ++++++++++++++++++++++++++-- 2 files changed, 116 insertions(+), 5 deletions(-) diff --git a/src/coreclr/vm/ClrEtwAllMeta.lst b/src/coreclr/vm/ClrEtwAllMeta.lst index 2fae1ae9af5934..1d5c92cbce24b5 100644 --- a/src/coreclr/vm/ClrEtwAllMeta.lst +++ b/src/coreclr/vm/ClrEtwAllMeta.lst @@ -640,7 +640,37 @@ nomac:CLRStackStress:::CLRStackWalkStress # Events from the Mono profiler provider ################################# noclrinstanceid:MonoProfiler::: -nostack:MonoProfiler:::MonoProfilerExceptionClause +nostack:MonoProfiler:::MonoProfilerContextLoaded +nostack:MonoProfiler:::MonoProfilerContextUnloaded +nostack:MonoProfiler:::MonoProfilerAppDomainLoading +nostack:MonoProfiler:::MonoProfilerAppDomainLoaded +nostack:MonoProfiler:::MonoProfilerAppDomainUnloading +nostack:MonoProfiler:::MonoProfilerAppDomainUnloaded +nostack:MonoProfiler:::MonoProfilerAppDomainName +nostack:MonoProfiler:::MonoProfilerJitBegin +nostack:MonoProfiler:::MonoProfilerJitFailed +nostack:MonoProfiler:::MonoProfilerJitDone +nostack:MonoProfiler:::MonoProfilerJitDone_V1 +nostack:MonoProfiler:::MonoProfilerJitChunkCreated +nostack:MonoProfiler:::MonoProfilerJitChunkDestroyed +nostack:MonoProfiler:::MonoProfilerJitCodeBuffer +nostack:MonoProfiler:::MonoProfilerClassLoading +nostack:MonoProfiler:::MonoProfilerClassFailed +nostack:MonoProfiler:::MonoProfilerClassLoaded +nostack:MonoProfiler:::MonoProfilerClassLoaded_V1 +nostack:MonoProfiler:::MonoProfilerVTableLoading +nostack:MonoProfiler:::MonoProfilerVTableFailed +nostack:MonoProfiler:::MonoProfilerVTableLoaded +nostack:MonoProfiler:::MonoProfilerModuleLoading +nostack:MonoProfiler:::MonoProfilerModuleFailed +nostack:MonoProfiler:::MonoProfilerModuleLoaded +nostack:MonoProfiler:::MonoProfilerModuleUnloading +nostack:MonoProfiler:::MonoProfilerModuleUnloaded +nostack:MonoProfiler:::MonoProfilerAssemblyLoading +nostack:MonoProfiler:::MonoProfilerAssemblyFailed +nostack:MonoProfiler:::MonoProfilerAssemblyLoaded +nostack:MonoProfiler:::MonoProfilerAssemblyUnloading +nostack:MonoProfiler:::MonoProfilerAssemblyUnloaded nostack:MonoProfiler:::MonoProfilerMethodEnter nostack:MonoProfiler:::MonoProfilerMethodLeave nostack:MonoProfiler:::MonoProfilerMethodTailCall @@ -648,6 +678,7 @@ nostack:MonoProfiler:::MonoProfilerMethodExceptionLeave nostack:MonoProfiler:::MonoProfilerMethodFree nostack:MonoProfiler:::MonoProfilerMethodBeginInvoke nostack:MonoProfiler:::MonoProfilerMethodEndInvoke +nostack:MonoProfiler:::MonoProfilerExceptionClause nostack:MonoProfiler:::MonoProfilerGCEvent nostack:MonoProfiler:::MonoProfilerGCMoves nostack:MonoProfiler:::MonoProfilerGCResize @@ -666,4 +697,5 @@ nostack:MonoProfiler:::MonoProfilerThreadStopping nostack:MonoProfiler:::MonoProfilerThreadStopped nostack:MonoProfiler:::MonoProfilerThreadExited nostack:MonoProfiler:::MonoProfilerThreadName +nostack:MonoProfiler:::MonoProfilerJitDoneVerbose nostack:MonoProfiler:::MonoProfilerGCHeapDumpVTableClassReference \ No newline at end of file diff --git a/src/mono/mono/eventpipe/ep-rt-mono.c b/src/mono/mono/eventpipe/ep-rt-mono.c index 9e9cd70dece9d3..1d7e5ac5b13579 100644 --- a/src/mono/mono/eventpipe/ep-rt-mono.c +++ b/src/mono/mono/eventpipe/ep-rt-mono.c @@ -38,6 +38,7 @@ gboolean _ep_rt_mono_initialized; // EventPipe TLS key. MonoNativeTlsKey _ep_rt_mono_thread_holder_tls_id; +MonoNativeTlsKey _ep_rt_mono_thread_data_tls_id; // Random byte provider. gpointer _ep_rt_mono_rand_provider; @@ -53,6 +54,11 @@ char *_ep_rt_mono_os_cmd_line = NULL; mono_lazy_init_t _ep_rt_mono_managed_cmd_line_init = MONO_LAZY_INIT_STATUS_NOT_INITIALIZED; char *_ep_rt_mono_managed_cmd_line = NULL; +// Custom Mono EventPipe thread data. +typedef struct _EventPipeThreadData EventPipeThreadData; +struct _EventPipeThreadData { + bool prevent_profiler_event_recursion; +}; // Sample profiler. static GArray * _ep_rt_mono_sampled_thread_callstacks = NULL; @@ -295,6 +301,14 @@ static ep_rt_spin_lock_handle_t _ep_rt_mono_profiler_gc_state_lock = {0}; * Forward declares of all static functions. */ +static +EventPipeThreadData * +eventpipe_thread_data_get_or_create (void); + +static +void +eventpipe_thread_data_free (EventPipeThreadData *thread_data); + static bool fire_method_rundown_events_func ( @@ -1221,6 +1235,26 @@ clr_instance_get_id (void) return 9; } +static +EventPipeThreadData * +eventpipe_thread_data_get_or_create (void) +{ + EventPipeThreadData *thread_data = (EventPipeThreadData *)mono_native_tls_get_value (_ep_rt_mono_thread_data_tls_id); + if (!thread_data) { + thread_data = ep_rt_object_alloc (EventPipeThreadData); + mono_native_tls_set_value (_ep_rt_mono_thread_data_tls_id, thread_data); + } + return thread_data; +} + +static +void +eventpipe_thread_data_free (EventPipeThreadData *thread_data) +{ + ep_return_void_if_nok (thread_data != NULL); + ep_rt_object_free (thread_data); +} + static bool fire_method_rundown_events_func ( @@ -1845,7 +1879,8 @@ profiler_eventpipe_thread_exited ( ep_rt_mono_thread_exited (); } -static bool +static +bool parse_mono_profiler_options (const ep_char8_t *option) { do { @@ -1931,6 +1966,7 @@ void ep_rt_mono_init (void) { mono_native_tls_alloc (&_ep_rt_mono_thread_holder_tls_id, NULL); + mono_native_tls_alloc (&_ep_rt_mono_thread_data_tls_id, NULL); mono_100ns_ticks (); mono_rand_open (); @@ -2335,6 +2371,11 @@ ep_rt_mono_thread_exited (void) if (thread_holder) thread_holder_free_func (thread_holder); mono_native_tls_set_value (_ep_rt_mono_thread_holder_tls_id, NULL); + + EventPipeThreadData *thread_data = (EventPipeThreadData *)mono_native_tls_get_value (_ep_rt_mono_thread_data_tls_id); + if (thread_data) + eventpipe_thread_data_free (thread_data); + mono_native_tls_set_value (_ep_rt_mono_thread_data_tls_id, NULL); } } @@ -2646,11 +2687,32 @@ ep_rt_mono_walk_managed_stack_for_thread ( stack_walk_data.safe_point_frame = false; stack_walk_data.runtime_invoke_frame = false; + bool restore_async_context = FALSE; + bool prevent_profiler_event_recursion = FALSE; + EventPipeThreadData *thread_data = eventpipe_thread_data_get_or_create (); + if (thread_data) { + prevent_profiler_event_recursion = thread_data->prevent_profiler_event_recursion; + if (prevent_profiler_event_recursion && !mono_thread_info_is_async_context ()) { + // Running stackwalk in async context mode is currently the only way to prevent + // unwinder to NOT load additional classes during stackwalk, making it signal unsafe and + // potential triggering uncontrolled recursion in profiler class loading event. + mono_thread_info_set_is_async_context (TRUE); + restore_async_context = TRUE; + } + thread_data->prevent_profiler_event_recursion = TRUE; + } + if (thread == ep_rt_thread_get_handle () && mono_get_eh_callbacks ()->mono_walk_stack_with_ctx) mono_get_eh_callbacks ()->mono_walk_stack_with_ctx (eventpipe_walk_managed_stack_for_thread_func, NULL, MONO_UNWIND_SIGNAL_SAFE, &stack_walk_data); else if (mono_get_eh_callbacks ()->mono_walk_stack_with_state) mono_get_eh_callbacks ()->mono_walk_stack_with_state (eventpipe_walk_managed_stack_for_thread_func, mono_thread_info_get_suspend_state (thread), MONO_UNWIND_SIGNAL_SAFE, &stack_walk_data); + if (thread_data) { + if (restore_async_context) + mono_thread_info_set_is_async_context (FALSE); + thread_data->prevent_profiler_event_recursion = prevent_profiler_event_recursion; + } + return true; } @@ -2715,8 +2777,11 @@ ep_rt_mono_sample_profiler_write_sampling_event_for_threads ( mono_stop_world (MONO_THREAD_INFO_FLAGS_NO_GC); - gboolean async_context = mono_thread_info_is_async_context (); - mono_thread_info_set_is_async_context (TRUE); + bool restore_async_context = FALSE; + if (!mono_thread_info_is_async_context ()) { + mono_thread_info_set_is_async_context (TRUE); + restore_async_context = TRUE; + } // Record all info needed in sample events while runtime is suspended, must be async safe. FOREACH_THREAD_SAFE_EXCLUDE (thread_info, MONO_THREAD_INFO_FLAGS_NO_GC | MONO_THREAD_INFO_FLAGS_NO_SAMPLE) { @@ -2758,7 +2823,9 @@ ep_rt_mono_sample_profiler_write_sampling_event_for_threads ( filtered_thread_count++; } FOREACH_THREAD_SAFE_END - mono_thread_info_set_is_async_context (async_context); + if (restore_async_context) + mono_thread_info_set_is_async_context (FALSE); + mono_restart_world (MONO_THREAD_INFO_FLAGS_NO_GC); // Fire sample event for threads. Must be done after runtime is resumed since it's not async safe. @@ -4504,7 +4571,19 @@ runtime_profiler_class_loading ( MonoProfiler *prof, MonoClass *klass) { + bool prevent_profiler_event_recursion = FALSE; + EventPipeThreadData *thread_data = eventpipe_thread_data_get_or_create (); + if (thread_data) { + // Prevent additional class loading to happen recursively as part of fire TypeLoadStart event. + // Additional class loading can happen as part of capturing callstack for TypeLoadStart event. + prevent_profiler_event_recursion = thread_data->prevent_profiler_event_recursion; + thread_data->prevent_profiler_event_recursion = TRUE; + } + ep_rt_mono_write_event_type_load_start (m_class_get_byval_arg (klass)); + + if (thread_data) + thread_data->prevent_profiler_event_recursion = prevent_profiler_event_recursion; } static From 136dfcd02bb559d02b7bfe352ecdf0a9d11f8e55 Mon Sep 17 00:00:00 2001 From: Stefan Schulze Frielinghaus Date: Mon, 4 Jul 2022 20:56:50 +0200 Subject: [PATCH 4/5] [mono][eventpipe] Write primitive types and UTF-16 strings in little endian order (#68648) * [mono][eventpipe] Add big-endian support. --- src/coreclr/scripts/genEventPipe.py | 79 +++++++++++++-- src/coreclr/scripts/genEventing.py | 7 +- .../vm/eventing/eventpipe/ep-rt-coreclr.h | 74 ++++++++++++++- src/mono/mono/eglib/giconv.c | 95 +++++++++++++++---- src/mono/mono/eglib/glib.h | 6 ++ src/mono/mono/eventpipe/CMakeLists.txt | 4 + src/mono/mono/eventpipe/ds-rt-mono.h | 4 +- src/mono/mono/eventpipe/ep-rt-mono.c | 13 ++- src/mono/mono/eventpipe/ep-rt-mono.h | 78 ++++++++++++++- src/native/eventpipe/ds-dump-protocol.c | 2 +- src/native/eventpipe/ds-eventpipe-protocol.c | 4 +- src/native/eventpipe/ds-process-protocol.c | 17 ++-- src/native/eventpipe/ds-protocol.c | 29 +++--- src/native/eventpipe/ds-types.h | 10 -- src/native/eventpipe/ep-block.c | 62 +++++------- src/native/eventpipe/ep-config.c | 6 +- src/native/eventpipe/ep-event-source.c | 14 +-- src/native/eventpipe/ep-file.c | 14 +-- src/native/eventpipe/ep-metadata-generator.c | 2 + src/native/eventpipe/ep-provider.c | 2 +- src/native/eventpipe/ep-rt.h | 47 ++++++++- src/native/eventpipe/ep-stack-contents.h | 3 +- src/native/eventpipe/ep-stream.c | 38 ++++++-- src/native/eventpipe/ep-stream.h | 36 +++++++ src/native/eventpipe/ep.h | 50 ++++++++++ 25 files changed, 552 insertions(+), 144 deletions(-) diff --git a/src/coreclr/scripts/genEventPipe.py b/src/coreclr/scripts/genEventPipe.py index 2f46735cc43e2c..fa737eaa15360a 100644 --- a/src/coreclr/scripts/genEventPipe.py +++ b/src/coreclr/scripts/genEventPipe.py @@ -239,6 +239,20 @@ def generateClrEventPipeWriteEventsImpl( def generateWriteEventBody(template, providerName, eventName, runtimeFlavor): + def winTypeToFixedWidthType(t): + return {'win:Int8': 'int8_t', + 'win:UInt8': 'uint8_t', + 'win:Int16': 'int16_t', + 'win:UInt16': 'uint16_t', + 'win:Int32': 'int32_t', + 'win:UInt32': 'uint32_t', + 'win:Int64': 'int64_t', + 'win:UInt64': 'uint64_t', + 'win:Pointer': 'uintptr_t', + 'win:AnsiString': 'UTF8String', + 'win:UnicodeString': 'UTF16String' + }[t] + fnSig = template.signature pack_list = [] @@ -267,9 +281,33 @@ def generateWriteEventBody(template, providerName, eventName, runtimeFlavor): if template.name in specialCaseSizes and paramName in specialCaseSizes[template.name]: size = "(int)(%s)" % specialCaseSizes[template.name][paramName] if runtimeFlavor.mono: + pack_list.append("#if BIGENDIAN") + pack_list.append(" const uint8_t *valuePtr = %s;" % paramName) + pack_list.append(" for (uint32_t i = 0; i < %s; ++i) {" % template.structs[paramName]) + types = [winTypeToFixedWidthType(t) for t in template.structTypes[paramName]] + for t in set(types) - {"UTF8String", "UTF16String"}: + pack_list.append(" %(type)s value_%(type)s;" % {'type': t}) + if "UTF8String" in types or "UTF16String" in types: + pack_list.append(" size_t value_len;") + for t in types: + if t == "UTF8String": + pack_list.append(" value_len = strlen((const char *)valuePtr);") + pack_list.append(" success &= write_buffer_string_utf8_t((const ep_char8_t *)valuePtr, value_len, &buffer, &offset, &size, &fixedBuffer);") + pack_list.append(" valuePtr += value_len + 1;") + elif t == "UTF16String": + pack_list.append(" value_len = strlen((const char *)valuePtr);") + pack_list.append(" success &= write_buffer_string_utf8_to_utf16_t((const ep_char8_t *)valuePtr, value_len, &buffer, &offset, &size, &fixedBuffer);") + pack_list.append(" valuePtr += value_len + 1;") + else: + pack_list.append(" memcpy (&value_%(type)s, valuePtr, sizeof (value_%(type)s));" % {'type': t}) + pack_list.append(" valuePtr += sizeof (%s);" % t) + pack_list.append(" success &= write_buffer_%(type)s (value_%(type)s, &buffer, &offset, &size, &fixedBuffer);" % {'type': t}) + pack_list.append(" }") + pack_list.append("#else") pack_list.append( " success &= write_buffer((const uint8_t *)%s, %s, &buffer, &offset, &size, &fixedBuffer);" % (paramName, size)) + pack_list.append("#endif // BIGENDIAN") emittedWriteToBuffer = True elif runtimeFlavor.coreclr: pack_list.append( @@ -283,9 +321,16 @@ def generateWriteEventBody(template, providerName, eventName, runtimeFlavor): if template.name in specialCaseSizes and paramName in specialCaseSizes[template.name]: size = "(int)(%s)" % specialCaseSizes[template.name][paramName] if runtimeFlavor.mono: + t = winTypeToFixedWidthType(parameter.winType) + pack_list.append("#if BIGENDIAN") + pack_list.append(" for (uint32_t i = 0; i < %s; ++i) {" % template.arrays[paramName]) + pack_list.append(" success &= write_buffer_%(type)s (%(name)s[i], &buffer, &offset, &size, &fixedBuffer);" % {'name': paramName, 'type': t}) + pack_list.append(" }") + pack_list.append("#else") pack_list.append( " success &= write_buffer((const uint8_t *)%s, %s, &buffer, &offset, &size, &fixedBuffer);" % (paramName, size)) + pack_list.append("#endif // BIGENDIAN") emittedWriteToBuffer = True elif runtimeFlavor.coreclr: pack_list.append( @@ -304,13 +349,13 @@ def generateWriteEventBody(template, providerName, eventName, runtimeFlavor): emittedWriteToBuffer = True elif parameter.winType == "win:AnsiString" and runtimeFlavor.mono: pack_list.append( - " success &= write_buffer_string_utf8_t(%s, &buffer, &offset, &size, &fixedBuffer);" % - (parameter.name,)) + " success &= write_buffer_string_utf8_t(%s, strlen((const char *)%s), &buffer, &offset, &size, &fixedBuffer);" % + (parameter.name, parameter.name)) emittedWriteToBuffer = True elif parameter.winType == "win:UnicodeString" and runtimeFlavor.mono: pack_list.append( - " success &= write_buffer_string_utf8_to_utf16_t(%s, &buffer, &offset, &size, &fixedBuffer);" % - (parameter.name,)) + " success &= write_buffer_string_utf8_to_utf16_t(%s, strlen((const char *)%s), &buffer, &offset, &size, &fixedBuffer);" % + (parameter.name, parameter.name)) emittedWriteToBuffer = True elif parameter.winType == "win:UInt8" and runtimeFlavor.mono: pack_list.append( @@ -558,6 +603,7 @@ def getMonoEventPipeHelperFileImplPrefix(): bool write_buffer_string_utf8_to_utf16_t ( const ep_char8_t *value, + size_t value_len, uint8_t **buffer, size_t *offset, size_t *size, @@ -566,6 +612,7 @@ def getMonoEventPipeHelperFileImplPrefix(): bool write_buffer_string_utf8_t ( const ep_char8_t *value, + size_t value_len, uint8_t **buffer, size_t *offset, size_t *size, @@ -640,6 +687,7 @@ def getMonoEventPipeHelperFileImplPrefix(): bool write_buffer_string_utf8_to_utf16_t ( const ep_char8_t *value, + size_t value_len, uint8_t **buffer, size_t *offset, size_t *size, @@ -653,12 +701,12 @@ def getMonoEventPipeHelperFileImplPrefix(): custom_alloc_data.buffer_size = *size - *offset; custom_alloc_data.req_buffer_size = 0; - if (!g_utf8_to_utf16_custom_alloc (value, -1, NULL, NULL, g_fixed_buffer_custom_allocator, &custom_alloc_data, NULL)) { + if (!g_utf8_to_utf16le_custom_alloc (value, (glong)value_len, NULL, NULL, g_fixed_buffer_custom_allocator, &custom_alloc_data, NULL)) { ep_raise_error_if_nok (resize_buffer (buffer, size, *offset, *size + custom_alloc_data.req_buffer_size, fixed_buffer)); custom_alloc_data.buffer = *buffer + *offset; custom_alloc_data.buffer_size = *size - *offset; custom_alloc_data.req_buffer_size = 0; - ep_raise_error_if_nok (g_utf8_to_utf16_custom_alloc (value, -1, NULL, NULL, g_fixed_buffer_custom_allocator, &custom_alloc_data, NULL) != NULL); + ep_raise_error_if_nok (g_utf8_to_utf16le_custom_alloc (value, (glong)value_len, NULL, NULL, g_fixed_buffer_custom_allocator, &custom_alloc_data, NULL) != NULL); } *offset += custom_alloc_data.req_buffer_size; @@ -671,6 +719,7 @@ def getMonoEventPipeHelperFileImplPrefix(): bool write_buffer_string_utf8_t ( const ep_char8_t *value, + size_t value_len, uint8_t **buffer, size_t *offset, size_t *size, @@ -679,10 +728,6 @@ def getMonoEventPipeHelperFileImplPrefix(): if (!value) return true; - size_t value_len = 0; - while (value [value_len]) - value_len++; - return write_buffer ((const uint8_t *)value, (value_len + 1) * sizeof(*value), buffer, offset, size, fixed_buffer); } @@ -802,6 +847,7 @@ def getMonoEventPipeImplFilePrefix(): bool write_buffer_string_utf8_t ( const ep_char8_t *value, + size_t value_len, uint8_t **buffer, size_t *offset, size_t *size, @@ -810,6 +856,7 @@ def getMonoEventPipeImplFilePrefix(): bool write_buffer_string_utf8_to_utf16_t ( const ep_char8_t *value, + size_t value_len, uint8_t **buffer, size_t *offset, size_t *size, @@ -851,6 +898,7 @@ def getMonoEventPipeImplFilePrefix(): size_t *size, bool *fixed_buffer) { + value = ep_rt_val_uint16_t (value); return write_buffer ((const uint8_t *)&value, sizeof (uint16_t), buffer, offset, size, fixed_buffer); } @@ -864,6 +912,7 @@ def getMonoEventPipeImplFilePrefix(): size_t *size, bool *fixed_buffer) { + value = ep_rt_val_uint32_t (value); return write_buffer ((const uint8_t *)&value, sizeof (uint32_t), buffer, offset, size, fixed_buffer); } @@ -877,6 +926,7 @@ def getMonoEventPipeImplFilePrefix(): size_t *size, bool *fixed_buffer) { + value = ep_rt_val_int32_t (value); return write_buffer ((const uint8_t *)&value, sizeof (int32_t), buffer, offset, size, fixed_buffer); } @@ -890,6 +940,7 @@ def getMonoEventPipeImplFilePrefix(): size_t *size, bool *fixed_buffer) { + value = ep_rt_val_uint64_t (value); return write_buffer ((const uint8_t *)&value, sizeof (uint64_t), buffer, offset, size, fixed_buffer); } @@ -903,6 +954,7 @@ def getMonoEventPipeImplFilePrefix(): size_t *size, bool *fixed_buffer) { + value = ep_rt_val_int64_t (value); return write_buffer ((const uint8_t *)&value, sizeof (int64_t), buffer, offset, size, fixed_buffer); } @@ -916,6 +968,12 @@ def getMonoEventPipeImplFilePrefix(): size_t *size, bool *fixed_buffer) { +#if BIGENDIAN + uint64_t value_as_uint64_t; + memcpy (&value_as_uint64_t, &value, sizeof (uint64_t)); + value_as_uint64_t = ep_rt_val_uint64_t (value_as_uint64_t); + memcpy (&value, &value_as_uint64_t, sizeof (uint64_t)); +#endif return write_buffer ((const uint8_t *)&value, sizeof (double), buffer, offset, size, fixed_buffer); } @@ -942,6 +1000,7 @@ def getMonoEventPipeImplFilePrefix(): size_t *size, bool *fixed_buffer) { + value = ep_rt_val_uintptr_t (value); return write_buffer ((const uint8_t *)&value, sizeof (uintptr_t), buffer, offset, size, fixed_buffer); } diff --git a/src/coreclr/scripts/genEventing.py b/src/coreclr/scripts/genEventing.py index e557a8e8aabb8a..a711373d2d4b32 100644 --- a/src/coreclr/scripts/genEventing.py +++ b/src/coreclr/scripts/genEventing.py @@ -188,10 +188,11 @@ class Template: def __repr__(self): return "