ACTUALIZAR

A partir de C # 6, la respuesta a esta pregunta es:

SomeEvent?.Invoke(this, e);

Frecuentemente escucho / leo los siguientes consejos:

Siempre haga una copia de un evento antes de verificarlo nully dispararlo. Esto eliminará un problema potencial con el enhebrado donde el evento se convierte nullen la ubicación justo entre el lugar donde verifica si es nulo y el lugar donde dispara el 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

Actualizado : al leer sobre optimizaciones, pensé que esto también podría requerir que el miembro del evento sea volátil, pero Jon Skeet afirma en su respuesta que el CLR no optimiza la copia.

Pero mientras tanto, para que este problema ocurra, otro hilo debe haber hecho algo como esto:

// 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 secuencia real podría ser esta mezcla:

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

El punto es que se OnTheEventejecuta después de que el autor se ha dado de baja, y sin embargo, simplemente se dieron de baja específicamente para evitar que eso suceda. Sin duda, lo que realmente se necesita es una aplicación de eventos personalizado con la sincronización adecuada en el addy removedescriptores de acceso. Además, existe el problema de posibles puntos muertos si se mantiene un bloqueo mientras se dispara un evento.

Entonces, ¿es esta programación de Cargo Cult ? Parece así: muchas personas deben estar dando este paso para proteger su código de múltiples hilos, cuando en realidad me parece que los eventos requieren mucho más cuidado que esto antes de que puedan usarse como parte de un diseño de subprocesos múltiples . En consecuencia, las personas que no están tomando esa atención adicional podrían ignorar este consejo: simplemente no es un problema para los programas de subproceso único y, de hecho, dada la ausencia de la volatilemayoría de los códigos de ejemplo en línea, el consejo puede no tener efecto en absoluto.

(¿Y no es mucho más simple asignar el vacío delegate { }en la declaración del miembro para que nunca tenga que verificarlo nullen primer lugar?)

Actualizado:En caso de que no estuviera claro, comprendí la intención del consejo: evitar una excepción de referencia nula en todas las circunstancias. Mi punto es que esta excepción de referencia nula en particular solo puede ocurrir si otro subproceso está excluido del evento, y la única razón para hacerlo es asegurarse de que no se recibirán más llamadas a través de ese evento, lo que claramente NO se logra con esta técnica . Estaría ocultando una condición de carrera, ¡sería mejor revelarla! Esa excepción nula ayuda a detectar un abuso de su componente. Si desea que su componente esté protegido contra el abuso, puede seguir el ejemplo de WPF: almacene la ID del hilo en su constructor y luego arroje una excepción si otro hilo intenta interactuar directamente con su componente. O bien, implemente un componente verdaderamente seguro para subprocesos (no es una tarea fácil).

Por lo tanto, afirmo que simplemente hacer este idioma de copiar / verificar es programación de culto de carga, agregando desorden y ruido a su código. Para protegerse realmente contra otros hilos se requiere mucho más trabajo.

Actualización en respuesta a las publicaciones del blog de Eric Lippert:

Así que hay una cosa importante que me he perdido de los controladores de eventos: "los controladores de eventos deben ser robustos para ser llamados incluso después de que el evento se haya dado de baja", y obviamente, por lo tanto, solo debemos preocuparnos por la posibilidad del evento ser delegado null. ¿Se documenta ese requisito en los controladores de eventos en alguna parte?

Y así: "Hay otras formas de resolver este problema; por ejemplo, inicializar el controlador para que tenga una acción vacía que nunca se elimine. Pero hacer una comprobación nula es el patrón estándar".

Entonces, el fragmento restante de mi pregunta es, ¿por qué es explícita-null-check el "patrón estándar"? La alternativa, asignar el delegado vacío, solo requiere = delegate {}agregarse a la declaración del evento, y esto elimina esas pequeñas pilas de ceremonia apestosa de cada lugar donde se realiza el evento. Sería fácil asegurarse de que el delegado vacío sea barato de instanciar. ¿O todavía me falta algo?

Seguramente debe ser que (como sugirió Jon Skeet) este es solo un consejo de .NET 1.x que no se ha extinguido, como debería haberlo hecho en 2005.

