In C#, Do events keep a reference to the entire cl

2019-07-31 07:40发布

问题:

i have a class that has a ConcurrentDictionary as a private member. This class also defines a delegate/call back method. The base class registers this method as the callback for an external event. this is one only ONCE.

I am running ANT memory profiler and i'm seeing 1000s of instances of MyObj referenced from hundreds of instances of the ConcurrentDictionary property. The GC root for these is the event callback.

This seems to be causing memory to rise significantly as the application runs..after maybe about 5 min or so, a good portion of that memory is reclaimed, but i'm worried that the app could potentially run into issue since it swells sto quickly and for so long before GC kicks in.

What's going on here and how do i resolve?

This is a snippet of the base calls that registers the handler

protected abstract void DataReceivedEventHandler(DataChangedEvent evt);

public virtual void RegisterForChanges(ICollection<MemoryTable> tables)
{
    foreach (MemoryTable table in tables)
    {
        _subscribedTables.Add(table);
        table.RegisterEventListener(new DataChangedCallBack(this.DataReceivedEventHandler));

    }
}

Here is the handler which is implemented in a subclass of the above mentioned baseclass:

private ConcurrentDictionary<string, DataRecord> _cachedRecords;

protected override void DataReceivedEventHandler(DataChangedEvent evt)
{
    DataRecord record = evt.Record as DataRecord;
    string key = record.Key;
    if (string.IsNullOrEmpty(key)) { return; }

    if (_cachedRecords.ContainsKey(key))
    {
        _cachedRecords[key] = record;

        DateTime updateTime = record.UpdateTime;
        TimeSpan delta = updateTime - _lastNotifyTime;
        if (delta.TotalMilliseconds > _notificationFrequency)
        {
            PublishData(updateTime);
        }
    }
}

The publishData method publishes prism events

回答1:

Is it possible that you are re-subscribing tables over and over again? I see this:

foreach (MemoryTable table in tables)
{
    _subscribedTables.Add(table);
    table.RegisterEventListener(new DataChangedCallBack(this.DataReceivedEventHandler));
}

and I'd expect to see a check to make sure that tables are not being re-subscribed:

foreach (MemoryTable table in tables)
{
    if (!_subscribedTables.Contains(table)) {
        _subscribedTables.Add(table);
        table.RegisterEventListener(new DataChangedCallBack(this.DataReceivedEventHandler));
    }
}

EDIT: Given the comments at the beginning of the question, I am fairly confident that the problem (if you can call it a problem) lies here:

if (_cachedRecords.ContainsKey(key))
{
    _cachedRecords[key] = record;

What you are saying here is that if the record's key already exists in cachedRecords, then replace the value with the (presumably) new row instance. This is probably because some background process caused the row's data to be changed and you need to propagate those new values to the UI.

My guess is that the MemoryTable class is creating a new instance of DataRecord for these changes, and sending that new instance up the event chain to the handler we see here. If the event is fired thousands of times, then of course you are going to end up with thousands of them in memory. The garbage collector is usually pretty good about cleaning these things up, but you might want to consider in-place updates to avoid the massive GC that will occur when these instances get collected.

What you should not do is try to control (or even predict) when the GC is going to run. Just make sure that after the GC collects, the excess objects are gone (in other words, make sure that they are not being leaked) and you will be alright.



回答2:

What's going on

Yes, events are a list of delegates, which have two relevant fields: target and method. Unless you are referencing a static method, target is a reference to the class. And method is a reflection MemberInfo which tells the event which method to call.

How to troubleshoot

Consider putting a breakpoint in the add_EventName method. (If you don't have an explicit add_EventName and remove_EventName, you would have to redefine your event with this explicit code).

  private event EventHandler eventName;
  public event EventHandler EventName
  {
     add { eventName += value; } // Breakpoint here
     remove { eventName -= value; }
  }

This will help you find why it is subscribed so many times.



回答3:

After the external event is raised the one time, unsubscribe your class from it (SomeClass.SomeEvent -= MyEventHandler), or you can look at using WeakReferences



回答4:

A delegate contains a strong reference to an object along with an indication of what method to call on that object. A live object which holds a strong reference to a delegate will thus keep alive the object upon which the delegate will operate (as well as any objects to which that object holds strong references).

Sometimes one will wish to register a callback or event which will operate on an object for the benefit of some other object, but the callback or event shouldn't keep the object alive solely for its own sake. For example, an object might wish to keep a count of how many times some long-lived object raises a particular event, and thus create an instance of an "event counter" object which it attaches to the event. As long as the long-lived object holds the event subscription for the counter object, that counter object will be kept alive and the counter will get incremented every time the event is raised. Of course, if everybody who would ever have looked at the counter has ceased to exist, neither the counter, nor the effort required to increment it, will serve any useful purpose.

If one has a callback which one expects will be fired at some definite point in the future, but the callback would only serve a useful purpose if some live reference (outside the callback itself) exists to the object upon which it would operate, it may be useful to have the callback registered to a forwarding object which would in turn forward the call to the main object if doing so would make sense. The simplest way to accomplish that is to have the forwarding object hold a WeakReference. When the forwarding object receives a call, it retrieves the Target from the WeakReference. If that's non-null, it casts the retrieved target to the main object's type and calls the appropriate method on it. If the main object ceases to exist before the callback executes, the WeakReference.Target property will be null, and the forwarding object's callback will simply return silently.

A couple of further notes: (1) it might be tempting to set the Target of the WeakReference to a delegate and call that, but that approach will only work if the real target object itself holds a reference to that delegate; otherwise, the delegate itself will be eligible for garbage-collection even if its target isn't; (2) it may be helpful to cast the WeakReference to an interface, and have the main object implement that interface. This would allow one forwarding-object class to be used with many other classes. If one class might want to be attached to a number of weak events, it may be helpful to use a generic interface:

interface IDispatchAction<DummyType,ParamType>
{
  void Act(ParamType ref param);
}

That would allow the main object to expose a number of IDispatchAction actions (e.g. if if a class implements IDispatchAction<foo,int>.Act and IDispatchAction<bar,int>.Act, then casting a reference to that class to one of those interfaces and calling Act upon it will invoke the appropriate method).



回答5:

If we want that the object defined callback method should not stay in memory, we have to define this method as Static.