Eventos de C # e segurança de threads

237

ATUALIZAR

A partir do C # 6, a resposta para esta pergunta é:

SomeEvent?.Invoke(this, e);

Frequentemente, ouço / leio os seguintes conselhos:

Sempre faça uma cópia de um evento antes de verificar nulle acioná-lo. Isso eliminará um possível problema com a segmentação onde o evento se torna nullno local entre o local em que você verifica nulo e o local em que o evento é disparado:

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

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

Atualizado : pensei em ler sobre otimizações que isso também pode exigir que o membro do evento seja volátil, mas Jon Skeet afirma em sua resposta que o CLR não otimiza a cópia.

Entretanto, para que esse problema ocorra, outro segmento deve ter feito algo assim:

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

A sequência real pode ser essa mistura:

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

O ponto é que OnTheEventé executado após o cancelamento da assinatura do autor e, no entanto, eles simplesmente cancelam a assinatura especificamente para evitar que isso aconteça. Certamente o que é realmente necessário é uma implementação de evento personalizada com sincronização apropriada nos acessadores adde remove. Além disso, há o problema de possíveis conflitos se um bloqueio for mantido enquanto um evento é disparado.

Então, isso é programação de culto à carga ? Parece que sim - muitas pessoas devem seguir esta etapa para proteger seu código de vários threads, quando na realidade me parece que os eventos exigem muito mais cuidado do que isso antes que possam ser usados ​​como parte de um design com vários threads . Consequentemente, as pessoas que não estão tomando esse cuidado adicional também podem ignorar esse conselho - simplesmente não é um problema para programas de thread único e, de fato, dada a ausência do volatilecódigo de exemplo on-line, o conselho pode não ter efeito em tudo.

(E não é muito mais simples atribuir o vazio delegate { }na declaração do membro para que você nunca precise procurar nullem primeiro lugar?)

Atualizada:Caso não estivesse claro, compreendi a intenção do conselho - evitar uma exceção de referência nula em todas as circunstâncias. O que quero dizer é que essa exceção de referência nula em particular só pode ocorrer se outro segmento for excluído do evento, e a única razão para isso é garantir que nenhuma outra chamada seja recebida por esse evento, o que claramente NÃO é alcançado por essa técnica. . Você estaria escondendo uma condição de corrida - seria melhor revelá-la! Essa exceção nula ajuda a detectar um abuso do seu componente. Se você deseja que seu componente seja protegido contra abuso, siga o exemplo do WPF - armazene o ID do encadeamento em seu construtor e, em seguida, lance uma exceção se outro encadeamento tentar interagir diretamente com seu componente. Ou então, implemente um componente verdadeiramente seguro para threads (não é uma tarefa fácil).

Por isso, afirmo que apenas fazer esse idioma de cópia / verificação é uma programação de cultos de carga, adicionando bagunça e ruído ao seu código. Para realmente proteger contra outros threads, é necessário muito mais trabalho.

Atualização em resposta às postagens do blog de Eric Lippert:

Portanto, havia uma coisa importante que eu tinha perdido nos manipuladores de eventos: "é necessário que os manipuladores de eventos sejam robustos mesmo após serem cancelados", e obviamente, portanto, precisamos apenas nos preocupar com a possibilidade do evento delegar ser null. Esse requisito nos manipuladores de eventos está documentado em algum lugar?

E assim: "Existem outras maneiras de resolver esse problema; por exemplo, inicializar o manipulador para ter uma ação vazia que nunca é removida. Mas fazer uma verificação nula é o padrão padrão".

Portanto, o único fragmento restante da minha pergunta é: por que o explícito-nulo-verifica o "padrão padrão"? A alternativa, atribuir o delegado vazio, exige apenas = delegate {}que seja adicionada à declaração do evento, e isso elimina aquelas pequenas pilhas de cerimônia fedida de todos os lugares onde o evento é gerado. Seria fácil garantir que o delegado vazio seja barato para instanciar. Ou ainda estou faltando alguma coisa?

Certamente deve ser que (como Jon Skeet sugeriu) este é apenas o conselho do .NET 1.x que não desapareceu, como deveria ter ocorrido em 2005?

