fix Flush deadlock by implementing SpinWait pattern using Interlocked.CompareExchange

[Timothy Mothra (TimothyMothra)]

Fix Issue #1186.

Metrics Flushing has a deadlock which occurs here:

ApplicationInsights-dotnet/BASE/src/Microsoft.ApplicationInsights/Metrics/Extensibility/MetricSeriesAggregatorBase.cs

Lines 398 to 411 in 5aed71e

	lock (buffer)
	{
	int maxFlushIndex = Math.Min(buffer.PeekLastWriteIndex(), buffer.Capacity - 1);
	int minFlushIndex = buffer.NextFlushIndex;
	if (minFlushIndex > maxFlushIndex)
	{
	return;
	}
	stage1Result = this.UpdateAggregate_Stage1(buffer, minFlushIndex, maxFlushIndex);
	buffer.NextFlushIndex = maxFlushIndex + 1;
	}

This PR removes the lock and implements a SpinWait pattern using Interlocked.CompareExchange().
To make this work, Interlocked.CompareExchange needs access to the index variable in the class MetricValuesBuffer. Because of this, I've had to do some refactoring.

Changes

• removed lock from the MetricSeriesAggregatorBase class.
• added new method TryGetFlushIndexes() into the MetricValuesBuffer class.
  This new method uses Interlocked.CompareExchange to try to advance the index variable.

Alternatives considered

I had a PoC that replaced the lock with Monitor.TryEnter. #2594
This more closely matches the original author's intent using a lock, and adds a timeout to mitigate the deadlock.
This was abandoned in favor of removing the lock entirely.

Checklist

• I ran Unit Tests locally.
• CHANGELOG.md updated with one line description of the fix, and a link to the original issue if available.

For significant contributions please make sure you have completed the following items:

• Design discussion issue #
• Changes in public surface reviewed

The PR will trigger build, unit tests, and functional tests automatically. Please follow these instructions to build and test locally.

Notes for authors:

• FxCop and other analyzers will fail the build. To see these errors yourself, compile localy using the Release configuration.

Notes for reviewers:

• We support comment build triggers
  • /AzurePipelines run will queue all builds
  • /AzurePipelines run <pipeline-name> will queue a specific build

[Timothy Mothra (TimothyMothra)]

GitHub won't let Kennedy comment. He shared his comments offline:

Won't let me leave a comment on that PR...
But taking a look, I'm curious why a while(true) and not a for loop with a break and return false at the end?
continue at the bottom is also not needed
Otherwise looks good