El JIT no puede realizar la optimización de la que está hablando en la primera parte, debido a la condición. Sé que esto se planteó como un espectro hace un tiempo, pero no es válido. (Lo comprobé con Joe Duffy o Vance Morrison hace un tiempo; no recuerdo cuál).

Sin el modificador volátil, es posible que la copia local tomada esté desactualizada, pero eso es todo. No causará a NullReferenceException.

Y sí, ciertamente hay una condición de carrera, pero siempre la habrá. Supongamos que simplemente cambiamos el código a:

TheEvent(this, EventArgs.Empty);

Ahora suponga que la lista de invocación para ese delegado tiene 1000 entradas. Es perfectamente posible que la acción al comienzo de la lista se haya ejecutado antes de que otro hilo cancele la suscripción de un controlador cerca del final de la lista. Sin embargo, ese controlador aún se ejecutará porque será una nueva lista. (Los delegados son inmutables). Hasta donde puedo ver, esto es inevitable.

Usar un delegado vacío ciertamente evita la verificación de nulidad, pero no corrige la condición de la carrera. Tampoco garantiza que siempre "vea" el último valor de la variable.

Veo a muchas personas yendo hacia el método de extensión para hacer esto ...

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

Eso te da una sintaxis más agradable para generar el evento ...

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

Y también elimina la copia local, ya que se captura en el momento de la llamada al método.

"¿Por qué es explícita-null-check el 'patrón estándar'?"

Sospecho que la razón de esto podría ser que la verificación nula es más eficaz.

Si siempre suscribe un delegado vacío a sus eventos cuando se crean, habrá algunos gastos generales:

• Costo de construir el delegado vacío.
• Costo de construir una cadena de delegados para contenerlo.
• Costo de invocar al delegado sin sentido cada vez que se genera el evento.

(Tenga en cuenta que los controles de IU a menudo tienen una gran cantidad de eventos, la mayoría de los cuales nunca están suscritos. Tener que crear un suscriptor ficticio para cada evento y luego invocarlo probablemente sea un éxito significativo).

Hice algunas pruebas de rendimiento superficial para ver el impacto del enfoque de suscripción-vacío-delegado, y aquí están mis resultados:

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

Tenga en cuenta que para el caso de cero o uno de suscriptores (común para los controles de la interfaz de usuario, donde los eventos son abundantes), el evento preinicializado con un delegado vacío es notablemente más lento (más de 50 millones de iteraciones ...)

Para obtener más información y código fuente, visite esta publicación de blog sobre seguridad de subprocesos de invocación de eventos .NET que publiqué justo el día antes de que se hiciera esta pregunta (!)

(La configuración de mi prueba puede ser defectuosa, así que no dude en descargar el código fuente e inspeccionarlo usted mismo. Cualquier comentario es muy apreciado).

Realmente disfruté esta lectura, ¡no! ¡Aunque lo necesito para trabajar con la función C # llamada eventos!

¿Por qué no arreglar esto en el compilador? Sé que hay personas con EM que leen estas publicaciones, ¡así que por favor no lo llamen!

1 - la cuestión nula ) ¿Por qué no hacer que los eventos sean .Empty en lugar de nulos en primer lugar? ¿Cuántas líneas de código se guardarían para la verificación nula o tener que pegar una = delegate {}en la declaración? Deje que el compilador maneje el caso vacío, ¡IE no haga nada! Si todo le importa al creador del evento, ¡pueden verificar si está vacío y hacer lo que les importe! De lo contrario, todas las comprobaciones nulas / adiciones de delegados son trucos para solucionar el problema.

Honestamente, estoy cansado de tener que hacer esto con cada evento, también conocido como código repetitivo.

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 - el problema de la condición de carrera ) Leí la publicación del blog de Eric, estoy de acuerdo en que el H (controlador) debe manejar cuando se desreferencia, pero ¿no se puede hacer que el evento sea inmutable / seguro para subprocesos? IE, establezca un indicador de bloqueo en su creación, de modo que cada vez que se llame, bloquee todas las suscripciones y desuscripciones mientras se ejecuta.

