2013-11-23 8 views
8

Sto giocando con un event aggregator utilizzando un weak reference per il method nel mio oggetto subscriber che desidero gestire l'evento.WeakReference is dead

Quando subscribing il weak reference viene creato correttamente e la mia collezione subscribers si aggiorna di conseguenza. Quando provo a publish un evento, tuttavia, il weak reference è stato ripulito dal GC. Qui di seguito è il mio codice:

public class EventAggregator 
{ 
    private readonly ConcurrentDictionary<Type, List<Subscriber>> subscribers = 
     new ConcurrentDictionary<Type, List<Subscriber>>(); 

    public void Subscribe<TMessage>(Action<TMessage> handler) 
    { 
     if (handler == null) 
     { 
      throw new ArgumentNullException("handler"); 
     } 

     var messageType = typeof (TMessage); 
     if (this.subscribers.ContainsKey(messageType)) 
     { 
      this.subscribers[messageType].Add(new Subscriber(handler)); 
     } 
     else 
     { 
      this.subscribers.TryAdd(messageType, new List<Subscriber> {new Subscriber(handler)}); 
     } 
    } 

    public void Publish(object message) 
    { 
     if (message == null) 
     { 
      throw new ArgumentNullException("message"); 
     } 

     var messageType = message.GetType(); 
     if (!this.subscribers.ContainsKey(messageType)) 
     { 
      return; 
     } 

     var handlers = this.subscribers[messageType]; 
     foreach (var handler in handlers) 
     { 
      if (!handler.IsAlive) 
      { 
       continue; 
      } 

      var actionType = handler.GetType(); 
      var invoke = actionType.GetMethod("Invoke", new[] {messageType}); 
      invoke.Invoke(handler, new[] {message}); 
     } 
    } 

    private class Subscriber 
    { 
     private readonly WeakReference reference; 

     public Subscriber(object subscriber) 
     { 
      this.reference = new WeakReference(subscriber); 
     } 

     public bool IsAlive 
     { 
      get 
      { 
       return this.reference.IsAlive; 
      } 
     } 
    } 
} 

I subscribe e publish via:

ea.Subscribe<SomeEvent>(SomeHandlerMethod); 
ea.Publish(new SomeEvent { ... }); 

probabilmente sto facendo qualcosa di molto sciocco, che ha detto Ho difficoltà a vedere il mio errore.

risposta

16

Ci sono alcuni problemi qui (altri hanno già detto alcuni di loro già), ma quello principale è che il compilatore è la creazione di un nuovo oggetto delegato che nessuno è tenendo un forte riferimento a.Il compilatore prende

ea.Subscribe<SomeEvent>(SomeHandlerMethod); 

e inserisce la conversione delegato del caso, dando in modo efficace:

ea.Subscribe<SomeEvent>(new Action<SomeEvent>(SomeHandlerMethod)); 

