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

fix Metric livelock by replacing potential infinate loop in MetricValuesBuffer.GetAndResetValue #2612

Merged
merged 10 commits into from
Jun 17, 2022
Merged

fix Metric livelock by replacing potential infinate loop in MetricValuesBuffer.GetAndResetValue #2612

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
74b921c
initial commit
TimothyMothra Jun 15, 2022
59a34a2
less invasive change
TimothyMothra Jun 16, 2022
891bced
rename
TimothyMothra Jun 16, 2022
eac36ef
Merge branch 'main' into tilee/1186_GetAndResetValue
TimothyMothra Jun 16, 2022
0817a0b
review comment and changelog
TimothyMothra Jun 16, 2022
b9c3166
Update CHANGELOG.md
TimothyMothra Jun 16, 2022
ed14949
update changelog
TimothyMothra Jun 17, 2022
e062585
log warning
TimothyMothra Jun 17, 2022
404ef6c
sort usings
TimothyMothra Jun 17, 2022
10f521c
fxcop
TimothyMothra Jun 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions BASE/src/Microsoft.ApplicationInsights/Metrics/Extensibility/MetricValuesBuffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ public int NextFlushIndex
set { this.nextFlushIndex = value; }
}

protected abstract TValue DefaultValue { get; }

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public int IncWriteIndex()
{
Expand Down Expand Up @@ -79,31 +81,29 @@ public void ResetIndicesAndData()

public TValue GetAndResetValue(int index)
{
TValue value = this.GetAndResetValueOnce(this.values, index);

if (this.IsInvalidValue(value))
for (int spinCountdown = 10000; spinCountdown > 0; spinCountdown--)
TimothyMothra marked this conversation as resolved.
Show resolved Hide resolved
{
#pragma warning disable SA1129 // Do not use default value type constructor
var spinWait = new SpinWait();
#pragma warning restore SA1129 // Do not use default value type constructor
TValue value = this.GetAndResetValueOnce(this.values, index);

value = this.GetAndResetValueOnce(this.values, index);
while (this.IsInvalidValue(value))
if (this.IsInvalidValue(value))
{
spinWait.SpinOnce();

if (spinWait.Count % 100 == 0)
if (spinCountdown % 100 == 0)
{
// In tests (including stress tests) we always finished wating before 100 cycles.
// In tests (including stress tests) we always finished waiting before 100 cycles.
// However, this is a protection against en extreme case on a slow machine.
Task.Delay(10).ConfigureAwait(continueOnCapturedContext: false).GetAwaiter().GetResult();
}

value = this.GetAndResetValueOnce(this.values, index);
continue;
}
else
{
return value;
}
}

return value;
// exceeded maximum spin count
return this.DefaultValue;
}

protected abstract void ResetValues(TValue[] values);
Expand All @@ -127,6 +127,8 @@ public MetricValuesBuffer_Double(int capacity)
{
}

protected override double DefaultValue => double.NaN;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
protected override bool IsInvalidValue(double value)
{
Expand Down Expand Up @@ -162,6 +164,8 @@ public MetricValuesBuffer_Object(int capacity)
{
}

protected override object DefaultValue => null;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
protected override bool IsInvalidValue(object value)
{
Expand Down