conclusión ,

¿No se supone que los idiomas de hoy en día nos resuelven problemas como estos?

Con C # 6 y superior, el código podría simplificarse utilizando un nuevo ?.operador como en:

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

Aquí está la documentación de MSDN.

Según Jeffrey Richter en el libro CLR a través de C # , el método correcto es:

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

Porque obliga a una copia de referencia. Para obtener más información, consulte su sección Evento en el libro.

He estado usando este patrón de diseño para asegurar que los controladores de eventos no se ejecuten después de que se hayan dado de baja. Hasta ahora funciona bastante bien, aunque no he probado ningún perfil de rendimiento.

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

Actualmente estoy trabajando principalmente con Mono para Android, y parece que a Android no le gusta cuando intenta actualizar una Vista después de que su Actividad se haya enviado a un segundo plano.

Esta práctica no se trata de hacer cumplir un cierto orden de operaciones. En realidad se trata de evitar una excepción de referencia nula.

El razonamiento detrás de las personas que se preocupan por la excepción de referencia nula y no por la condición de la raza requeriría una investigación psicológica profunda. Creo que tiene algo que ver con el hecho de que solucionar el problema de referencia nula es mucho más fácil. Una vez que eso se arregla, cuelgan un gran cartel de "Misión cumplida" en su código y descomprimen su traje de vuelo.

Nota: la fijación de la condición de carrera probablemente implica el uso de una pista de bandera síncrona si el controlador debe ejecutarse

Así que llego un poco tarde a la fiesta aquí. :)

En cuanto al uso de nulo en lugar del patrón de objeto nulo para representar eventos sin suscriptores, considere este escenario. Debe invocar un evento, pero construir el objeto (EventArgs) no es trivial y, en el caso común, su evento no tiene suscriptores. Sería beneficioso para usted si pudiera optimizar su código para verificar si tenía algún suscriptor antes de comprometerse con el esfuerzo de procesamiento para construir los argumentos e invocar el evento.

Con esto en mente, una solución es decir "bueno, cero suscriptores está representado por nulo". Luego, simplemente realice la verificación nula antes de realizar su operación costosa. Supongo que otra forma de hacerlo habría sido tener una propiedad Count en el tipo Delegado, por lo que solo realizaría la operación costosa si myDelegate.Count> 0. El uso de una propiedad Count es un patrón bastante bueno que resuelve el problema original de permitir la optimización, y también tiene la buena propiedad de poder ser invocado sin causar una NullReferenceException.

Sin embargo, tenga en cuenta que, dado que los delegados son tipos de referencia, se les permite ser nulos. Tal vez simplemente no había una buena manera de ocultar este hecho debajo de las cubiertas y admitir solo el patrón de objeto nulo para eventos, por lo que la alternativa puede haber sido obligar a los desarrolladores a verificar tanto suscriptores nulos como cero. Eso sería aún más feo que la situación actual.

Nota: Esto es pura especulación. No estoy involucrado con los lenguajes .NET o CLR.

para aplicaciones de un solo subproceso, usted es correcto, esto no es un problema.

Sin embargo, si está creando un componente que expone eventos, no hay garantía de que un consumidor de su componente no vaya a ser multiproceso, en cuyo caso debe prepararse para lo peor.

El uso del delegado vacío resuelve el problema, pero también causa un impacto en el rendimiento en cada llamada al evento y posiblemente podría tener implicaciones de GC.

Tiene razón en que el consumidor debe darse de baja para que esto suceda, pero si supera la copia temporal, considere el mensaje que ya está en tránsito.

Si no usa la variable temporal, y no usa el delegado vacío, y alguien cancela la suscripción, obtendrá una excepción de referencia nula, que es fatal, por lo que creo que el costo vale la pena.

Realmente nunca he considerado que esto sea un gran problema porque generalmente solo protejo contra este tipo de maldad potencial de subprocesos en métodos estáticos (etc.) en mis componentes reutilizables, y no hago eventos estáticos.

¿Lo estoy haciendo mal?

Conecte todos sus eventos en la construcción y déjelos en paz. El diseño de la clase Delegate no puede manejar ningún otro uso correctamente, como explicaré en el párrafo final de esta publicación.

