EventCounter publishing thread can hang

[noahfalk]

Description

When listening to EventCounters there is a dedicated thread that is supposed to publish the metric values via EventSource on a periodic timer. However due a bug it is possible to put it into an infinite loop here. This occurs any time _pollingIntervalMilliseconds <= 0, which could occur for a few reasons:

1. The listener specified EventCounterIntervalSec=0 after previously specifying a non-zero value. We would never expect a well-behaved listener to request a 0 sec interval, but the runtime shouldn't blindly trust this input.
2. The listener enables and then disables the EventSource. Due to a race the infinite loop code could be run prior to re-checking _eventSource.IsEnabled() and it would observe _pollingIntervaMilliseconds=0

Configuration

.NET 5 (but presumably also reproable in .NET 3 and 3.1)

Regression?

The underlying bug was introduced in 2019 and shipped in .NET Core 3, 3.1, and 5:
However at this point only failure reason (1) above would have been able to trigger it.

In August 2020 an additional change was made in .NET Core 5 (and backported to 3.1) which opened the race condition allowing failure reason (2) above to be hit.

Repro

using System;
using System.Collections.Generic;
using System.Diagnostics.Tracing;
using System.Threading.Tasks;

internal sealed class RuntimeEventListener : EventListener
{
    protected override void OnEventSourceCreated(EventSource source)
    {
        if (source.Name.Equals("System.Runtime"))
        {
            EnableEvents(source, EventLevel.LogAlways, EventKeywords.None, new Dictionary<string, string>()
                { { "EventCounterIntervalSec", "1" } });
            
            // The next line makes the example fail, comment this out to see it work normally
            EnableEvents(source, EventLevel.LogAlways, EventKeywords.None, new Dictionary<string, string>()
                { { "EventCounterIntervalSec", "0" } });
        }
    }

    protected override void OnEventWritten(EventWrittenEventArgs eventData)
    {
        Console.WriteLine(eventData.TimeStamp + " " + eventData.EventName);
    }
}

class Program
{
    static void Main(string[] args)
    {
        RuntimeEventListener listener = new RuntimeEventListener();
        Console.ReadLine();
    }
}

Expected behavior: No threads should spin in the counter infinite loop
Actual behavior: A thread will hang in that loop

Note: this repro only shows technique (1) to repro the problem. Any fix still needs to account for technique (2) as well.

[ghost]

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

When listening to EventCounters there is a dedicated thread that is supposed to publish the metric values via EventSource on a periodic timer. However due a bug it is possible to put it into an infinite loop here. This occurs any time _pollingIntervalMilliseconds <= 0, which could occur for a few reasons:

1. The listener specified EventCounterIntervalSec=0 after previously specifying a non-zero value. We would never expect a well-behaved listener to request a 0 sec interval, but the runtime shouldn't blindly trust this input.
2. The listener enables and then disables the EventSource. Due to a race the infinite loop code could be run prior to re-checking _eventSource.IsEnabled() and it would observe _pollingIntervaMilliseconds=0

Configuration

.NET 5 (but presumably also reproable in .NET 3 and 3.1)

Regression?

The underlying bug was introduced in 2019 and shipped in .NET Core 3, 3.1, and 5:
However at this point only failure reason (1) above would have been able to trigger it.

In August 2020 an additional change was made in .NET Core 5 (and backported to 3.1) which opened the race condition allowing failure reason (2) above to be hit.

Author:	noahfalk
Assignees:	-
Labels:	area-System.Diagnostics.Tracing, untriaged
Milestone:	-

[noahfalk]

@josalem