Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[release/8.0] Fix Options Source Gen RangeAttribute Thread Safety #97110

Merged
merged 2 commits into from
Jan 17, 2024
Merged

[release/8.0] Fix Options Source Gen RangeAttribute Thread Safety #97110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f42dcf0
[release/8.0] Fix Options Source Gen RangeAttribute Thread Safety
tarekgh Jan 17, 2024
4e4ca7a
Delete un-wanted line
tarekgh Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
83 changes: 45 additions & 38 deletions src/libraries/Microsoft.Extensions.Options/gen/Emitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,26 +382,26 @@ public void EmitRangeAttribute(string modifier, string prefix, string className,

string initializationString = emitTimeSpanSupport ?
"""
if (OperandType == typeof(global::System.TimeSpan))
{
if (!global::System.TimeSpan.TryParse((string)Minimum, culture, out global::System.TimeSpan timeSpanMinimum) ||
!global::System.TimeSpan.TryParse((string)Maximum, culture, out global::System.TimeSpan timeSpanMaximum))
{
throw new global::System.InvalidOperationException(c_minMaxError);
}
Minimum = timeSpanMinimum;
Maximum = timeSpanMaximum;
}
else
{
Minimum = ConvertValue(Minimum, culture) ?? throw new global::System.InvalidOperationException(c_minMaxError);
Maximum = ConvertValue(Maximum, culture) ?? throw new global::System.InvalidOperationException(c_minMaxError);
}
if (OperandType == typeof(global::System.TimeSpan))
{
if (!global::System.TimeSpan.TryParse((string)Minimum, culture, out global::System.TimeSpan timeSpanMinimum) ||
!global::System.TimeSpan.TryParse((string)Maximum, culture, out global::System.TimeSpan timeSpanMaximum))
{
throw new global::System.InvalidOperationException(MinMaxError);
}
Minimum = timeSpanMinimum;
Maximum = timeSpanMaximum;
}
else
{
Minimum = ConvertValue(Minimum, culture) ?? throw new global::System.InvalidOperationException(MinMaxError);
Maximum = ConvertValue(Maximum, culture) ?? throw new global::System.InvalidOperationException(MinMaxError);
}
"""
:
"""
Minimum = ConvertValue(Minimum, culture) ?? throw new global::System.InvalidOperationException(c_minMaxError);
Maximum = ConvertValue(Maximum, culture) ?? throw new global::System.InvalidOperationException(c_minMaxError);
Minimum = ConvertValue(Minimum, culture) ?? throw new global::System.InvalidOperationException(MinMaxError);
Maximum = ConvertValue(Maximum, culture) ?? throw new global::System.InvalidOperationException(MinMaxError);
""";

string convertValue = emitTimeSpanSupport ?
Expand Down Expand Up @@ -470,7 +470,7 @@ public void EmitRangeAttribute(string modifier, string prefix, string className,
public {{qualifiedClassName}}(global::System.Type type, string minimum, string maximum) : base()
{
OperandType = type;
NeedToConvertMinMax = true;
_needToConvertMinMax = true;
Minimum = minimum;
Maximum = maximum;
}
Expand All @@ -483,33 +483,40 @@ public void EmitRangeAttribute(string modifier, string prefix, string className,
public bool ConvertValueInInvariantCulture { get; set; }
public override string FormatErrorMessage(string name) =>
string.Format(global::System.Globalization.CultureInfo.CurrentCulture, GetValidationErrorMessage(), name, Minimum, Maximum);
private bool NeedToConvertMinMax { get; }
private bool Initialized { get; set; }
private const string c_minMaxError = "The minimum and maximum values must be set to valid values.";
private readonly bool _needToConvertMinMax;
private volatile bool _initialized;
private readonly object _lock = new();
private const string MinMaxError = "The minimum and maximum values must be set to valid values.";

public override bool IsValid(object? value)
{
if (!Initialized)
if (!_initialized)
{
if (Minimum is null || Maximum is null)
{
throw new global::System.InvalidOperationException(c_minMaxError);
}
if (NeedToConvertMinMax)
lock (_lock)
{
System.Globalization.CultureInfo culture = ParseLimitsInInvariantCulture ? global::System.Globalization.CultureInfo.InvariantCulture : global::System.Globalization.CultureInfo.CurrentCulture;
if (!_initialized)
{
if (Minimum is null || Maximum is null)
{
throw new global::System.InvalidOperationException(MinMaxError);
}
if (_needToConvertMinMax)
{
System.Globalization.CultureInfo culture = ParseLimitsInInvariantCulture ? global::System.Globalization.CultureInfo.InvariantCulture : global::System.Globalization.CultureInfo.CurrentCulture;
{{initializationString}}
}
int cmp = ((global::System.IComparable)Minimum).CompareTo((global::System.IComparable)Maximum);
if (cmp > 0)
{
throw new global::System.InvalidOperationException("The maximum value '{Maximum}' must be greater than or equal to the minimum value '{Minimum}'.");
}
else if (cmp == 0 && (MinimumIsExclusive || MaximumIsExclusive))
{
throw new global::System.InvalidOperationException("Cannot use exclusive bounds when the maximum value is equal to the minimum value.");
}
_initialized = true;
}
}
int cmp = ((global::System.IComparable)Minimum).CompareTo((global::System.IComparable)Maximum);
if (cmp > 0)
{
throw new global::System.InvalidOperationException("The maximum value '{Maximum}' must be greater than or equal to the minimum value '{Minimum}'.");
}
else if (cmp == 0 && (MinimumIsExclusive || MaximumIsExclusive))
{
throw new global::System.InvalidOperationException("Cannot use exclusive bounds when the maximum value is equal to the minimum value.");
}
Initialized = true;
}

if (value is null or string { Length: 0 })
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Microsoft.Extensions.Options/src/Microsoft.Extensions.Options.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
<TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.1;netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
<EnableDefaultItems>true</EnableDefaultItems>
<IsPackable>true</IsPackable>
<GeneratePackageOnBuild>false</GeneratePackageOnBuild>
<ServicingVersion>1</ServicingVersion>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<ServicingVersion>2</ServicingVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👌

<PackageDescription>Provides a strongly typed way of specifying and accessing settings using dependency injection.</PackageDescription>
</PropertyGroup>

Expand Down
53 changes: 30 additions & 23 deletions ...tions/tests/SourceGeneration.Unit.Tests/Baselines/EmitterWithCustomValidator.netcore.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public __SourceGen__RangeAttribute(double minimum, double maximum) : base()
public __SourceGen__RangeAttribute(global::System.Type type, string minimum, string maximum) : base()
{
OperandType = type;
NeedToConvertMinMax = true;
_needToConvertMinMax = true;
Minimum = minimum;
Maximum = maximum;
}
Expand All @@ -97,34 +97,41 @@ public __SourceGen__RangeAttribute(global::System.Type type, string minimum, str
public bool ConvertValueInInvariantCulture { get; set; }
public override string FormatErrorMessage(string name) =>
string.Format(global::System.Globalization.CultureInfo.CurrentCulture, GetValidationErrorMessage(), name, Minimum, Maximum);
private bool NeedToConvertMinMax { get; }
private bool Initialized { get; set; }
private const string c_minMaxError = "The minimum and maximum values must be set to valid values.";
private readonly bool _needToConvertMinMax;
private volatile bool _initialized;
private readonly object _lock = new();
private const string MinMaxError = "The minimum and maximum values must be set to valid values.";

public override bool IsValid(object? value)
{
if (!Initialized)
if (!_initialized)
{
if (Minimum is null || Maximum is null)
lock (_lock)
{
throw new global::System.InvalidOperationException(c_minMaxError);
if (!_initialized)
{
if (Minimum is null || Maximum is null)
{
throw new global::System.InvalidOperationException(MinMaxError);
}
if (_needToConvertMinMax)
{
System.Globalization.CultureInfo culture = ParseLimitsInInvariantCulture ? global::System.Globalization.CultureInfo.InvariantCulture : global::System.Globalization.CultureInfo.CurrentCulture;
Minimum = ConvertValue(Minimum, culture) ?? throw new global::System.InvalidOperationException(MinMaxError);
Maximum = ConvertValue(Maximum, culture) ?? throw new global::System.InvalidOperationException(MinMaxError);
}
int cmp = ((global::System.IComparable)Minimum).CompareTo((global::System.IComparable)Maximum);
if (cmp > 0)
{
throw new global::System.InvalidOperationException("The maximum value '{Maximum}' must be greater than or equal to the minimum value '{Minimum}'.");
}
else if (cmp == 0 && (MinimumIsExclusive || MaximumIsExclusive))
{
throw new global::System.InvalidOperationException("Cannot use exclusive bounds when the maximum value is equal to the minimum value.");
}
_initialized = true;
}
}
if (NeedToConvertMinMax)
{
System.Globalization.CultureInfo culture = ParseLimitsInInvariantCulture ? global::System.Globalization.CultureInfo.InvariantCulture : global::System.Globalization.CultureInfo.CurrentCulture;
Minimum = ConvertValue(Minimum, culture) ?? throw new global::System.InvalidOperationException(c_minMaxError);
Maximum = ConvertValue(Maximum, culture) ?? throw new global::System.InvalidOperationException(c_minMaxError);
}
int cmp = ((global::System.IComparable)Minimum).CompareTo((global::System.IComparable)Maximum);
if (cmp > 0)
{
throw new global::System.InvalidOperationException("The maximum value '{Maximum}' must be greater than or equal to the minimum value '{Minimum}'.");
}
else if (cmp == 0 && (MinimumIsExclusive || MaximumIsExclusive))
{
throw new global::System.InvalidOperationException("Cannot use exclusive bounds when the maximum value is equal to the minimum value.");
}
Initialized = true;
}

if (value is null or string { Length: 0 })
Expand Down
53 changes: 30 additions & 23 deletions ...Options/tests/SourceGeneration.Unit.Tests/Baselines/EmitterWithCustomValidator.netfx.g.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public __SourceGen__RangeAttribute(double minimum, double maximum) : base()
public __SourceGen__RangeAttribute(global::System.Type type, string minimum, string maximum) : base()
{
OperandType = type;
NeedToConvertMinMax = true;
_needToConvertMinMax = true;
Minimum = minimum;
Maximum = maximum;
}
Expand All @@ -95,34 +95,41 @@ public __SourceGen__RangeAttribute(global::System.Type type, string minimum, str
public bool ConvertValueInInvariantCulture { get; set; }
public override string FormatErrorMessage(string name) =>
string.Format(global::System.Globalization.CultureInfo.CurrentCulture, GetValidationErrorMessage(), name, Minimum, Maximum);
private bool NeedToConvertMinMax { get; }
private bool Initialized { get; set; }
private const string c_minMaxError = "The minimum and maximum values must be set to valid values.";
private readonly bool _needToConvertMinMax;
private volatile bool _initialized;
private readonly object _lock = new();
private const string MinMaxError = "The minimum and maximum values must be set to valid values.";

public override bool IsValid(object? value)
{
if (!Initialized)
if (!_initialized)
{
if (Minimum is null || Maximum is null)
lock (_lock)
{
throw new global::System.InvalidOperationException(c_minMaxError);
if (!_initialized)
{
if (Minimum is null || Maximum is null)
{
throw new global::System.InvalidOperationException(MinMaxError);
}
if (_needToConvertMinMax)
{
System.Globalization.CultureInfo culture = ParseLimitsInInvariantCulture ? global::System.Globalization.CultureInfo.InvariantCulture : global::System.Globalization.CultureInfo.CurrentCulture;
Minimum = ConvertValue(Minimum, culture) ?? throw new global::System.InvalidOperationException(MinMaxError);
Maximum = ConvertValue(Maximum, culture) ?? throw new global::System.InvalidOperationException(MinMaxError);
}
int cmp = ((global::System.IComparable)Minimum).CompareTo((global::System.IComparable)Maximum);
if (cmp > 0)
{
throw new global::System.InvalidOperationException("The maximum value '{Maximum}' must be greater than or equal to the minimum value '{Minimum}'.");
}
else if (cmp == 0 && (MinimumIsExclusive || MaximumIsExclusive))
{
throw new global::System.InvalidOperationException("Cannot use exclusive bounds when the maximum value is equal to the minimum value.");
}
_initialized = true;
}
}
if (NeedToConvertMinMax)
{
System.Globalization.CultureInfo culture = ParseLimitsInInvariantCulture ? global::System.Globalization.CultureInfo.InvariantCulture : global::System.Globalization.CultureInfo.CurrentCulture;
Minimum = ConvertValue(Minimum, culture) ?? throw new global::System.InvalidOperationException(c_minMaxError);
Maximum = ConvertValue(Maximum, culture) ?? throw new global::System.InvalidOperationException(c_minMaxError);
}
int cmp = ((global::System.IComparable)Minimum).CompareTo((global::System.IComparable)Maximum);
if (cmp > 0)
{
throw new global::System.InvalidOperationException("The maximum value '{Maximum}' must be greater than or equal to the minimum value '{Minimum}'.");
}
else if (cmp == 0 && (MinimumIsExclusive || MaximumIsExclusive))
{
throw new global::System.InvalidOperationException("Cannot use exclusive bounds when the maximum value is equal to the minimum value.");
}
Initialized = true;
}

if (value is null or string { Length: 0 })
Expand Down