Daniel Earwicker
fonte
4
Esta questão surgiu em uma discussão interna há algum tempo; Eu tenho a intenção de blogar isso há algum tempo. Meu post sobre o assunto está aqui: Eventos e Corridas
Eric Lippert
3
Stephen Cleary tem um artigo do CodeProject que examina esse problema e conclui um propósito geral: a solução "thread-safe" não existe. Basicamente, cabe ao invocador de eventos garantir que o delegado não seja nulo, e cabe ao manipulador de eventos poder lidar com a invocação depois de ter sido cancelada a inscrição.
Rkagerer
3
@rkagerer - na verdade, o segundo problema às vezes precisa ser tratado pelo manipulador de eventos, mesmo que os threads não estejam envolvidos. Pode acontecer se um manipulador de eventos solicitar a outro manipulador para cancelar a inscrição no evento que está sendo tratado atualmente, mas esse segundo assinante receberá o evento de qualquer maneira (porque cancelou a inscrição enquanto o tratamento estava em andamento).
Daniel Earwicker
3
Adicionar uma assinatura a um evento com zero assinantes, remover a única assinatura do evento, invocar um evento com zero assinantes e invocar um evento com exatamente um assinante, são operações muito mais rápidas do que adicionar / remover / chamar cenários envolvendo outros números de assinantes. Adicionar um delegado fictício torna mais lento o caso comum. O verdadeiro problema com o C # é que seus criadores decidiram EventName(arguments)invocar o delegado do evento incondicionalmente, em vez de apenas invocar o delegado se não for nulo (não faça nada se for nulo).
Supercat 21/05

Respostas:

100

O JIT não tem permissão para executar a otimização de que você está falando na primeira parte, devido à condição. Eu sei que isso foi criado como um espectro há um tempo atrás, mas não é válido. (Eu verifiquei com Joe Duffy ou Vance Morrison há um tempo; não me lembro qual.)

Sem o modificador volátil, é possível que a cópia local obtida esteja desatualizada, mas é tudo. Não causará a NullReferenceException.

E sim, certamente há uma condição de corrida - mas sempre haverá. Suponha que apenas alteremos o código para:

TheEvent(this, EventArgs.Empty);

Agora, suponha que a lista de chamadas para esse representante tenha 1000 entradas. É perfeitamente possível que a ação no início da lista tenha sido executada antes que outro encadeamento cancele a inscrição de um manipulador próximo ao final da lista. No entanto, esse manipulador ainda será executado porque será uma nova lista. (Os delegados são imutáveis.) Tanto quanto posso ver, isso é inevitável.

Usar um delegado vazio certamente evita a verificação de nulidade, mas não corrige a condição de corrida. Também não garante que você sempre "veja" o valor mais recente da variável.

Jon Skeet
fonte
4
A "Programação simultânea no Windows", de Joe Duffy, aborda o aspecto de otimização de JIT e modelo de memória da questão; veja code.logos.com/blog/2008/11/events_and_threads_part_4.html
Bradley Grainger
2
Aceitei isso com base no comentário sobre o conselho "padrão" ser anterior ao C # 2, e não estou ouvindo ninguém contradizer isso. A menos que seja realmente caro instanciar seus argumentos de evento, basta colocar '= delegate {}' no final da sua declaração de evento e depois chamar seus eventos diretamente como se fossem métodos; nunca atribua nulo a eles. (O outro material que trouxe para garantir que um manipulador não seja chamado após a exclusão da lista, isso era irrelevante e impossível de garantir, mesmo para um código de thread único, por exemplo, se o manipulador 1 solicitar que o manipulador 2 cancele a lista, o manipulador 2 ainda será chamado seguinte).
Daniel Earwicker
2
O único caso de problema (como sempre) são as estruturas, nas quais você não pode garantir que elas serão instanciadas com algo diferente de valores nulos em seus membros. Mas as estruturas são péssimas.
Daniel Earwicker 05/05
1
Sobre o representante vazio, consulte também esta pergunta: stackoverflow.com/questions/170907/… .
Vladimir
2
@ Tony: Ainda existe fundamentalmente uma condição de corrida entre algo que está se inscrevendo / se desinscrevendo e o delegado sendo executado. Seu código (que acabou de navegar por ele brevemente) reduz essa condição de corrida, permitindo que a assinatura / cancelamento de inscrição entre em vigor enquanto está sendo criada, mas suspeito que na maioria dos casos em que o comportamento normal não seja bom o suficiente, isso também não é.
Jon Skeet
52

