C # Eventi e sicurezza del filo

AGGIORNARE

A partire da C # 6, la risposta a questa domanda è:

SomeEvent?.Invoke(this, e); 

Spesso ascolto / leggo il seguente consiglio:

Crea sempre una copia di un evento prima di controllarlo e annullarlo. Questo eliminerà un potenziale problema con il threading in cui l’evento diventa null nel punto giusto tra il punto in cui si verifica il null e dove si triggers l’evento:

 // Copy the event delegate before checking/calling EventHandler copy = TheEvent; if (copy != null) copy(this, EventArgs.Empty); // Call any handlers on the copied list 

Aggiornato : ho pensato di leggere sull’ottimizzazione che questo potrebbe richiedere anche che il membro dell’evento sia volatile, ma Jon Skeet afferma nella sua risposta che il CLR non ottimizza la copia.

Ma nel frattempo, affinché questo problema si verifichi, un altro thread deve aver fatto qualcosa del genere:

 // Better delist from event - don't want our handler called from now on: otherObject.TheEvent -= OnTheEvent; // Good, now we can be certain that OnTheEvent will not run... 

La sequenza effettiva potrebbe essere questa miscela:

 // Copy the event delegate before checking/calling EventHandler copy = TheEvent; // Better delist from event - don't want our handler called from now on: otherObject.TheEvent -= OnTheEvent; // Good, now we can be certain that OnTheEvent will not run... if (copy != null) copy(this, EventArgs.Empty); // Call any handlers on the copied list 

Il punto è che OnTheEvent viene eseguito dopo che l’autore ha annullato l’iscrizione, eppure si sono appena annullati per evitare che ciò accada. Sicuramente ciò che è veramente necessario è un’implementazione personalizzata degli eventi con la sincronizzazione appropriata add e remove accessor. Inoltre, esiste il problema dei possibili deadlock se viene tenuto un blocco mentre viene triggersto un evento.

Quindi questa programmazione Cargo Cult ? Sembra così – molte persone devono fare questo passo per proteggere il loro codice da più thread, quando in realtà mi sembra che gli eventi richiedano molta più attenzione di questo prima che possano essere usati come parte di un design multi-thread . Di conseguenza, le persone che non stanno prendendo quell’assistenza aggiuntiva potrebbero anche ignorare questo consiglio – semplicemente non è un problema per i programmi single-threaded, e infatti, data l’assenza di volatile nella maggior parte dei codici di esempio online, il consiglio potrebbe essere avere nessun effetto.

(E non è molto più semplice assegnare semplicemente il delegate { } vuoto delegate { } sulla dichiarazione del membro in modo che non sia mai necessario controllare null in primo luogo?)

Aggiornato: nel caso in cui non fosse chiaro, ho colto l’intenzione del consiglio – per evitare un’eccezione di riferimento null in tutte le circostanze. Il mio punto è che questa particolare eccezione di riferimento null può verificarsi solo se un altro thread viene cancellato dall’evento, e l’unica ragione per farlo è assicurarsi che non vengano ricevute ulteriori chiamate tramite quell’evento, cosa che chiaramente NON viene raggiunta da questa tecnica . Dovresti hide una condizione di gara – sarebbe meglio rivelarla! Quell’eccezione nulla aiuta a rilevare un abuso del tuo componente. Se si desidera proteggere il componente dall’abuso, è ansible seguire l’esempio di WPF: memorizzare l’ID del thread nel costruttore e quindi generare un’eccezione se un altro thread tenta di interagire direttamente con il componente. Oppure implementare un componente veramente thread-safe (non un compito facile).

Quindi sostengo che il solo fatto di fare questo idioma di copia / controllo è la programmazione settoriale del carico, aggiungendo confusione e rumore al codice. Per proteggere effettivamente contro altri thread richiede molto più lavoro.

Aggiornamento in risposta ai post del blog di Eric Lippert:

Quindi c’è una cosa che mi mancava di più nei gestori di eventi: “i gestori di eventi devono essere solidi di essere chiamati anche dopo che l’evento è stato annullato”, e ovviamente dobbiamo solo preoccuparci della possibilità dell’evento delegato è null . Questo requisito sui gestori di eventi è documentato ovunque?

E così: “Ci sono altri modi per risolvere questo problema, ad esempio inizializzare il gestore per avere un’azione vuota che non viene mai rimossa, ma fare un controllo nullo è il modello standard.”

Quindi l’unico frammento residuo della mia domanda è, perché è esplicitamente nullo controllare il “modello standard”? L’alternativa, assegnando il delegato vuoto, richiede solo = delegate {} da aggiungere alla dichiarazione dell’evento, e questo elimina quelle piccole pile di cerimonie puzzole da ogni luogo in cui l’evento è stato sollevato. Sarebbe facile assicurarsi che il delegato vuoto sia economico da istanziare. O mi manca ancora qualcosa?

Sicuramente dev’essere questo (come suggeriva Jon Skeet) questo è solo un consiglio .NET 1.x che non è scomparso, come avrebbe dovuto fare nel 2005?

Il JIT non è autorizzato a eseguire l’ottimizzazione di cui si sta parlando nella prima parte, a causa della condizione. So che questo è stato sollevato come fantasma qualche tempo fa, ma non è valido. (L’ho controllato con Joe Duffy o Vance Morrison qualche tempo fa, non ricordo quale.)

Senza il modificatore volatile è ansible che la copia locale acquisita non sia aggiornata, ma questo è tutto. Non causerà una NullReferenceException .

E sì, c’è certamente una condizione di gara – ma ci sarà sempre. Supponiamo di cambiare semplicemente il codice in:

 TheEvent(this, EventArgs.Empty); 

Supponiamo ora che l’elenco di chiamate per quel delegato abbia 1000 voci. È perfettamente ansible che l’azione all’inizio della lista sia stata eseguita prima che un altro thread annulli l’iscrizione di un gestore vicino alla fine dell’elenco. Tuttavia, quel gestore verrà comunque eseguito perché sarà una nuova lista. (I delegati sono immutabili). Per quanto posso vedere, questo è inevitabile.

L’utilizzo di un delegato vuoto evita certamente il controllo di nullità, ma non corregge le condizioni della competizione. Inoltre, non garantisce di “vedere” sempre l’ultimo valore della variabile.

Vedo molte persone andare verso il metodo di estensione di fare questo …

 public static class Extensions { public static void Raise(this EventHandler handler, object sender, T args) where T : EventArgs { if (handler != null) handler(sender, args); } } 

Questo ti dà una syntax migliore per aumentare l’evento …

 MyEvent.Raise( this, new MyEventArgs() ); 

Inoltre, elimina la copia locale poiché viene catturata al momento della chiamata al metodo.

“Perché è esplicitamente nullo controllare il ‘modello standard’?”

Sospetto che il motivo per questo potrebbe essere che il controllo null è più performante.

Se si iscrive sempre un delegato vuoto ai propri eventi quando vengono creati, ci saranno alcuni costi generali:

  • Costo della costruzione del delegato vuoto.
  • Costo della costruzione di una catena di delegati per contenerlo.
  • Costo di invocare il delegato inutile ogni volta che viene sollevato l’evento.

(Si noti che i controlli dell’interfaccia utente hanno spesso un numero elevato di eventi, la maggior parte dei quali non sono mai sottoscritti. Dovendo creare un sottoscrittore fittizio per ogni evento e quindi invocarlo sarebbe probabilmente un impatto significativo sulle prestazioni.)

Ho fatto alcuni test prestazionali per vedere l’impatto dell’approccio subscribe-empty-delegate, e qui ci sono i miei risultati:

 Executing 50000000 iterations . . . OnNonThreadSafeEvent took: 432ms OnClassicNullCheckedEvent took: 490ms OnPreInitializedEvent took: 614ms <-- Subscribing an empty delegate to each event . . . Executing 50000000 iterations . . . OnNonThreadSafeEvent took: 674ms OnClassicNullCheckedEvent took: 674ms OnPreInitializedEvent took: 2041ms <-- Subscribing another empty delegate to each event . . . Executing 50000000 iterations . . . OnNonThreadSafeEvent took: 2011ms OnClassicNullCheckedEvent took: 2061ms OnPreInitializedEvent took: 2246ms <-- Done 

Si noti che per il caso di zero o di un abbonato (comune per i controlli dell'interfaccia utente, dove gli eventi sono abbondanti), l'evento preinizializzato con un delegato vuoto è notevolmente più lento (oltre 50 milioni di iterazioni ...)

Per maggiori informazioni e per il codice sorgente, visita questo post del blog sulla sicurezza del thread di invocazione di eventi .NET che ho pubblicato il giorno prima della domanda (!)

(Il mio setup di prova potrebbe essere imperfetto, quindi sentiti libero di scaricare il codice sorgente e di controllarlo tu stesso. Qualsiasi feedback è molto apprezzato.)

Ho davvero apprezzato questa lettura – non! Anche se ho bisogno che funzioni con la funzione C # chiamata eventi!

Perché non aggiustarlo nel compilatore? So che ci sono persone con SM che leggono questi post, quindi per favore non scottate!

1 – il problema Null ) Perché non rendere gli eventi essere. Empty invece di null in primo luogo? Quante righe di codice verrebbero salvate per il controllo nullo o dovendo attaccare a = delegate {} sulla dichiarazione? Lascia che il compilatore gestisca il caso Empty, IE non fa nulla! Se tutto è importante per il creatore dell’evento, possono controllare .Empty e fare qualunque cosa gli interessi! Altrimenti tutti gli assegni null / delegati aggiunti sono hack attorno al problema!

Sinceramente sono stanco di doverlo fare con ogni evento – alias codice boilerplate!

 public event Action Some; protected virtual void DoSomeEvent(string someValue) { var e = Some; // avoid race condition here! if(null != e) // avoid null condition here! e(this, someValue); } 

2 – il problema delle condizioni di gara ) Ho letto il post sul blog di Eric, sono d’accordo sul fatto che l’H (handler) dovrebbe gestire quando si discosta da solo, ma l’evento non può essere reso immutabile / thread sicuro? IE, imposta un flag di blocco sulla sua creazione, in modo tale che ogni volta che viene chiamato, blocca tutto il sottoscrittore e l’annullamento della sottoscrizione durante l’esecuzione?

Conclusione ,

Le lingue moderne non dovrebbero risolvere problemi come questi per noi?

Secondo Jeffrey Richter nel libro CLR via C # , il metodo corretto è:

 // Copy a reference to the delegate field now into a temporary field for thread safety EventHandler temp = Interlocked.CompareExchange(ref NewMail, null, null); // If any methods registered interest with our event, notify them if (temp != null) temp(this, e); 

Perché costringe una copia di riferimento. Per ulteriori informazioni, vedere la sua sezione Evento nel libro.

Ho utilizzato questo modello di progettazione per garantire che i gestori di eventi non vengano eseguiti dopo che sono stati annullati. Funziona abbastanza bene finora, anche se non ho provato alcun profilo delle prestazioni.

 private readonly object eventMutex = new object(); private event EventHandler _onEvent = null; public event EventHandler OnEvent { add { lock(eventMutex) { _onEvent += value; } } remove { lock(eventMutex) { _onEvent -= value; } } } private void HandleEvent(EventArgs args) { lock(eventMutex) { if (_onEvent != null) _onEvent(args); } } 

Al giorno d’oggi lavoro principalmente con Mono per Android, e Android non sembra gradirlo quando tenti di aggiornare una vista dopo che la sua attività è stata inviata in background.

Questa pratica non riguarda l’applicazione di un certo ordine di operazioni. Si tratta in realtà di evitare un’eccezione di riferimento null.

Il ragionamento alla base delle persone che si preoccupano dell’eccezione di riferimento nulla e non della condizione di razza richiederebbe una profonda ricerca psicologica. Penso che abbia qualcosa a che fare con il fatto che la risoluzione del problema di riferimento null è molto più semplice. Una volta risolto, appendono un grande banner “Missione compiuta” sul loro codice e decomprimono la tuta di volo.

Nota: il fixing della condizione di gara probabilmente implica l’uso di una flag track sincrona indipendentemente dal fatto che il gestore debba essere eseguito

Quindi sono un po ‘in ritardo per la festa qui. 🙂

Per quanto riguarda l’utilizzo di null anziché del modello di object nullo per rappresentare eventi senza sottoscrittori, considerare questo scenario. È necessario invocare un evento, ma la costruzione dell’object (EventArgs) non è banale, e nel caso comune il tuo evento non ha abbonati. Sarebbe utile se tu potessi ottimizzare il tuo codice per verificare se avevi degli abbonati prima di impegnarti a elaborare gli argomenti e invocare l’evento.

Con questo in mente, una soluzione è dire “bene, zero iscritti sono rappresentati da null”. Quindi esegui semplicemente il controllo null prima di eseguire l’operazione costosa. Suppongo che un altro modo per farlo sarebbe stato avere una proprietà Count sul tipo Delegate, quindi eseguiresti la costosa operazione solo se myDelegate.Count> 0. Usando una proprietà Count è un pattern piuttosto carino che risolve il problema originale di consentire l’ottimizzazione e ha anche la buona proprietà di poter essere invocato senza causare una NullReferenceException.

Tieni presente, tuttavia, che poiché i delegati sono tipi di riferimento, possono essere nulli. Forse semplicemente non c’era un buon modo per hide questo fatto sotto le copertine e supportare solo il modello di object nullo per gli eventi, quindi l’alternativa avrebbe potuto costringere gli sviluppatori a controllare sia gli abbonati nulli che quelli zero. Sarebbe ancora più brutto della situazione attuale.

Nota: questa è pura speculazione. Non sono coinvolto con i linguaggi .NET o CLR.

Con C # 6 e sopra, il codice potrebbe essere semplificato usando nuovo .? operator .? operator come in TheEvent?.Invoke(this, EventArgs.Empty);

https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-conditional-operators

per applicazioni a thread singolo, si sta correc questo non è un problema.

Tuttavia, se stai realizzando un componente che espone eventi, non c’è alcuna garanzia che un consumatore del tuo componente non abbia intenzione di passare al multithreading, nel qual caso devi prepararti al peggio.

L’utilizzo del delegato vuoto risolve il problema, ma causa anche un impatto sulle prestazioni su ogni chiamata all’evento e potrebbe avere implicazioni GC.

Hai ragione che il consumatore deve annullare l’iscrizione per far sì che ciò accada, ma se lo ha superato la copia temporanea, considera il messaggio già in transito.

Se non si utilizza la variabile temporanea e non si utilizza il delegato vuoto e si annulla l’iscrizione, si ottiene un’eccezione di riferimento null, che è fatale, quindi penso che ne valga la pena.

Non ho mai pensato che questo fosse un grosso problema, perché generalmente proteggo solo da questo tipo di potenziale cattivo threading nei metodi statici (etc) sui miei componenti riutilizzabili e non faccio eventi statici.

Sto sbagliando?

Collega tutti i tuoi eventi alla costruzione e lasciali soli. Il design della class Delegate non può gestire correttamente nessun altro utilizzo, come spiegherò nell’ultimo paragrafo di questo post.

Prima di tutto, non ha senso tentare di intercettare una notifica di un evento quando i gestori di eventi devono già prendere una decisione sincronizzata su se / come rispondere alla notifica .

Qualsiasi cosa che possa essere notificata, dovrebbe essere notificata. Se i gestori degli eventi gestiscono correttamente le notifiche (ovvero hanno accesso a uno stato di applicazione autorevole e rispondono solo se appropriato), allora sarà opportuno notificarle in qualsiasi momento e fidarsi che risponderanno correttamente.

L’unica volta in cui un gestore non deve essere informato che si è verificato un evento, è se l’evento non si è verificato in realtà! Quindi, se non si desidera che un gestore venga avvisato, interrompere la generazione degli eventi (es. Disabilitare il controllo o qualsiasi altra cosa che sia responsabile della rilevazione e della realizzazione dell’esistenza in primo luogo).

Onestamente, penso che la class dei delegati sia inviolabile. La fusione / transizione a MulticastDelegate è stato un errore enorme, perché ha effettivamente cambiato la definizione (utile) di un evento da qualcosa che accade in un singolo istante nel tempo, a qualcosa che accade in un intervallo di tempo. Tale modifica richiede un meccanismo di sincronizzazione che può logicamente comprimerlo in un singolo istante, ma MulticastDelegate manca di tale meccanismo. La sincronizzazione dovrebbe comprendere l’intero intervallo o istante in cui si verifica l’evento, in modo tale che una volta che un’applicazione decide di iniziare a gestire un evento, finisce di gestirlo completamente (a livello di transazione). Con la scatola nera che è la class ibrida MulticastDelegate / Delegate, questo è quasi imansible, quindi segui l’uso di un singolo sottoscrittore e / o implementa il tuo tipo di MulticastDelegate che ha un handle di sincronizzazione che può essere rimosso mentre la catena del gestore è essere usato / modificato Lo raccomando, perché l’alternativa sarebbe implementare la sincronizzazione / integrità transazionale in modo ridondante in tutti i gestori, il che sarebbe ridicolmente / inutilmente complesso.

Please take a look here: http://www.danielfortunov.com/software/%24daniel_fortunovs_adventures_in_software_development/2009/04/23/net_event_invocation_thread_safety This is the correct solution and should always be used instead of all other workarounds.

“You can ensure that the internal invocation list always has at least one member by initializing it with a do-nothing anonymous method. Because no external party can have a reference to the anonymous method, no external party can remove the method, so the delegate will never be null” — Programming .NET Components, 2nd Edition, by Juval Löwy

 public static event EventHandler PreInitializedEvent = delegate { }; public static void OnPreInitializedEvent(EventArgs e) { // No check required - event will never be null because // we have subscribed an empty anonymous delegate which // can never be unsubscribed. (But causes some overhead.) PreInitializedEvent(null, e); } 

I don’t believe the question is constrained to the c# “event” type. Removing that restriction, why not re-invent the wheel a bit and do something along these lines?

Raise event thread safely – best practice

  • Ability to sub/unsubscribe from any thread while within a raise (race condition removed)
  • Operator overloads for += and -= at the class level.
  • Generic caller-defined delegate

Thanks for a useful discussion. I was working on this problem recently and made the following class which is a bit slower, but allows to avoid callings to disposed objects.

The main point here is that invocation list can be modified even event is raised.

 ///  /// Thread safe event invoker ///  public sealed class ThreadSafeEventInvoker { ///  /// Dictionary of delegates ///  readonly ConcurrentDictionary delegates = new ConcurrentDictionary(); ///  /// List of delegates to be called, we need it because it is relatevely easy to implement a loop with list /// modification inside of it ///  readonly LinkedList delegatesList = new LinkedList(); ///  /// locker for delegates list ///  private readonly ReaderWriterLockSlim listLocker = new ReaderWriterLockSlim(); ///  /// Add delegate to list ///  ///  public void Add(Delegate value) { var holder = new DelegateHolder(value); if (!delegates.TryAdd(value, holder)) return; listLocker.EnterWriteLock(); delegatesList.AddLast(holder); listLocker.ExitWriteLock(); } ///  /// Remove delegate from list ///  ///  public void Remove(Delegate value) { DelegateHolder holder; if (!delegates.TryRemove(value, out holder)) return; Monitor.Enter(holder); holder.IsDeleted = true; Monitor.Exit(holder); } ///  /// Raise an event ///  ///  public void Raise(params object[] args) { DelegateHolder holder = null; try { // get root element listLocker.EnterReadLock(); var cursor = delegatesList.First; listLocker.ExitReadLock(); while (cursor != null) { // get its value and a next node listLocker.EnterReadLock(); holder = cursor.Value; var next = cursor.Next; listLocker.ExitReadLock(); // lock holder and invoke if it is not removed Monitor.Enter(holder); if (!holder.IsDeleted) holder.Action.DynamicInvoke(args); else if (!holder.IsDeletedFromList) { listLocker.EnterWriteLock(); delegatesList.Remove(cursor); holder.IsDeletedFromList = true; listLocker.ExitWriteLock(); } Monitor.Exit(holder); cursor = next; } } catch { // clean up if (listLocker.IsReadLockHeld) listLocker.ExitReadLock(); if (listLocker.IsWriteLockHeld) listLocker.ExitWriteLock(); if (holder != null && Monitor.IsEntered(holder)) Monitor.Exit(holder); throw; } } ///  /// helper class ///  class DelegateHolder { ///  /// delegate to call ///  public Delegate Action { get; private set; } ///  /// flag shows if this delegate removed from list of calls ///  public bool IsDeleted { get; set; } ///  /// flag shows if this instance was removed from all lists ///  public bool IsDeletedFromList { get; set; } ///  /// Constuctor ///  ///  public DelegateHolder(Delegate d) { Action = d; } } } 

And the usage is:

  private readonly ThreadSafeEventInvoker someEventWrapper = new ThreadSafeEventInvoker(); public event Action SomeEvent { add { someEventWrapper.Add(value); } remove { someEventWrapper.Remove(value); } } public void RaiseSomeEvent() { someEventWrapper.Raise(); } 

Test

I tested it in the following manner. I have a thread which creates and destroys objects like this:

 var objects = Enumerable.Range(0, 1000).Select(x => new Bar(foo)).ToList(); Thread.Sleep(10); objects.ForEach(x => x.Dispose()); 

In a Bar (a listener object) constructor I subscribe to SomeEvent (which is implemented as shown above) and unsubscribe in Dispose :

  public Bar(Foo foo) { this.foo = foo; foo.SomeEvent += Handler; } public void Handler() { if (disposed) Console.WriteLine("Handler is called after object was disposed!"); } public void Dispose() { foo.SomeEvent -= Handler; disposed = true; } 

Also I have couple of threads which raise event in a loop.

All these actions are performsd simultaneously: many listeners are created and destroyed and event is being fired at the same time.

If there were a race conditions I should see a message in a console, but it is empty. But if I use clr events as usual I see it full of warning messages. So, I can conclude that it is possible to implement a thread safe events in c#.

Cosa ne pensi?