Poi, più tardi questo delegato viene raccolto (c'è solo il tuo WeakReference ad esso) e la sottoscrizione è hosed.

Si hanno anche problemi di sicurezza dei thread (presumo che si stia utilizzando ConcurrentDictionary per questo scopo). In particolare, l'accesso a entrambi gli strumenti ConcurrentDictionary e non è affatto sicuro per i thread. Gli elenchi devono essere bloccati ed è necessario utilizzare correttamente ConcurrentDictionary per effettuare aggiornamenti. Ad esempio, nel codice corrente, è possibile che due thread separati si trovino nel blocco TryAdd e uno di essi non riesca a causare la perdita di una sottoscrizione.

Siamo in grado di risolvere questi problemi, ma lasciatemi delineare la soluzione. Il pattern di eventi deboli può essere difficile da implementare in .Net a causa di quelle istanze delegate generate automaticamente. Quello che farà invece è catturare lo Target del delegato in un WeakReference, se ne ha uno (potrebbe non esserlo se si tratta di un metodo statico). Quindi se il metodo è un metodo di istanza costruiremo un equivalente Delegate che non ha Target e quindi non ci sarà alcun riferimento forte.

using System.Collections.Concurrent; 
using System.Diagnostics; 

public class EventAggregator 
{ 
    private readonly ConcurrentDictionary<Type, List<Subscriber>> subscribers = 
     new ConcurrentDictionary<Type, List<Subscriber>>(); 

    public void Subscribe<TMessage>(Action<TMessage> handler) 
    { 
     if (handler == null) 
      throw new ArgumentNullException("handler"); 

     var messageType = typeof(TMessage); 
     var handlers = this.subscribers.GetOrAdd(messageType, key => new List<Subscriber>()); 
     lock(handlers) 
     { 
      handlers.Add(new Subscriber(handler)); 
     } 
    } 

    public void Publish(object message) 
    { 
     if (message == null) 
      throw new ArgumentNullException("message"); 

     var messageType = message.GetType(); 

     List<Subscriber> handlers; 
     if (this.subscribers.TryGetValue(messageType, out handlers)) 
     { 
      Subscriber[] tmpHandlers; 
      lock(handlers) 
      { 
       tmpHandlers = handlers.ToArray(); 
      } 

      foreach (var handler in tmpHandlers) 
      { 
       if (!handler.Invoke(message)) 
       { 
        lock(handlers) 
        { 
         handlers.Remove(handler); 
        } 
       } 
      } 
     } 
    } 

    private class Subscriber 
    { 
     private readonly WeakReference reference; 
     private readonly Delegate method; 

     public Subscriber(Delegate subscriber) 
     { 
      var target = subscriber.Target; 

      if (target != null) 
      { 
       // An instance method. Capture the target in a WeakReference. 
       // Construct a new delegate that does not have a target; 
       this.reference = new WeakReference(target); 
       var messageType = subscriber.Method.GetParameters()[0].ParameterType; 
       var delegateType = typeof(Action<,>).MakeGenericType(target.GetType(), messageType); 
       this.method = Delegate.CreateDelegate(delegateType, subscriber.Method); 
      } 
      else 
      { 
       // It is a static method, so there is no associated target. 
       // Hold a strong reference to the delegate. 
       this.reference = null; 
       this.method = subscriber; 
      } 

      Debug.Assert(this.method.Target == null, "The delegate has a strong reference to the target."); 
     } 

     public bool IsAlive 
     { 
      get 
      { 
       // If the reference is null it was a Static method 
       // and therefore is always "Alive". 
       if (this.reference == null) 
        return true; 

       return this.reference.IsAlive; 
      } 
     } 

     public bool Invoke(object message) 
     { 
      object target = null; 
      if (reference != null) 
       target = reference.Target; 

      if (!IsAlive) 
       return false; 

      if (target != null) 
      { 
       this.method.DynamicInvoke(target, message); 
      } 
      else 
      { 
       this.method.DynamicInvoke(message); 
      } 

      return true;     
     } 
    } 
} 

E un programma di test:

public class Program 
{ 
    public static void Main(string[] args) 
    { 
     var agg = new EventAggregator(); 
     var test = new Test(); 
     agg.Subscribe<Message>(test.Handler); 
     agg.Subscribe<Message>(StaticHandler); 
     agg.Publish(new Message() { Data = "Start test" }); 
     GC.KeepAlive(test); 

     for(int i = 0; i < 10; i++) 
     { 
      byte[] b = new byte[1000000]; // allocate some memory 
      agg.Publish(new Message() { Data = i.ToString() }); 
      Console.WriteLine(GC.CollectionCount(2)); 
      GC.KeepAlive(b); // force the allocator to allocate b (if not in Debug). 
     } 

     GC.Collect(); 
     agg.Publish(new Message() { Data = "End test" }); 
    } 

    private static void StaticHandler(Message m) 
    { 
     Console.WriteLine("Static Handler: {0}", m.Data); 
    } 
} 

public class Test 
{ 
    public void Handler(Message m) 
    { 
     Console.WriteLine("Instance Handler: {0}", m.Data); 
    } 
} 

public class Message 
{ 
    public string Data { get; set; } 
} 
2

L'oggetto delegato che avvolge SomeHandlerMethod dietro le quinte è probabilmente garbage collection tra Subscribe e Publish.

provare quanto segue:

Action<SomeEvent> action = SomeHandlerMethod; 
ea.Subscribe<SomeEvent>(SomeHandlerMethod); 
ea.Publish(new SomeEvent { ... }); 
GC.KeepAlive(action); 

Forse la vecchia sintassi è un po 'più chiaro in questo caso:

Action<SomeEvent> action = new Action<SomeEvent>(SomeHandlerMethod); 

Un'altra cosa da guardare fuori per se il codice è multithread è la condizione di competizione in cui un evento sottoscritto potrebbe non essere aggiunto (TryAdd può restituire false).

quanto per una soluzione, vedere atomaras risposta:

public void Subscribe<TMessage>(IHandle<TMessage> handler) 
{ 
[...] 

public interface IHandler<T> 
{ 
    Handle(T event); 
} 

Oppure:

public void Subscribe<TMessage>(Action<TMessage> handler) 
{ 
    [...] 
    object targetObject = handler.Target; 
    MethodInfo method = handler.Method; 
    new Subscriber(targetObject, method) 

    [...] 
    subscriber.method.Invoke(subscriber.object, new object[]{message}); 

Non so se l'oggetto MethodInfo riflessione potrebbe essere memorizzato in un WeakReference, vale a dire se si tratta di temporaneo o no, e se è memorizzato in modo sicuro referenziato se rimarrà o meno sull'assieme contenente il Tipo (se stiamo parlando di un plugin dll) ...

1

Stai passando in un'istanza di un'azione che nessuno mantiene un forte riferimento per cui è immediatamente disponibile per Garbage Collection. La tua azione mantiene comunque un forte riferimento alla tua istanza con il metodo (se non è statico).

Cosa si può fare se si desidera mantenere la stessa firma API (è possibile passare un'interfaccia IHandle anche se lo si desidera) è modificare il parametro Sottoscritto per essere un'espressione, analizzarlo e individuare l'istanza di l'oggetto Obiettivo dell'azione e invece di mantenere una Debolezza.

vedere qui come farlo Action delegate. How to get the instance that call the method