Eu vejo muitas pessoas indo para o método de extensão de fazer isso ...

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

Isso fornece uma sintaxe melhor para aumentar o evento ...

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

E também elimina a cópia local, pois é capturada no momento da chamada do método.

JP Alioto
fonte
9
Gosto da sintaxe, mas sejamos claros ... ele não resolve o problema com um manipulador obsoleto sendo invocado, mesmo depois de não ter sido registrado. Isso resolve apenas o problema de desreferência nula. Enquanto eu gosto da sintaxe, questiono se é realmente melhor que: public event EventHandler <T> MyEvent = delete {}; ... MyEvent (este, novo MyEventArgs ()); Essa também é uma solução de baixo atrito que eu gosto por sua simplicidade.
617 Simon Gillbee
@ Simon Vejo pessoas diferentes dizendo coisas diferentes sobre isso. Eu testei e o que fiz indica que isso lida com o problema do manipulador nulo. Mesmo que o coletor original cancele o registro do evento após a verificação do manipulador! = Null, o evento ainda será gerado e nenhuma exceção será lançada.
JP Alioto
yup, CF esta pergunta: stackoverflow.com/questions/192980/...
Benjol
1
+1. Acabei de escrever esse método, começando a pensar em segurança de threads, fiz algumas pesquisas e me deparei com essa pergunta.
Niels van der Rest
Como isso pode ser chamado no VB.NET? Ou o 'RaiseEvent' já atende a cenários com vários threads?
35

"Por que a verificação nula explícita é o 'padrão padrão'?"

Suspeito que o motivo disso seja o fato de a verificação nula ter mais desempenho.

Se você sempre assina um representante vazio para seus eventos quando eles são criados, haverá algumas despesas gerais:

  • Custo de construção do delegado vazio.
  • Custo da construção de uma cadeia de delegados para contê-la.
  • Custo de chamar o delegado inútil toda vez que o evento é gerado.

(Observe que os controles da interface do usuário geralmente têm um grande número de eventos, a maioria dos quais nunca são registrados. Ter que criar um assinante fictício para cada evento e depois invocá-lo provavelmente seria um impacto significativo no desempenho.)

Fiz alguns testes de desempenho superficial para ver o impacto da abordagem de inscrição-vazio-delegado e aqui estão meus 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

Observe que, no caso de zero ou um assinante (comum para controles da interface do usuário, onde os eventos são abundantes), o evento pré-inicializado com um representante vazio é notavelmente mais lento (mais de 50 milhões de iterações ...)

Para obter mais informações e código-fonte, visite esta postagem no blog sobre segurança do thread de chamada de eventos .NET que publiquei apenas um dia antes da pergunta (!)

(Minha configuração de teste pode ter falhas, fique à vontade para fazer o download do código-fonte e inspecioná-lo. Qualquer comentário é muito apreciado.)

