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

EventSource dynamic events (Write<T>) will be dispatched to all listeners regardless whether the listener enabled the source #9105

Closed
karolz-ms opened this issue Oct 11, 2017 · 4 comments
Labels
area-Tracing-coreclr backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity
Milestone

Comments

@karolz-ms
Copy link
Member

Run the following program to reproduce the problem:

    class Program
    {
        static void Main(string[] args)
        {
        
            var aSource = new AlphaSource();
            var bSource = new BravoSource();
            using (var al = new AlphaListener())
            {
                aSource.AlphaMessage("A1");
                bSource.BravoMessage("B1");

                using (var bl = new BravoListener())
                {
                    aSource.AlphaMessage("A2");
                    bSource.BravoMessage("B2");
                    aSource.Write("Dynamic", new { Message = "A3 dynamic" });

                    Console.ReadKey();
                }
            }         
        }
   }


    [EventSource(Name ="Alpha")]
    class AlphaSource: EventSource
    {
        [Event(1)]
        public void AlphaMessage(string arg)
        {
            this.WriteEvent(1, arg);
        }
    }

    [EventSource(Name = "Bravo")]
    class BravoSource : EventSource
    {
        [Event(1)]
        public void BravoMessage(string arg)
        {
            this.WriteEvent(1, arg);
        }
    }

    class AlphaListener : EventListener
    {
        protected override void OnEventWritten(EventWrittenEventArgs eventData)
        {
            Console.WriteLine($"AlphaListener: got event from {eventData.EventSource.Name}: {eventData.Payload[0]}");
        }

        protected override void OnEventSourceCreated(EventSource source)
        {
            if (source is AlphaSource)
            {
                this.EnableEvents(source, EventLevel.LogAlways, (EventKeywords) (-1));
            }
        }
    }

    class BravoListener : EventListener
    {
        protected override void OnEventWritten(EventWrittenEventArgs eventData)
        {
            Console.WriteLine($"BravoListener: got event from {eventData.EventSource.Name}: {eventData.Payload[0]}");
        }

        protected override void OnEventSourceCreated(EventSource source)
        {
            if (source is BravoSource)
            {
                this.EnableEvents(source, EventLevel.LogAlways, (EventKeywords)(-1));
            }
        }
    }

Talked to @vancem , he thinks it is a legitimate issue in DispatchToAllListeners implementation, specifically the eventId == -1 in the code if (eventId == -1 || dispatcher.m_EventEnabled[eventId])

@karolz-ms
Copy link
Member Author

@brianrob FYI

@vancem
Copy link
Contributor

vancem commented Oct 11, 2017

Two things

First, this is a bug, but it is more like incomplete feature. When Write was added to EventSource, the subscription logic in DispatchToAllListerns was not appropriately completed.

The problem was that Dispatchers (which decide which EvenListeners get event), have a array of Booleans indexed by event ID that make this determination. Write events don't have eventIDs (just names, keywords and levels) and the logic simply ignored this (and unconditionally sent the event to all listeners).

At the very least we should have a bit in the Dispatcher that indicates that the listener has subscribe to the EventSource AT ALL, and skip listeners that did not subscribe at all (that is pretty easy).

However better is that we should dispatch only those Write events that, based on keywords and level, were subscribed by the listener. One way of doing this is to give Write events an event ID (allocated on the fly), and use the existing Boolean array (thus everything happens dynamically). The other way is to remember the keywords and level in the dispatcher and then do the comparison logic in the DispatchToAllListeners.

It will probably take a couple hours to design and code up.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@ghost
Copy link

ghost commented Oct 27, 2022

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

@ghost ghost added backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity labels Oct 27, 2022
@ghost
Copy link

ghost commented Nov 10, 2022

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.

@ghost ghost closed this as completed Nov 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tracing-coreclr backlog-cleanup-candidate An inactive issue that has been marked for automated closure. no-recent-activity
Projects
None yet
Development

No branches or pull requests

3 participants