En primer lugar, no tiene sentido intentar interceptar una notificación de evento cuando los controladores de eventos ya deben tomar una decisión sincronizada sobre si / cómo responder a la notificación .

Cualquier cosa que pueda ser notificada, debe ser notificada. Si sus controladores de eventos manejan adecuadamente las notificaciones (es decir, tienen acceso a un estado de aplicación autorizado y responden solo cuando es apropiado), entonces estará bien notificarlos en cualquier momento y confiar en que responderán correctamente.

La única vez que un controlador no debe ser notificado de que se ha producido un evento, es si el evento no ha ocurrido. Entonces, si no desea que se notifique a un controlador, deje de generar los eventos (es decir, desactive el control o lo que sea responsable de detectar y hacer que el evento exista en primer lugar).

Honestamente, creo que la clase Delegate es insalvable. La fusión / transición a un MulticastDelegate fue un gran error, ya que efectivamente cambió la definición (útil) de un evento de algo que sucede en un solo instante en el tiempo, a algo que sucede en un intervalo de tiempo. Tal cambio requiere un mecanismo de sincronización que pueda colapsarlo lógicamente en un solo instante, pero el MulticastDelegate carece de dicho mecanismo. La sincronización debe abarcar todo el intervalo de tiempo o el instante en que tiene lugar el evento, de modo que una vez que una aplicación toma la decisión sincronizada de comenzar a manejar un evento, termina de manejarlo por completo (transaccionalmente). Con el cuadro negro que es la clase híbrida MulticastDelegate / Delegate, esto es casi imposible, así queadhiérase al uso de un suscriptor único y / o implemente su propio tipo de MulticastDelegate que tiene un controlador de sincronización que se puede extraer mientras se usa / modifica la cadena del controlador . Recomiendo esto, porque la alternativa sería implementar la sincronización / integridad transaccional de forma redundante en todos sus controladores, lo que sería ridículamente / innecesariamente complejo.

Por favor, eche un vistazo aquí: http://www.danielfortunov.com/software/%24daniel_fortunovs_adventures_in_software_development/2009/04/23/net_event_invocation_thread_safety Esta es la solución correcta y siempre debe usarse en lugar de todas las demás soluciones.

“Puede asegurarse de que la lista de invocación interna siempre tenga al menos un miembro inicializándola con un método anónimo de no hacer nada. Debido a que ninguna parte externa puede tener una referencia al método anónimo, ninguna parte externa puede eliminar el método, por lo que el delegado nunca será nulo "- Programación de componentes .NET, 2ª edición, por 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);  
}  

No creo que la pregunta esté restringida al tipo de "evento" de c #. Eliminando esa restricción, ¿por qué no reinventar un poco la rueda y hacer algo en este sentido?

Eleve la secuencia de eventos de forma segura: mejor práctica

• Posibilidad de suscribirse / darse de baja de cualquier hilo mientras está dentro de un aumento (condición de carrera eliminada)
• El operador sobrecarga para + = y - = a nivel de clase.
• Delegado genérico definido por la persona que llama

Gracias por una discusión útil. Estaba trabajando en este problema recientemente e hice la siguiente clase, que es un poco más lenta, pero permite evitar llamadas a objetos desechados.

El punto principal aquí es que la lista de invocación se puede modificar incluso si se genera un evento.

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

Y el uso es:

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

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

Prueba

Lo probé de la siguiente manera. Tengo un hilo que crea y destruye objetos como este:

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

En un Barconstructor (un objeto de escucha) me suscribo SomeEvent(que se implementa como se muestra arriba) y me doy de baja en 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;
    }

También tengo un par de hilos que generan eventos en un bucle.

Todas estas acciones se realizan simultáneamente: muchos oyentes se crean y destruyen y el evento se dispara al mismo tiempo.

Si hubiera condiciones de carrera, debería ver un mensaje en una consola, pero está vacío. Pero si uso los eventos clr como de costumbre, lo veo lleno de mensajes de advertencia. Entonces, puedo concluir que es posible implementar un evento seguro para subprocesos en c #.

¿Qué piensas?