Daniel Fortunov
fonte
8
Eu acho que você fez o ponto principal no seu blog: não há necessidade de se preocupar com as implicações de desempenho até que seja um gargalo. Por que deixar o caminho feio ser o caminho recomendado? Se quiséssemos otimização prematura em vez de clareza, estaríamos usando assembler - então minha pergunta permanece, e acho que a resposta provável é que o conselho simplesmente antecede delegados anônimos e leva muito tempo para a cultura humana mudar o conselho antigo, como na famosa "história do assado".
1111 Daniel Earwicker
13
E suas cifras provam o ponto muito bem: a sobrecarga se reduz a apenas dois e meio NANOSECONDS (!!!) por evento gerado (pré-init vs. clássico-nulo). Isso seria indetectável em quase aplicativos com trabalho real a ser feito, mas, como a grande maioria do uso de eventos está em estruturas de GUI, é necessário comparar isso com o custo de repintar partes de uma tela no WinForms, etc. torna-se ainda mais invisível no dilúvio do trabalho real da CPU e à espera de recursos. Você recebe +1 de mim pelo trabalho duro, pelo menos. :)
Daniel Earwicker
1
@DanielEarwicker disse isso corretamente, você me levou a acreditar no evento público WrapperDoneHandler OnWrapperDone = (x, y) => {}; modelo.
21912 Mickey Perlstein
1
Também pode ser bom cronometrar um Delegate.Combine/ Delegate.Removepar nos casos em que o evento tem zero, um ou dois inscritos; se alguém adicionar e remover repetidamente a mesma instância delegada, as diferenças de custo entre os casos serão especialmente pronunciadas, pois Combineapresentam um comportamento rápido de caso especial quando um dos argumentos é null(basta retornar o outro) e Removeé muito rápido quando os dois argumentos são igual (basta retornar nulo).
supercat
12

Eu realmente gostei desta leitura - não! Mesmo que eu precise trabalhar com o recurso C # chamado events!

Por que não consertar isso no compilador? Eu sei que existem pessoas com esclerose múltipla que lêem essas postagens, então não chame isso!

1 - a questão Nula ) Por que não tornar os eventos vazios em vez de nulos? Quantas linhas de código seriam salvas para verificação nula ou tendo que colar um = delegate {}na declaração? Deixe o compilador lidar com o caso vazio, ou seja, não faça nada! Se tudo isso importa para o criador do evento, eles podem verificar .Empty e fazer o que quiserem! Caso contrário, todas as verificações / delegações nulas adicionadas são hacks em torno do problema!

Honestamente, estou cansado de ter que fazer isso com todos os eventos - também conhecido como código padrão!

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 - a questão das condições da corrida ) Li o post do Eric, concordo que o H (manipulador) deve lidar quando se desferencia a si próprio, mas o evento não pode ser tornado imutável / seguro para threads? IE, defina um sinalizador de bloqueio em sua criação, para que, sempre que for chamado, ele bloqueie todas as assinaturas e cancelamentos de assinatura durante a execução?

Conclusão ,

As línguas modernas não devem resolver problemas como esses para nós?

Chuck Savage
fonte
Concordado, deve haver um suporte melhor para isso no compilador. Até então, criei um aspecto PostSharp que faz isso em uma etapa pós-compilação . :)
Steven Jeuris
4
O bloqueio das solicitações de subscrição / cancelamento de assinatura de encadeamento enquanto aguarda a conclusão de código externo arbitrário é muito pior do que fazer com que os assinantes recebam eventos após o cancelamento das assinaturas, especialmente porque o último "problema" pode ser resolvido facilmente , simplesmente fazendo com que os manipuladores de eventos verifiquem um sinalizador para ver se eles ainda estão interessados ​​em receber o evento, mas os conflitos resultantes do design anterior podem ser intratáveis.
supercat
@supercat. Imo, o comentário "muito pior" depende bastante da aplicação. Quem não gostaria de um bloqueio muito rigoroso sem sinalizadores extras quando é uma opção? Um deadlock deve ocorrer apenas se o encadeamento de manipulação de eventos estiver aguardando outro encadeamento (que está se inscrevendo / cancelando a inscrição), pois os bloqueios são reentrantes no mesmo encadeamento e uma assinatura / cancelamento de inscrição no manipulador de eventos original não seria bloqueada. Se houver cross thread aguardando como parte de um manipulador de eventos que faria parte do design, eu preferiria retrabalhar. Estou vindo de um ângulo de aplicativo do lado do servidor que possui padrões previsíveis.
crokusek
2
@ crokusek: A análise necessária para provar que um sistema está livre de impasse é fácil se não houver ciclos em um gráfico direcionado conectando cada bloqueio a todos os bloqueios que possam ser necessários enquanto estiver em espera [a falta de ciclos comprova o sistema sem impasse]. Permitir que código arbitrário seja chamado enquanto um bloqueio é mantido criará uma borda no gráfico "pode ​​ser necessário" desse bloqueio para qualquer bloqueio que o código arbitrário possa adquirir (nem todos os bloqueios do sistema, mas não muito longe dele) ) A conseqüente existência de ciclos não implicaria que um impasse ocorresse, mas ... #
21713
1
... aumentaria bastante o nível de análise necessário para provar que não podia.
Supercat 21/13
8

