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.

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

Monitor OccurredEvents ordering #1728

Closed
JordanGottardoImagicle opened this issue Nov 4, 2021 · 2 comments · Fixed by #1773
Closed

Monitor OccurredEvents ordering #1728

JordanGottardoImagicle opened this issue Nov 4, 2021 · 2 comments · Fixed by #1773
Labels

Comments

@JordanGottardoImagicle
Copy link

JordanGottardoImagicle commented Nov 4, 2021

Description

Monitor OccurredEvents ordering seems to be random and does not reflect the order in which events were raised.

Complete minimal example reproducing the issue

public class MyClass
{
    public event EventHandler FirstEvent;
    public event EventHandler SecondEvent;
    public event EventHandler ThirdEvent;

    public void RaiseFirstEvent()
    {
        FirstEvent?.Invoke(this, EventArgs.Empty);
    }

    public void RaiseSecondEvent()
    {
        SecondEvent?.Invoke(this, EventArgs.Empty);
    }

    public void RaiseThirdEvent()
    {
        ThirdEvent?.Invoke(this, EventArgs.Empty);
    }
}

[Test]
public void Test()
{
    var myObj = new MyClass();

    using (var monitor = myObj.Monitor())
    {
        myObj.RaiseFirstEvent();
        myObj.RaiseSecondEvent();
        myObj.RaiseThirdEvent();

        monitor.OccurredEvents[0].EventName.Should().Be(nameof(myObj.FirstEvent));
        monitor.OccurredEvents[1].EventName.Should().Be(nameof(myObj.SecondEvent));
        monitor.OccurredEvents[2].EventName.Should().Be(nameof(myObj.ThirdEvent));
    }
}

Expected behavior:

OccurredEvents ordering should be the same as the raised events

Actual behavior:

OccurredEvents ordering is random, and changes with every run

Versions

I'm using FluentAssertions 5.6.0.
.NET Framework version 4.8.

@MullerWasHere
Copy link
Contributor

I managed to reproduce this issue only on .NET Framework. The "culprit", so to speak, is the precision of DateTime.Ticks used in EventRecorder.

get
{
IEnumerable<OccurredEvent> query =
from eventName in recorderMap.Keys
let recording = GetRecordingFor(eventName)
from @event in recording
orderby @event.TimestampUtc
select @event;
return query.ToArray();
}

This wrong behavior might still happen on .NET Core onwards given a lot of method calls but is rarer due to dotnet/coreclr#9736.

I'm not sure what an elegant solution might look like. BlockingCollection<EventRecorder> could be moved up to EventMonitor from EventRecorder and act as the de facto list for occurred events for all child EventRecorders, backed by an object acting as a lock whenever an event is raised.

Let me know what you think.

@dennisdoomen
Copy link
Member

Thank you for digging into this. I think what you're suggesting is one way of doing it. Another might be a thread-safe counter owned by the EventMonitor class that is used to assign unique incrementing numbers to each OccurredEvent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants