METTRE À JOUR

Depuis C # 6, la réponse à cette question est:

SomeEvent?.Invoke(this, e);

J'entends / lis fréquemment les conseils suivants:

Faites toujours une copie d'un événement avant de le vérifier nullet de le déclencher. Cela éliminera un problème potentiel avec le filetage où l'événement se situe nullà l'emplacement exact entre l'endroit où vous vérifiez la valeur null et l'endroit où vous déclenchez l'événement:

// Copy the event delegate before checking/calling
EventHandler copy = TheEvent;

if (copy != null)
    copy(this, EventArgs.Empty); // Call any handlers on the copied list

Mise à jour : J'ai pensé à la lecture des optimisations que cela pourrait également nécessiter la volatilité du membre de l'événement, mais Jon Skeet déclare dans sa réponse que le CLR n'optimise pas la copie.

Mais en attendant, pour que ce problème se produise, un autre thread doit avoir fait quelque chose comme ceci:

// 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 séquence réelle pourrait être ce mélange:

// 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

Le fait étant que OnTheEventl'auteur s'exécute après sa désinscription, et pourtant ils se sont simplement désabonnés spécifiquement pour éviter que cela ne se produise. Ce qui est vraiment nécessaire, c'est une implémentation d'événements personnalisée avec une synchronisation appropriée dans les accesseurs addet remove. De plus, il y a le problème des interblocages possibles si un verrou est maintenu pendant le déclenchement d'un événement.

Alors est - ce Programming Cargo Cult ? Il semble que de cette façon - beaucoup de gens doivent prendre cette mesure pour protéger leur code de plusieurs threads, alors qu'en réalité il me semble que les événements nécessitent beaucoup plus de soin que cela avant de pouvoir être utilisés dans le cadre d'une conception multi-thread . Par conséquent, les personnes qui ne prennent pas ces précautions supplémentaires pourraient tout aussi bien ignorer ce conseil - ce n'est tout simplement pas un problème pour les programmes à un seul thread, et en fait, étant donné l'absence de la volatileplupart des exemples de code en ligne, le conseil peut ne pas avoir effet du tout.

(Et n'est-il pas beaucoup plus simple d'affecter simplement le vide delegate { }sur la déclaration de membre afin que vous n'ayez jamais besoin de vérifier nullen premier lieu?)

Actualisé:Au cas où ce ne serait pas clair, j'ai bien compris l'intention de l'avis - éviter une exception de référence nulle en toutes circonstances. Mon point est que cette exception de référence nulle particulière ne peut se produire que si un autre thread est retiré de l'événement, et la seule raison de le faire est de s'assurer qu'aucun autre appel ne sera reçu via cet événement, ce qui n'est clairement PAS atteint par cette technique. . Vous cacheriez une condition de course - il vaudrait mieux la révéler! Cette exception nulle permet de détecter un abus de votre composant. Si vous souhaitez que votre composant soit protégé contre les abus, vous pouvez suivre l'exemple de WPF - stocker l'ID de thread dans votre constructeur, puis lever une exception si un autre thread tente d'interagir directement avec votre composant. Ou bien implémentez un composant vraiment thread-safe (pas une tâche facile).

Je soutiens donc que le simple fait de faire cet idiome de copie / vérification est une programmation culte, ajoutant du désordre et du bruit à votre code. Pour se protéger contre d'autres threads, il faut beaucoup plus de travail.

Mise à jour en réponse aux articles de blog d'Eric Lippert:

Il y a donc une chose importante que j'ai manquée à propos des gestionnaires d'événements: "les gestionnaires d'événements doivent être robustes face à être appelés même après que l'événement a été désabonné", et évidemment, nous n'avons donc qu'à nous soucier de la possibilité de l'événement être délégué null. Cette exigence sur les gestionnaires d'événements est-elle documentée n'importe où?

Et donc: "Il existe d'autres façons de résoudre ce problème; par exemple, initialiser le gestionnaire pour avoir une action vide qui n'est jamais supprimée. Mais faire une vérification nulle est le modèle standard."

Donc, le dernier fragment de ma question est, pourquoi est-null-check explicite le "modèle standard"? L'alternative, attribuer le délégué vide, ne nécessite que = delegate {}d'être ajoutée à la déclaration de l'événement, ce qui élimine ces petits tas de cérémonie puante de chaque endroit où l'événement est déclenché. Il serait facile de s'assurer que le délégué vide est bon marché à instancier. Ou est-ce que je manque encore quelque chose?

Il doit sûrement s'agir (comme l'a suggéré Jon Skeet) d'un conseil .NET 1.x qui n'a pas disparu, comme il aurait dû le faire en 2005?

Le JIT n'est pas autorisé à effectuer l'optimisation dont vous parlez dans la première partie, en raison de la condition. Je sais que cela a été soulevé comme un spectre il y a quelque temps, mais ce n'est pas valable. (Je l'ai vérifié avec Joe Duffy ou Vance Morrison il y a quelque temps; je ne me souviens plus lequel.)

Sans le modificateur volatile, il est possible que la copie locale prise soit obsolète, mais c'est tout. Cela ne provoquera pas de NullReferenceException.

Et oui, il y a certainement une condition de course - mais il y en aura toujours. Supposons que nous changions simplement le code en:

TheEvent(this, EventArgs.Empty);

Supposons maintenant que la liste d'appels de ce délégué comporte 1 000 entrées. Il est parfaitement possible que l'action au début de la liste se soit exécutée avant qu'un autre thread ne désabonne un gestionnaire vers la fin de la liste. Cependant, ce gestionnaire sera toujours exécuté car il s'agira d'une nouvelle liste. (Les délégués sont immuables.) Autant que je sache, cela est inévitable.

L'utilisation d'un délégué vide évite certainement le contrôle de nullité, mais ne résout pas la condition de concurrence. Cela ne garantit pas non plus que vous "voyez" toujours la dernière valeur de la variable.

Je vois beaucoup de gens se diriger vers la méthode d'extension pour ce faire ...

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

Cela vous donne une syntaxe plus agréable pour déclencher l'événement ...

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

Et supprime également la copie locale car elle est capturée au moment de l'appel de méthode.

"Pourquoi explicit-null-check le 'modèle standard'?"

Je soupçonne que cela pourrait être dû au fait que le contrôle nul est plus performant.

Si vous abonnez toujours un délégué vide à vos événements lors de leur création, il y aura des frais généraux:

• Coût de construction du délégué vide.
• Coût de construction d'une chaîne de délégués pour la contenir.
• Coût d'invoquer le délégué inutile à chaque fois que l'événement est déclenché.

(Notez que les contrôles d'interface utilisateur comportent souvent un grand nombre d'événements, dont la plupart ne sont jamais abonnés. Le fait de créer un abonné factice pour chaque événement, puis de l'invoquer serait probablement un impact significatif sur les performances.)

J'ai fait quelques tests de performances superficiels pour voir l'impact de l'approche subscribe-empty-delegate, et voici mes résultats:

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

Notez que pour le cas de zéro ou un abonné (courant pour les contrôles UI, où les événements sont nombreux), l'événement pré-initialisé avec un délégué vide est notamment plus lent (plus de 50 millions d'itérations ...)

Pour plus d'informations et le code source, visitez cet article de blog sur la sécurité des threads d'invocation d'événements .NET que j'ai publié la veille de la question (!)

(Ma configuration de test peut être défectueuse, alors n'hésitez pas à télécharger le code source et à l'inspecter vous-même. Tout commentaire est très apprécié.)

J'ai vraiment apprécié cette lecture - pas! Même si j'en ai besoin pour travailler avec la fonctionnalité C # appelée événements!

Pourquoi ne pas corriger cela dans le compilateur? Je sais qu'il y a des personnes atteintes de sclérose en plaques qui lisent ces messages, alors s'il vous plaît, ne flambez pas!

1 - le problème Null ) Pourquoi ne pas faire en sorte que les événements soient .Empty au lieu de null en premier? Combien de lignes de code seraient enregistrées pour une vérification nulle ou pour coller un = delegate {}sur la déclaration? Laissez le compilateur gérer le cas vide, IE ne fait rien! Si tout cela est important pour le créateur de l'événement, ils peuvent vérifier .Vider et faire ce qu'ils veulent avec! Sinon, toutes les vérifications nulles / ajouts de délégués sont des hacks autour du problème!

Honnêtement, je suis fatigué d'avoir à le faire avec chaque événement - aka code passe-partout!

public event Action<thisClass, string> 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 - le problème de la condition de concurrence ) J'ai lu le blog d'Eric, je suis d'accord que le H (gestionnaire) devrait gérer quand il se déréférence, mais l'événement ne peut-il pas être rendu immuable / thread-safe? IE, définir un indicateur de verrouillage lors de sa création, de sorte que chaque fois qu'il est appelé, il verrouille tous les abonnements et désabonnements pendant son exécution?

Conclusion ,

Les langues modernes ne sont-elles pas censées résoudre de tels problèmes pour nous?

Avec C # 6 et supérieur, le code pourrait être simplifié en utilisant un nouvel ?.opérateur comme dans:

TheEvent?.Invoke(this, EventArgs.Empty);

Voici la documentation MSDN.

Selon Jeffrey Richter dans le livre CLR via C # , la bonne méthode est:

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

Parce que cela force une copie de référence. Pour plus d'informations, consultez sa section Événement dans le livre.

J'ai utilisé ce modèle de conception pour garantir que les gestionnaires d'événements ne sont pas exécutés après leur désinscription. Cela fonctionne assez bien jusqu'à présent, même si je n'ai pas essayé de profilage des performances.

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);
  }
}

Je travaille principalement avec Mono pour Android ces jours-ci, et Android ne semble pas l'aimer lorsque vous essayez de mettre à jour une vue après que son activité a été envoyée en arrière-plan.

Cette pratique ne consiste pas à faire respecter un certain ordre d'opérations. Il s'agit en fait d'éviter une exception de référence nulle.

Le raisonnement derrière les personnes se souciant de l'exception de référence nulle et non de la condition de race nécessiterait une recherche psychologique approfondie. Je pense que cela a quelque chose à voir avec le fait que la résolution du problème de référence nulle est beaucoup plus facile. Une fois que cela est résolu, ils accrochent une grande bannière "Mission accomplie" sur leur code et décompressent leur combinaison de vol.

Remarque: la correction de la condition de concurrence implique probablement l'utilisation d'une piste de drapeau synchrone si le gestionnaire doit s'exécuter

Je suis donc un peu en retard à la fête ici. :)

En ce qui concerne l'utilisation de null plutôt que le modèle d'objet null pour représenter des événements sans abonnés, envisagez ce scénario. Vous devez appeler un événement, mais la construction de l'objet (EventArgs) n'est pas anodine et, dans le cas courant, votre événement n'a pas d'abonné. Il serait avantageux pour vous que vous puissiez optimiser votre code pour vérifier si vous avez des abonnés avant de vous engager à traiter la construction des arguments et à invoquer l'événement.

Dans cet esprit, une solution consiste à dire "eh bien, zéro abonné est représenté par null". Ensuite, effectuez simplement la vérification nulle avant d'effectuer votre opération coûteuse. Je suppose qu'une autre façon de procéder aurait été d'avoir une propriété Count sur le type Delegate, donc vous ne feriez que l'opération coûteuse si myDelegate.Count> 0. L'utilisation d'une propriété Count est un modèle assez sympa qui résout le problème d'origine d'autoriser l'optimisation, et il a également la belle propriété de pouvoir être invoqué sans provoquer une exception NullReferenceException.

Gardez à l'esprit, cependant, que puisque les délégués sont des types de référence, ils peuvent être nuls. Peut-être qu'il n'y avait tout simplement pas de bon moyen de cacher ce fait sous les couvertures et de ne prendre en charge que le modèle d'objet null pour les événements, donc l'alternative pourrait avoir forcé les développeurs à vérifier à la fois pour les abonnés null et pour zéro. Ce serait encore plus laid que la situation actuelle.

Remarque: Il s'agit de pure spéculation. Je ne suis pas impliqué avec les langages .NET ou CLR.

pour les applicaitons à filetage unique, vous êtes correct ce n'est pas un problème.

Cependant, si vous créez un composant qui expose des événements, rien ne garantit qu'un consommateur de votre composant n'ira pas en multithreading, auquel cas vous devez vous préparer au pire.

L'utilisation du délégué vide résout le problème, mais entraîne également un impact sur les performances à chaque appel à l'événement et peut éventuellement avoir des implications pour le GC.

Vous avez raison de dire que le consommateur essaie de se désabonner pour que cela se produise, mais s'il a dépassé la copie temporaire, alors considérez le message déjà en transit.

Si vous n'utilisez pas la variable temporaire, n'utilisez pas le délégué vide et que quelqu'un se désabonne, vous obtenez une exception de référence nulle, ce qui est fatal, donc je pense que le coût en vaut la peine.

Je n'ai jamais vraiment considéré cela comme un problème, car je ne protège généralement que contre ce type de problème de threading potentiel dans les méthodes statiques (etc.) sur mes composants réutilisables, et je ne crée pas d'événements statiques.

Suis-je en train de mal faire?

Câblez tous vos événements à la construction et laissez-les tranquilles. La conception de la classe Delegate ne peut pas gérer correctement toute autre utilisation correctement, comme je l'expliquerai dans le dernier paragraphe de cet article.

Tout d'abord, il est inutile d'essayer d' intercepter une notification d' événement lorsque vos gestionnaires d' événements doivent déjà prendre une décision synchronisée quant à savoir / comment répondre à la notification .

Tout ce qui peut être notifié doit être notifié. Si vos gestionnaires d'événements gèrent correctement les notifications (c'est-à-dire qu'ils ont accès à un état d'application faisant autorité et ne répondent que lorsque cela est approprié), alors il sera bon de les informer à tout moment et de croire qu'ils répondront correctement.

La seule fois où un gestionnaire ne doit pas être averti qu'un événement s'est produit, c'est si l'événement en fait ne s'est pas produit! Donc, si vous ne voulez pas qu'un gestionnaire soit averti, arrêtez de générer les événements (c'est-à-dire désactivez le contrôle ou tout ce qui est responsable de la détection et de la mise en place de l'événement en premier lieu).

Honnêtement, je pense que la classe Délégué est irréversible. La fusion / transition vers un MulticastDelegate était une énorme erreur, car elle a effectivement changé la définition (utile) d'un événement de quelque chose qui se produit à un seul instant dans le temps, à quelque chose qui se produit sur une période de temps. Un tel changement nécessite un mécanisme de synchronisation qui peut logiquement le réduire en un seul instant, mais le MulticastDelegate ne dispose d'aucun de ces mécanismes. La synchronisation doit englober l'intégralité de la période ou l'instant où l'événement se produit, de sorte qu'une fois qu'une application prend la décision synchronisée de commencer à gérer un événement, elle termine de le gérer complètement (de manière transactionnelle). Avec la boîte noire qui est la classe hybride MulticastDelegate / Delegate, cela est presque impossible, doncadhérer à l'utilisation d'un seul abonné et / ou implémenter votre propre type de MulticastDelegate qui a une poignée de synchronisation qui peut être supprimée pendant que la chaîne de gestionnaire est utilisée / modifiée . Je recommande cela, car l'alternative serait d'implémenter la synchronisation / l'intégrité transactionnelle de manière redondante dans tous vos gestionnaires, ce qui serait ridiculement / inutilement complexe.

Veuillez jeter un œil ici: http://www.danielfortunov.com/software/%24daniel_fortunovs_adventures_in_software_development/2009/04/23/net_event_invocation_thread_safety Il s'agit de la bonne solution et doit toujours être utilisée à la place de toutes les autres solutions.

«Vous pouvez vous assurer que la liste d'invocation interne a toujours au moins un membre en l'initialisant avec une méthode anonyme de ne rien faire. Puisqu'aucune partie externe ne peut avoir de référence à la méthode anonyme, aucune partie externe ne peut supprimer la méthode, donc le délégué ne sera jamais nul »- Programmation des composants .NET, 2e édition, par Juval Löwy

public static event EventHandler<EventArgs> 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);  
}  

Je ne crois pas que la question soit limitée au type "événement" c #. Supprimer cette restriction, pourquoi ne pas réinventer un peu la roue et faire quelque chose dans ce sens?

Soulever le fil de l'événement en toute sécurité - meilleure pratique

• Possibilité de se souscrire / se désinscrire de n'importe quel thread pendant une relance (condition de course supprimée)
• Surcharge de l'opérateur pour + = et - = au niveau de la classe.
• Délégué générique défini par l'appelant

Merci pour une discussion utile. Je travaillais sur ce problème récemment et j'ai créé la classe suivante qui est un peu plus lente, mais permet d'éviter les appels aux objets supprimés.

Le point principal ici est que la liste d'invocation peut être modifiée même si l'événement est déclenché.

/// <summary>
/// Thread safe event invoker
/// </summary>
public sealed class ThreadSafeEventInvoker
{
    /// <summary>
    /// Dictionary of delegates
    /// </summary>
    readonly ConcurrentDictionary<Delegate, DelegateHolder> delegates = new ConcurrentDictionary<Delegate, DelegateHolder>();

    /// <summary>
    /// List of delegates to be called, we need it because it is relatevely easy to implement a loop with list
    /// modification inside of it
    /// </summary>
    readonly LinkedList<DelegateHolder> delegatesList = new LinkedList<DelegateHolder>();

    /// <summary>
    /// locker for delegates list
    /// </summary>
    private readonly ReaderWriterLockSlim listLocker = new ReaderWriterLockSlim();

    /// <summary>
    /// Add delegate to list
    /// </summary>
    /// <param name="value"></param>
    public void Add(Delegate value)
    {
        var holder = new DelegateHolder(value);
        if (!delegates.TryAdd(value, holder)) return;

        listLocker.EnterWriteLock();
        delegatesList.AddLast(holder);
        listLocker.ExitWriteLock();
    }

    /// <summary>
    /// Remove delegate from list
    /// </summary>
    /// <param name="value"></param>
    public void Remove(Delegate value)
    {
        DelegateHolder holder;
        if (!delegates.TryRemove(value, out holder)) return;

        Monitor.Enter(holder);
        holder.IsDeleted = true;
        Monitor.Exit(holder);
    }

    /// <summary>
    /// Raise an event
    /// </summary>
    /// <param name="args"></param>
    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;
        }
    }

    /// <summary>
    /// helper class
    /// </summary>
    class DelegateHolder
    {
        /// <summary>
        /// delegate to call
        /// </summary>
        public Delegate Action { get; private set; }

        /// <summary>
        /// flag shows if this delegate removed from list of calls
        /// </summary>
        public bool IsDeleted { get; set; }

        /// <summary>
        /// flag shows if this instance was removed from all lists
        /// </summary>
        public bool IsDeletedFromList { get; set; }

        /// <summary>
        /// Constuctor
        /// </summary>
        /// <param name="d"></param>
        public DelegateHolder(Delegate d)
        {
            Action = d;
        }
    }
}

Et l'utilisation est:

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

    public void RaiseSomeEvent()
    {
        someEventWrapper.Raise();
    }

Tester

Je l'ai testé de la manière suivante. J'ai un fil qui crée et détruit des objets comme celui-ci:

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

Dans un constructeur Bar(un objet écouteur) SomeEventauquel je m'abonne (qui est implémenté comme indiqué ci-dessus) et me désabonne dans 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;
    }

J'ai aussi quelques threads qui déclenchent un événement en boucle.

Toutes ces actions sont effectuées simultanément: de nombreux auditeurs sont créés et détruits et l'événement est déclenché en même temps.

S'il y avait des conditions de course, je devrais voir un message dans une console, mais il est vide. Mais si j'utilise les événements clr comme d'habitude, je le vois plein de messages d'avertissement. Donc, je peux conclure qu'il est possible d'implémenter des événements thread-safe en c #.

Qu'est-ce que tu penses?