Com o C # 6 e superior, o código pode ser simplificado usando o novo ?.operador, como em:

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

Aqui está a documentação do MSDN.

Phil1970
fonte
5

De acordo com Jeffrey Richter no livro CLR via C # , o método correto é:

// 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 força uma cópia de referência. Para mais informações, consulte a seção Evento no livro.

alyx
fonte
Pode estar faltando smth, mas Interlocked.CompareExchange lança NullReferenceException se o primeiro argumento for nulo, exatamente o que queremos evitar. msdn.microsoft.com/pt-br/library/bb297966.aspx
Kniganapolke
1
Interlocked.CompareExchangefalharia se, de alguma forma, fosse passado um nulo ref, mas isso não é o mesmo que ser passado a refpara um local de armazenamento (por exemplo NewMail) que exista e que inicialmente contenha uma referência nula.
Super28 28/02
4

Eu tenho usado esse padrão de design para garantir que os manipuladores de eventos não sejam executados depois de serem desinscritos. Até agora, está funcionando muito bem, embora eu não tenha experimentado nenhum perfil de desempenho.

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

Atualmente, estou trabalhando com o Mono para Android atualmente, e o Android parece não gostar quando você tenta atualizar uma Visualização depois que sua Atividade foi enviada para segundo plano.

Cinza
fonte
Na verdade, eu vejo alguém está usando um padrão muito semelhante aqui: stackoverflow.com/questions/3668953/...
Ash
2

Esta prática não é sobre impor uma certa ordem de operações. Na verdade, trata-se de evitar uma exceção de referência nula.

O raciocínio por trás das pessoas que se preocupam com a exceção de referência nula e não com a condição de raça exigiria alguma pesquisa psicológica profunda. Eu acho que tem algo a ver com o fato de que corrigir o problema de referência nula é muito mais fácil. Uma vez consertado, eles penduram um grande banner "Missão Cumprida" em seu código e descompactam seu traje de voo.

Nota: a correção da condição de corrida provavelmente envolve o uso de uma faixa de sinalização síncrona se o manipulador deve executar

dss539
fonte
Não estou pedindo uma solução para esse problema. Estou me perguntando por que existem conselhos amplos para espalhar uma bagunça extra de código em torno do disparo de eventos, quando ele apenas evita uma exceção nula quando há uma condição de corrida difícil de detectar, que ainda existirá.
21119 Daniel Earwicker
1
Bem, esse foi o meu ponto. Eles não se importam com a condição da corrida. Eles se preocupam apenas com a exceção de referência nula. Vou editar isso na minha resposta.
Dss539
E o que quero dizer é: por que faria sentido se importar com a exceção de referência nula e ainda não se importar com a condição de corrida?
21119 Daniel Earwicker
4
Um manipulador de eventos escrito corretamente deve estar preparado para lidar com o fato de que qualquer solicitação específica para gerar um evento cujo processamento possa se sobrepor a uma solicitação para adicioná-lo ou removê-lo, pode ou não acabar gerando o evento que estava sendo adicionado ou removido. A razão pela qual os programadores não se importam com a condição de corrida é que, em código escrito corretamente , não importa quem ganha .
Supercat 13/12
2
@ dss539: Embora se possa projetar uma estrutura de eventos que bloqueie solicitações de cancelamento de inscrição até o término das invocações pendentes, esse design tornaria impossível para qualquer evento (mesmo algo como um Unloadevento) cancelar com segurança as assinaturas de um objeto para outros eventos. Desagradável. Melhor é simplesmente dizer que solicitações de cancelamento de inscrição de eventos farão com que os eventos sejam cancelados "eventualmente" e que os assinantes de eventos devem verificar quando são chamados se há algo útil para eles.
Super28 28/02
1

Então, estou um pouco atrasado para a festa aqui. :)

