WeakEventDelegate implementation - feedback request

I have just implemented a WeakEventDelegate class in .NET.

I had seen other articles to the effect of implementing such a thing in http://code.logos.com/blog/2008/08/event_subscription_using_weak_references.html and http://blogs.msdn.com/b/greg_schechter/archive/2004/05/27/143605.aspx

However, the implementation I arrived at is less complicated (though less flexible), and seems to do the job, so I was wondering whether there was something I had missed.

Is there any problem with the following implementation, except for its relative lack of flexibility?

public class WeakEventDelegate<TEvent开发者_运维知识库Args> 
    where TEventArgs : EventArgs
{
    private readonly WeakReference handlerReference;

    public WeakEventDelegate(Action<object, TEventArgs> handler)
    {
        handlerReference = new WeakReference(handler);
    }

    public void Handle(object source, TEventArgs e)
    {
        Action<object, TEventArgs> unwrappedHandler = (Action<object, TEventArgs>)handlerReference.Target;
        if (unwrappedHandler != null)
        {
            unwrappedHandler.Invoke(source, e);
        }
    }
}

EDIT: My only intent in writing that class was to prevent the implicit reference from the publisher to the delegate to prevent garbage collecting of the subscriber.

Meaning that, instead of writing:

void subscribe()
{
    publisher.RaiseCustomEvent += this.HandleCustomEvent;
}

I would write:

private readonly WeakDelegate<CustomEventArgs> _customHandler = new WeakDelegate<CustomEventArgs>(this.HandleCustomEvent);
void subscribe()
{
    publisher.RaiseCustomEvent += _customHandler.Handle;
}

The main use-case I have in mind for that class is for a few collection classes (subscribers) that have a lifetime that I can only barely control. (However one of these cases happen in WPF data binding so it would be a perfect candidate for using the recommended Weak Event infrastructure).

The main issue here is that anything subscribing to your delegate (handlerReference.Target) is going to keep handlerReference alive.

Wrapping this up provides you with the means to call your delegate without holding a reference to it, but does nothing to prevent the subscriber from keeping the reference alive.

The Weak Event Patterns espoused by the framework function by having an intermediary. Subscriptions are handled through the intermediary, and both sides of the equation are maintained via weak references. Nothing holds a reference directly to a delegate - since the reference to the delegate keeps the object alive.

I have found a critical problem with that implementation - it only works if I keep a reference to the delegate (the handler passed in the constructor) somewhere else. Otherwise that delegate object is collected and the event never fires again.

Thoughts

I like the idea of implementing a weak delegate on the side of the class which handles the event because of the fact that i don't want to have weak references only in some cases. For my personal ui framework i react to events sometimes without holding a reference to the original object:

void HandlePropertyChanged(Object sender, PropertyChangedArgs args)
{
    // Do some stuff with the sender
}

This requires non-weak delegates, while other objects require weak ones.

Solution

I stumbled over the same problems which you've and solved it by not using a WeakReference to a delegate but using a WeakReference for the target of the delegate and copying the empty delegate.

// Create a weak reference to the target
_Sender = new WeakReference(target.Target);
_CallDelegate = (Action<TThis, TSender, TArgs>)Delegate.CreateDelegate(CallDelegateType, target.Method);

This works perfectly but one big problem remained: How to remove the event when not longer used?

Well there is a solution for this too, but the solution requires some reflection code and is a source for errors:

_RemoveMethod = (Action<DataEventHandler<TSender, TArgs>>)Delegate.CreateDelegate(AddMethodType, eventHolder, eventDeclaration.GetRemoveMethod());

The eventDeclaration is a EventInfo instance and comes with the arguments of the constructor, to make it a bit easier for the user i created a second constructor which just accepts the instance ( eventHolder ) and a string which represents the name of the event.

When the object isn't alive anymore the _RemoveMethod-delegate is called and removes the WeakDelegate.

Usage

n.Disposing += (_DisposeDelegate = new WeakDelegate<Drawable, IComponent,Object>(n, "Disposing", HandleRootDisposing));

As you can see there is still one issue: The WeakDelegate must be keept in memory when it's required to remove the event later. There is a solution for this too but it requires to override the equality methods of the WeakDelegate and store all the delegates in a hash table which might slow down the application a lot.

A major problem with weak events is that if the subscription list holds the only reference to a subscriber, there's no way the event publisher can know whether or not anything cares about the event. Fundamentally, the event should remain live as long as any references exists to anything which will be modified by the event handler, but there's no way the event publisher can know whether such references may exist or what form they might take.

An alternative approach would be to have each event subscriber pass a reference to an object which implements a suitable event-notification interface, and have that interface include a property which indicates whether the subscriber is still interested in the event. When event subscriptions are added, the event publisher would sometimes poll subscribers to see whether they were still interested; these operations could, as convenient, be done piecemeal or batched, but the average number of subscriber-poll operations per subscription added should be bounded by a constant.

Note that the existing event infrastructure works rather poorly with weak references, since repeatedly adding and abandoning subscribers for an event which doesn't fire could create an arbitrarily-large list of subscriptions by objects holding dead WeakReferences, without giving any of those objects a chance to clean themselves up.