0

In reviewing some code, I came across a simple copy/paste bug that led to counter intuitive behavior. The intent was to subscribe to System and Application event log events and process them both. The boiled down code below looks to me as though the first event producer, for Application log, gets replaced and therefore shouldn't produce any more events after garbage collection. However, what I observe is that any writes to the application log will still trigger the ApplicationEventLog_EntryWritten method.

I presume then that something is holding onto a reference to the original EventLog instance but I would like to understand what and why?

class Program
{
    public static EventLog ApplicationEventLog;

    static void Main(string[] args)
    {
        ApplicationEventLog = new EventLog("Application");
        ApplicationEventLog.EnableRaisingEvents = true;
        ApplicationEventLog.EntryWritten += ApplicationEventLog_EntryWritten;

        //new instance should have been assigned to new variable
        //public static EventLog sytemEventLog;
        ApplicationEventLog = new EventLog("System");
        ApplicationEventLog.EnableRaisingEvents = true;
        ApplicationEventLog.EntryWritten += SystemEventLog_EntryWritten;

        //for illustrative purposes
        //Original EventLog instance is no longer assigned to anything so I would expect it to get GC'd 
        GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, true);

        Console.WriteLine("Press ESC to stop");
        do
        {
            while (!Console.KeyAvailable)
            {
                Task.Delay(250);
            }
        } while (Console.ReadKey(true).Key != ConsoleKey.Escape);
    }

    private static void ApplicationEventLog_EntryWritten(object sender, EntryWrittenEventArgs e)
    {
        Debug.WriteLine("Application log written to");
    }
    private static void SystemEventLog_EntryWritten(object sender, EntryWrittenEventArgs e)
    {
        Debug.WriteLine("System log written to");
    }
}

UPDATED EXAMPLE:

This version takes all eventlog code out of the picture for a hopefully clearer example:

class Program
{
    static void Main(string[] args)
    {
        var producer = new Producer();
        producer.SomeEvent += EventConsumer1;
        //Obviously unsubscribing before reassignment works but why does the subscription prevent GC of this first instance?
        //producer.SomeEvent -= EventConsumer1;

        producer = new Producer();
        producer.SomeEvent += EventConsumer2;


        Console.WriteLine("Press ESC to stop");
        do
        {
            while (!Console.KeyAvailable)
            {
                //for illustrative purposes
                //Original producer instance is no longer assigned to anything so I would expect it to get GC'd 
                GC.WaitForPendingFinalizers();
                GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, true);

                Task.Delay(250);
            }
        } while (Console.ReadKey(true).Key != ConsoleKey.Escape);
    }


    private static void EventConsumer1(object sender, EventArgs e)
    {
        Debug.WriteLine("I would have expected this to get hit once at most");
    }
    private static void EventConsumer2(object sender, EventArgs e)
    {
        Debug.WriteLine("Should hit this periodically");
    }
}

public class Producer
{
    public event EventHandler SomeEvent;

    public Producer()
    {
        SendEventsAsync();
    }

    private async void SendEventsAsync()
    {
        while (true)
        {
            SomeEvent?.Invoke(this, new EventArgs());
            await Task.Delay(5000);
        }
    }

    ~Producer()
    {
        Debug.WriteLine("Instance being garbage collected");
    }
}
Tim
  • 412
  • 7
  • 18
  • https://stackoverflow.com/a/298276/3346583 – Christopher Oct 22 '19 at 18:06
  • You still have the event handler registered, to clear unregister the event handler with `-=` Event lingering event handlers are common source of leaks. – JSteward Oct 22 '19 at 18:07
  • The internal `EntryWrittenEventHandler` is not cleared when you call new(). Dispose of the previous `ApplicationEventLog` (which will also call the `StopRaisingEvents` clean-up method). Better unsubscribe events you subscribed to and dispose of disposable objects, right?. – Jimi Oct 22 '19 at 18:23
  • @Christopher that link seems to confirm my confusion: then "publisher" will keep "target" alive, but "target" will not keep "publisher" alive. – Tim Oct 22 '19 at 18:45
  • @Jimi I understand how to fix it. My question is more about why the reference is held despite the reassignment. Shouldn't the original instance be considered out of scope, the object's finalizer implicitly called, and therefore Dispose also called? – Tim Oct 22 '19 at 18:54
  • 1
    The `EntryWrittenEventHandler` delegate doesn't belong to the `ApplicationEventLog` object. When you `new()` the latter, the former is still alive and kicking. The new `ApplicationEventLog` adds a new handler. – Jimi Oct 22 '19 at 18:54
  • For a second, I thought I understood. :) From the reference sources it looks to me as though an EntryWrittenEventHandler belongs to each EventLog instance? Really its a redirect to a private instance of EventLogInternal.EntryWritten though. – Tim Oct 22 '19 at 20:04
  • I also see that EventLog derives from System.ComponentModel.Component which also has events = new EventHandlerList(this). To me it looks like if anything called the Events accessor it would cause a self reference that would cause it to never be garbage collected but I don't see any calls to it. – Tim Oct 22 '19 at 20:08
  • [EventLogInternal](https://referencesource.microsoft.com/#System/services/monitoring/system/diagnosticts/EventLogInternal.cs,48) is a distinct class, which initializes its own objects (and delegates). That's why you need to call [Dispose()](https://referencesource.microsoft.com/#System/services/monitoring/system/diagnosticts/EventLog.cs,764) on `ApplicationEventLog`. Disposing of it causes a call to the `EventLogInternal`'s [Close()](https://referencesource.microsoft.com/#System/services/monitoring/system/diagnosticts/EventLogInternal.cs,657) method. See what it does. – Jimi Oct 22 '19 at 20:27
  • An instance of EventLogInternal is owned by each EventLog instance: private EventLogInternal m_underlyingEventLog; https://referencesource.microsoft.com/system/services/monitoring/system/diagnosticts/EventLog.cs.html#https://referencesource.microsoft.com/system/services/monitoring/system/diagnosticts/EventLog.cs.html,https://referencesource.microsoft.com/system/services/monitoring/system/diagnosticts/EventLog.cs.html,247410ebc0e78113 – Tim Oct 22 '19 at 21:17
  • When it instantiates, EventLogInternal gets passed a reference to the parent via "this". Perhaps that would be enough to prevent garbage collection: new EventLogInternal(m_underlyingEventLog.logName, value, m_underlyingEventLog.sourceName, this); – Tim Oct 22 '19 at 21:20

0 Answers0