Quanto ao uso do padrão de objeto nulo, e não nulo, para representar eventos sem assinantes, considere este cenário. Você precisa invocar um evento, mas a construção do objeto (EventArgs) não é trivial e, no caso comum, seu evento não possui assinantes. Seria benéfico para você se você pudesse otimizar seu código para verificar se tinha algum assinante antes de comprometer o esforço de processamento para construir os argumentos e invocar o evento.

Com isso em mente, uma solução é dizer "bem, zero assinante é representado por nulo". Em seguida, basta executar a verificação nula antes de executar sua operação cara. Suponho que outra maneira de fazer isso seria ter uma propriedade Count no tipo Delegate, portanto, você só executaria a operação cara se myDelegate.Count> 0. O uso de uma propriedade Count é um padrão bastante agradável que resolve o problema original. de permitir a otimização e também possui a boa propriedade de poder ser chamada sem causar uma NullReferenceException.

Porém, lembre-se de que, como os delegados são tipos de referência, eles podem ser nulos. Talvez simplesmente não houvesse uma boa maneira de esconder esse fato oculto e oferecer suporte apenas ao padrão de objeto nulo para eventos, portanto a alternativa pode estar forçando os desenvolvedores a verificar se há assinantes nulos e zero. Isso seria ainda mais feio do que a situação atual.

Nota: Isso é pura especulação. Não estou envolvido com as linguagens .NET ou CLR.

Levi
fonte
Suponho que você queira dizer "o uso de delegado vazio em vez de ..." Você já pode fazer o que sugere, com um evento inicializado para o delegado vazio. O teste (MyEvent.GetInvocationList (). Length == 1) será verdadeiro se o delegado vazio inicial for a única coisa na lista. Ainda não seria necessário fazer uma cópia primeiro. Embora eu ache que o caso que você descreve seria extremamente raro de qualquer maneira.
Daniel Earwicker
Acho que estamos confundindo as idéias de delegados e eventos aqui. Se eu tenho um evento Foo na minha classe, quando usuários externos chamam MyType.Foo + = / - =, na verdade eles estão chamando os métodos add_Foo () e remove_Foo (). No entanto, quando faço referência ao Foo de dentro da classe em que está definido, na verdade estou fazendo referência direta ao delegado subjacente, não aos métodos add_Foo () e remove_Foo (). E com a existência de tipos como EventHandlerList, não há nada determinando que o delegado e o evento estejam no mesmo lugar. É isso que eu quis dizer com o parágrafo "Lembre-se" na minha resposta.
0111 Levi
(cont.) Admito que esse é um design confuso, mas a alternativa pode ter sido pior. Como no final tudo o que você tem é um delegado - você pode fazer referência direta ao delegado subjacente, pode obtê-lo de uma coleção, pode instancia-lo em tempo real - pode ter sido tecnicamente inviável oferecer suporte a algo que não seja a "verificação de nulo ".
Levi
Como estamos falando sobre o lançamento do evento, não vejo por que os acessadores adicionar / remover são importantes aqui.
Daniel Earwicker
@ Levi: Eu realmente não gosto da maneira como o C # lida com eventos. Se eu tivesse meus condutores, o delegado teria um nome diferente do evento. Fora da classe, as únicas operações permitidas no nome de um evento seriam +=e -=. Dentro da classe, as operações permitidas também incluiriam invocação (com verificação nula interna), teste nullou configuração como null. Para qualquer outra coisa, seria necessário usar o delegado cujo nome seria o nome do evento com algum prefixo ou sufixo específico.
Supercat
0

para aplicativos de encadeamento único, você está certo de que isso não é um problema.

No entanto, se você estiver criando um componente que expõe eventos, não há garantia de que um consumidor do seu componente não vá multithreading; nesse caso, você precisará se preparar para o pior.

O uso do representante vazio resolve o problema, mas também causa um impacto no desempenho em todas as chamadas para o evento e pode ter implicações no GC.

Você está certo de que o consumidor tentou cancelar a inscrição para que isso acontecesse, mas se eles passaram pela cópia temporária, considere a mensagem já em trânsito.

Se você não usa a variável temporária e o delegado vazio, e alguém cancela a inscrição, você recebe uma exceção de referência nula, que é fatal, então acho que o custo vale a pena.

Jason Coyne
fonte
0

Eu realmente nunca considerei isso um problema, porque geralmente só protejo contra esse tipo de maldade potencial de segmentação em métodos estáticos (etc) em meus componentes reutilizáveis ​​e não faço eventos estáticos.

Estou fazendo errado?

Greg D
fonte
Se você alocar uma instância de uma classe com estado mutável (campos que alteram seus valores) e permitir que vários threads acessem a mesma instância simultaneamente, sem usar o bloqueio para proteger esses campos de serem modificados por dois threads ao mesmo tempo, você estará provavelmente fazendo errado. Se todos os seus threads tiverem suas próprias instâncias separadas (compartilhando nada) ou todos os seus objetos forem imutáveis ​​(uma vez alocados, os valores de seus campos nunca mudam), provavelmente você está bem.
21119 Daniel Earwicker
Minha abordagem geral é deixar a sincronização para o chamador, exceto nos métodos estáticos. Se eu for o chamador, sincronizarei nesse nível mais alto. (com a excepção de um objecto, cujo único propósito é de manuseamento de acesso sincronizado, é claro :).)
Greg D
@ GregD, que depende de quão complexo é o método e de quais dados ele usa. se ela afeta membros internos, e você decidir executar em uma uma rosca estado / encarregado, você está em um monte de dor
Mickey Perlstein
0

Conecte todos os seus eventos na construção e deixe-os em paz. O design da classe Delegate não pode lidar com nenhum outro uso corretamente, como explicarei no parágrafo final deste post.

Primeiro, não faz sentido tentar interceptar uma notificação de evento quando seus manipuladores de eventos já devem tomar uma decisão sincronizada sobre se / como responder à notificação .

Qualquer coisa que possa ser notificada deve ser notificada. Se os manipuladores de eventos estiverem lidando adequadamente com as notificações (ou seja, eles tiverem acesso a um estado de aplicativo autorizado e responderão somente quando apropriado), será bom notificá-los a qualquer momento e confiar que eles responderão corretamente.

A única vez que um manipulador não deve ser notificado de que um evento ocorreu é se o evento de fato não ocorreu! Portanto, se você não deseja que um manipulador seja notificado, pare de gerar os eventos (ou seja, desative o controle ou o que for responsável por detectar e trazer o evento à existência em primeiro lugar).

Honestamente, acho que a classe Delegate não pode ser salva. A fusão / transição para um MulticastDelegate foi um grande erro, porque alterou efetivamente a definição (útil) de um evento de algo que acontece em um único instante no tempo, para algo que acontece em um período de tempo. Essa alteração requer um mecanismo de sincronização que possa recolhê-lo logicamente novamente em um único instante, mas o MulticastDelegate não possui esse mecanismo. A sincronização deve abranger todo o período de tempo ou instante em que o evento ocorre, para que, uma vez que um aplicativo tome a decisão sincronizada de começar a manipular um evento, ele termine de manipulá-lo completamente (transacionalmente). Com a caixa preta que é a classe híbrida MulticastDelegate / Delegate, isso é quase impossível, entãoadote o uso de um único assinante e / ou implemente seu próprio tipo de MulticastDelegate que possui um identificador de sincronização que pode ser retirado enquanto a cadeia de manipuladores estiver sendo usada / modificada . Eu recomendo isso, porque a alternativa seria implementar a sincronização / integridade transacional de forma redundante em todos os seus manipuladores, o que seria ridiculamente / desnecessariamente complexo.

Triynko
fonte
[1] Não há manipulador de eventos útil que ocorra em "um único instante no tempo". Todas as operações têm um período de tempo. Qualquer manipulador único pode ter uma sequência não trivial de etapas a serem executadas. O suporte a uma lista de manipuladores não muda nada.
Daniel Earwicker
[2] Manter um cadeado enquanto um evento é disparado é loucura total. Isso leva inevitavelmente a um impasse. A fonte retira o bloqueio A, dispara o evento, o coletor retira o bloqueio B, agora dois bloqueios são retidos. E se alguma operação em outro encadeamento resultar na remoção dos bloqueios na ordem inversa? Como essas combinações mortais podem ser descartadas quando a responsabilidade pelo bloqueio é dividida entre componentes projetados / testados separadamente (que é o ponto principal dos eventos)?
Daniel Earwicker
[3] Nenhum desses problemas reduz a utilidade generalizada dos delegados / eventos multicast normais na composição de componentes de thread único, especialmente em estruturas de GUI. Esse caso de uso cobre a grande maioria dos usos de eventos. Usar eventos de maneira livre é de valor questionável; isso não invalida, de forma alguma, seu design ou sua utilidade óbvia nos contextos em que eles fazem sentido.
Daniel Earwicker
[4] Threads + eventos síncronos são essencialmente um arenque vermelho. A comunicação assíncrona na fila é o caminho a percorrer.
Daniel Earwicker
[1] Eu não estava me referindo ao tempo medido ... eu estava falando sobre operações atômicas, que ocorrem logicamente instantaneamente ... e com isso quero dizer que nada mais envolvendo os mesmos recursos que eles estão usando pode mudar enquanto o evento está acontecendo porque é serializado com uma trava.
Triynko 28/05/09
0

Dê uma olhada aqui: http://www.danielfortunov.com/software/%24daniel_fortunovs_adventures_in_software_development/2009/04/23/net_event_invocation_thread_safety Esta é a solução correta e sempre deve ser usada em vez de todas as outras soluções alternativas.

“Você pode garantir que a lista de chamadas internas sempre tenha pelo menos um membro, inicializando-a com um método anônimo do-nothing. Como nenhuma parte externa pode ter uma referência ao método anônimo, nenhuma parte externa pode remover o método, para que o delegado nunca seja nulo ”- Programming .NET Components, 2nd Edition, 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);  
}  
Eli
fonte
0

Não acredito que a pergunta esteja restrita ao tipo de evento c #. Removendo essa restrição, por que não reinventar um pouco a roda e fazer algo nesse sentido?

Aumentar o encadeamento de eventos com segurança - práticas recomendadas

  • Capacidade de cancelar / cancelar a inscrição de qualquer segmento durante um aumento (condição de corrida removida)
  • O operador sobrecarrega para + = e - = no nível da classe.
  • Delegado definido pelo chamador genérico
crokusek
fonte
0

Obrigado por uma discussão útil. Eu estava trabalhando nesse problema recentemente e fiz a seguinte classe, que é um pouco mais lenta, mas permite evitar chamadas para objetos descartados.

O ponto principal aqui é que a lista de chamadas pode ser modificada, mesmo que o evento seja gerado.

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

E o uso é:

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

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

Teste

Eu testei da seguinte maneira. Eu tenho um thread que cria e destrói objetos como este:

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

Em um Barconstrutor (um objeto de ouvinte), assino SomeEvent(que é implementado como mostrado acima) e desinscrevo em 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;
    }

Também tenho alguns threads que aumentam o evento em um loop.

Todas essas ações são executadas simultaneamente: muitos ouvintes são criados e destruídos e o evento está sendo disparado ao mesmo tempo.

Se houvesse condições de corrida, eu deveria ver uma mensagem no console, mas ela está vazia. Mas se eu usar eventos clr como de costume, eu o vejo cheio de mensagens de aviso. Portanto, posso concluir que é possível implementar eventos seguros de thread em c #.

O que você acha?

Tony
fonte
Parece bom o suficiente para mim. Embora eu ache que seja possível (em teoria) disposed = trueacontecer antes foo.SomeEvent -= Handlerna sua aplicação de teste, produzindo um falso positivo. Além disso, há algumas coisas que você pode querer mudar. Você realmente deseja usar try ... finallyos bloqueios - isso ajudará a tornar isso não apenas seguro para threads, mas também seguro para abortar. Sem mencionar que você poderia se livrar desse tolo try catch. E você não está verificando se o delegado passou em Add/ Remove- poderia ser null(você deve jogar imediatamente em Add/ Remove).
Luaan