224

更新

C#6では、この質問に対する答えは次のとおりです。

SomeEvent?.Invoke(this, e);

私は以下のアドバイスをよく聞いたり読んだりします。

確認する前に、必ずイベントのコピーを作成してください。nullそしてそれを発射する。これにより、イベントが発生した場所でのスレッド処理に関する潜在的な問題が解消されます。nullnullをチェックした場所とイベントを発生させた場所の間の場所。

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

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

更新しました:私は最適化について読んだことから、これにはイベントのメンバーも不安定である必要があるかもしれないと考えましたが、Jon Skeetは彼の答えでCLRはコピーを最適化しないと述べています。

しかしその間も、この問題が発生するには、別のスレッドが次のようなことをしている必要があります。

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

実際のシーケンスは次のようになります。

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

それがポイントOnTheEvent作者が購読を中止した後に実行されます、それでも彼らは単にその事態を回避するために購読を中止します。きっと本当に必要とされているのは、適切な同期を備えたカスタムイベントの実装です。addそしてremoveアクセサまた、イベントが発生している間にロックが保持されると、デッドロックが発生する可能性があるという問題もあります。

だからこれはカーゴカルトプログラミング?マルチスレッド設計の一部として使用するには、実際にはイベントにこれ以上の注意が必要であると思われるとき、多くの人が自分のコードをマルチスレッドから保護するためにこのステップを踏まなければなりません。 。そのため、それ以上の注意を払っていない人はこのアドバイスを無視しているかもしれません - それは単にシングルスレッドプログラムにとっては問題ではありません。volatileほとんどのオンラインのサンプルコードでは、アドバイスはまったく効果がないかもしれません。

(そして、空を代入するだけのほうがずっと簡単ではありません。delegate { }あなたがチェックする必要がないように、メンバー宣言にnullそもそも?)

更新しました:明確ではない場合、私はアドバイスの意図を握りました - あらゆる状況下でnull参照例外を避けるためです。私の言いたいことは、この特定のnull参照例外は、他のスレッドがイベントから外れている場合にのみ発生する可能性があり、それを行う唯一の理由はそのイベントを通じてそれ以上の呼び出しを受け取らないことです。 。あなたは競合状態を隠していることでしょう - それを明らかにした方がいいでしょう!このnull例外は、コンポーネントの悪用を検出するのに役立ちます。コンポーネントを悪用から保護するには、WPFの例に従って、コンストラクタにスレッドIDを格納し、別のスレッドがコンポーネントと直接対話しようとすると例外をスローします。あるいは、本当にスレッドセーフなコンポーネントを実装する(簡単な作業ではありません)。

そのため、このコピー/チェックの慣用句を実行するだけではカーゴ・カルト・プログラミングになり、コードに混乱とノイズが加わることになります。他のスレッドから実際に保護するには、さらに多くの作業が必要です。

Eric Lippertのブログ投稿に応えて更新します。

イベントハンドラについて見逃していた大きなことがあります。「イベントハンドラは、登録解除された後も呼び出されることに対応するために堅牢である必要があります」、したがって、明らかにイベントの可能性についてだけ注意する必要があります。代理人nullイベントハンドラに関するその要件はどこかに文書化されていますか?

「この問題を解決する方法は他にもあります。たとえば、ハンドラを初期化して空のアクションが削除されることはありません。ただし、nullチェックを行うのが標準的なパターンです。」

だから私の質問の残りの部分は、「標準パターン」を明示的にnullチェックするのはなぜですか。空のデリゲートを代入する代案は、ただ必要とするだけです。= delegate {}イベント宣言に追加され、これはイベントが発生するすべての場所から臭い式のそれらの小さな山を排除します。空のデリゲートをインスタンス化するのに安価であることを確認するのは簡単です。それとも私はまだ何かが足りない?

(Jon Skeetが示唆しているように)これは.NET 1.xのアドバイスに過ぎず、2005年には行われるはずだったのでしょうか。


  • この質問は、しばらく前に社内で議論されました。私は今しばらくの間これをブログにするつもりです。この件に関する私の投稿はここにあります:イベントとレース - Eric Lippert
  • スティーブンクリアリーはCodeProjectの記事これはこの問題を検討し、彼は「スレッドセーフ」という一般的な目的を締めくくりました。解決策は存在しません。基本的に、デリゲートがnullではないことを確認するのはイベントの呼び出し側に任されており、購読が解除された後に呼び出されることを処理できるようにするのはイベントハンドラに任されています。 - rkagerer
  • @rkagerer - 実際には2番目の問題は、スレッドが関係していなくてもイベントハンドラで対処しなければならないことがあります。 1つのイベントハンドラが指示した場合に起こる可能性があります別の現在処理中のイベントの購読を中止するためのハンドラですが、その2番目のサブスクライバはいずれにしてもイベントを受信します(処理の進行中に購読を中止したため)。 - Daniel Earwicker
  • 0人の購読者を持つイベントへの購読の追加、そのイベントの唯一の購読の削除、0人の購読者を持つイベントの呼び出し、厳密に1人の購読者を持つイベントの呼び出しはすべて他の数のシナリオを含む追加/削除/呼び出しシナリオよりはるかに高速な操作です。購読者ダミーデリゲートを追加すると、一般的なケースが遅くなります。 C#の本当の問題は、その作成者が持つことに決めたことです。EventName(arguments)null以外の場合にのみデリゲートを呼び出すようにするのではなく、無条件でイベントのデリゲートを呼び出す(nullの場合は何もしない)。 - supercat

15 답변


97

条件のため、JITはあなたが最初の部分で話している最適化を実行することを許可されていません。私はこれがしばらく前に幽霊として提起されたことを知っています、しかしそれは有効ではありません。 (私はしばらく前にJoe DuffyかVance Morrisonのどちらかでそれをチェックした。どちらを覚えているのかわからない。)

volatile修飾子がないと、取得したローカルコピーが古くなる可能性がありますが、それだけです。それは引き起こしませんNullReferenceException

確かに、競合状態があります - しかし、常にそうなるでしょう。コードを次のように変更したとします。

TheEvent(this, EventArgs.Empty);

そのデリゲートの呼び出しリストに1000のエントリがあるとします。別のスレッドがリストの末尾近くでハンドラの登録を解除する前に、リストの先頭のアクションが実行されている可能性は十分にあります。ただし、そのハンドラは新しいリストになるため、引き続き実行されます。私が見ることができる限りでは、これは避けられません。

空のデリゲートを使用しても、無効性チェックは確実に回避されますが、競合状態は修正されません。また、常に変数の最新の値を「見る」という保証もありません。


  • Joe Duffyの「Windowsでの同時プログラミング」質問のJIT最適化とメモリモデルの側面について説明します。見るcode.logos.com/blog/2008/11/events_and_threads_part_4.html - Bradley Grainger
  • 「標準」に関するコメントに基づいて、これを承認しました。アドバイスはC#2以前のもので、矛盾する人はいないと聞いています。イベント引数をインスタンス化するのが本当に高価でない限り、' = delegate {}'と入力します。イベント宣言の最後に、それらがメソッドであるかのように直接イベントを呼び出します。それらにnullを代入しないでください。 (ハンドラが確実に破棄された後に呼び出されないという他のことはすべて無関係で、シングルスレッドのコードであっても保証することは不可能です。例えばハンドラ1がハンドラ2に破棄を要求してもハンドラ2は呼び出されるでしょう次。) - Daniel Earwicker
  • 唯一の問題は、(いつものように)構造体です。ここでは、メンバーの中でnull値以外のものでそれらがインスタンス化されることを保証できません。しかし、構造体は吸います。 - Daniel Earwicker
  • 空のデリゲートについては、この質問も参照してください。stackoverflow.com/questions/170907/…。 - Vladimir
  • @トニー:購読/購読解除とデリゲートが実行されるまでの間には、まだ基本的に競合条件があります。あなたのコードは(ちょっとブラウズしただけで)それが発生している間に購読/購読解除が有効になることを許可することによってその競合状態を減らします、しかし私は通常の振る舞いが十分に良くないほとんどの場合、これはどちらでもありません。 - Jon Skeet

50

私はこれを行う拡張方法に向かっている人がたくさんいるのを見ます...

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

それはあなたにイベントを発生させるためのより良い構文を与えます...

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

また、ローカルコピーはメソッド呼び出し時にキャプチャされるため、ローカルコピーも削除されます。


  • いい構文です!私はすぐにそれを使い始めるつもりです。 - Jason Coyne
  • 構文は好きですが、明確にしておきましょう。登録解除された後も古いハンドラが呼び出されても問題は解決しません。こののみnullの間接参照問題を解決します。構文は好きですが、それが次のいずれよりも優れているかどうかを疑問に思います。パブリックイベントEventHandler< T> MyEvent = delete {}; ... MyEvent(これ、新しいMyEventArgs());これは非常に低摩擦の解決策でもあり、私はその単純さのために好きです。 - Simon Gillbee
  • @Simonこれについては、さまざまな人々がさまざまなことを言っているのが見えます。私はそれをテストしました、そして私がしたことはこれがnullハンドラ問題を処理することを私に示しています。ハンドラー!= nullのチェックの後、元のSinkがEventから登録を解除しても、Eventは発生し続け、例外はスローされません。 - JP Alioto
  • うん、この質問を参照:stackoverflow.com/questions/192980/… - Benjol
  • +1。私は自分自身でこの方法を書いて、スレッドセーフについて考え始めましたが、いくつかの研究をしてこの質問に出くわしました。 - Niels van der Rest

31

「なぜ標準パターンを明示的にnullチェックするのですか?」

これは、nullチェックのほうがパフォーマンスが高いためと考えられます。

イベントが作成されたときに常に空のデリゲートを自分のイベントに登録すると、オーバーヘッドが発生します。

  • 空のデリゲートを構築するためのコスト。
  • それを含むためにデリゲートチェーンを構築するためのコスト。
  • イベントが発生するたびに無意味なデリゲートを呼び出すコスト。

(UIコントロールには多くのイベントがあり、そのほとんどはサブスクライブされていないことが多いことに注意してください。各イベントにダミーのサブスクライバを作成してから呼び出す必要があると、パフォーマンスが大幅に低下する可能性があります。)

subscribe-empty-delegateアプローチの影響を確認するためにいくつかの厄介なパフォーマンステストを行いました。これが私の結果です。

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

0人または1人の購読者(イベントが豊富なUIコントロールによく見られる)の場合、空のデリゲートで事前初期化されたイベントは特に遅くなります(5000万回以上の反復...)。

詳しい情報とソースコードについては、このブログ記事をご覧ください。.NETイベント呼び出しスレッドの安全性この質問が出される直前の日に私が発表したこと(!)

(私のテストの設定には欠陥があるかもしれませんので、ソースコードをダウンロードして自分で調べてください。どんなフィードバックでも大歓迎です。)


  • 私はあなたがあなたのブログ記事の中で重要なポイントを作ると思います:それがボトルネックになるまでパフォーマンスの影響について心配する必要はありません。なぜ醜い方法が推奨される方法にしましょうか。明快さの代わりに時期尚早の最適化が欲しいならば、私たちはアセンブラを使うでしょう - それで私の質問は残ります、そして私はその答えは単に匿名デリゲートより前にあり、人間文化が古いアドバイスを変えるのに長い時間がかかると思います有名な「ポットローストストーリー」のように。 - Daniel Earwicker
  • そして、あなたの数字はその点を非常によく証明しています。オーバーヘッドは、発生したイベントごとにわずか2.5ナノ秒(!!!)に達しています(初期化前と古典的ヌル)。実際に作業を行うほとんどのアプリではこれは検出できませんが、イベントの使用法の大部分がGUIフレームワークにあることを考えると、これをWinformなどの画面の一部を再描画するコストと比較する必要があります。そのため、実際のCPU作業とリソースの待機のあいだでは、それはさらに見えなくなります。とにかく、あなたは大変な仕事のために私から+1を得ます。 :) - Daniel Earwicker
  • @DanielEarwickerはそれを正しいと言いました、あなたは私をパブリックイベントの信者になるように動かしましたWrapperDoneHandler OnWrapperDone =(x、y)=> {};モデル。 - Mickey Perlstein
  • 時間を計るのも良いかもしれませんDelegate.Combine/Delegate.Removeイベントに0人、1人、または2人の加入者がいる場合のペア。同じデリゲートインスタンスを繰り返し追加および削除した場合、ケース間のコストの違いは特に顕著になります。Combine引数の1つがであるとき、速い特別な場合の振る舞いをしますnull(もう一方を返すだけ)Remove2つの引数が等しい場合は非常に高速です(単にnullを返します)。 - supercat

10

私はこの読書を本当に楽しんだ - そうではない!イベントと呼ばれるC#機能を扱うためにそれが必要だとしても!

コンパイラでこれを修正しないのはなぜでしょうか。私はこれらの記事を読んでいるMSの人々がいることを知っています、それでこれを燃やしてはいけません!

1 - ヌル問題)そもそもなぜイベントをnullではなく.Emptyにしないのですか?ヌルチェックのために何行のコードを保存するか、または= delegate {}宣言に?コンパイラに空の場合を処理させてください、IEは何もしません!それがすべてイベントの作成者にとって重要であるならば、彼らは.Emptyをチェックしてそれが気にすることは何でもすることができます!そうでなければ、すべてのnullチェック/デリゲート追加は問題を回避するハックです。

正直なところ、すべてのイベントでこれを実行する必要があるのはうんざりです - 別名ボイラープレートコード!

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 - 競合状態の問題Ericのブログ記事を読みました。H(ハンドラ)は自分自身を間接参照するときに処理すべきだと思いますが、イベントを不変/スレッドセーフにすることはできませんか? IEでは、作成時にロックフラグを設定して、呼び出されるたびに、実行中にそのサブスクリプションとアンサブスクライブをすべてロックします。

結論

現代の言語は私達のためにこのような問題を解決することになっていませんか?


  • 同意した、コンパイラでこれのためのより良いサポートがあるはずです。それまでは、私はポストコンパイルステップでこれを行うPostSharpアスペクトを作成しました。 :) - Steven Jeuris
  • 任意の外部コードが完了するのを待っている間にスレッドの購読/購読解除要求をブロックするはるかに悪い特に後者の「問題」から、購読がキャンセルされた後に購読者にイベントを受信させるよりも、解決することができます簡単に単にイベントハンドラにフラグをチェックさせて、イベントの受信にまだ関心があるかどうかを確認するだけですが、前者の設計によるデッドロックは解決できない場合があります。 - supercat
  • @ supercat。イモ、「はるかにひどい」コメントはかなりアプリケーションに依存します。オプションの場合、追加のフラグなしで非常に厳密なロックを希望しないのは誰でしょうか。ロックは同一スレッドのリエントラントであり、元のイベントハンドラ内のサブスクライブ/サブスクライブ解除はブロックされないため、イベント処理スレッドがさらに別のスレッド(サブスクライブ/サブスクライブ解除)で待機している場合にのみデッドロックが発生します。デザインの一部となるイベントハンドラの一部としてスレッド間で待機しているスレッドがある場合は、やり直すことをお勧めします。予測可能なパターンを持つサーバーサイドのアプリの角度から来ています。 - crokusek
  • @crokusek:分析に必要な証明する有向グラフで各ロックをすべてのロックに接続するサイクルがない場合、システムにデッドロックがないことは簡単です。たぶんそれが保持されている間に必要とされる[サイクルの欠如はシステムにデッドロックがないことを証明する]。ロックが保持されている間に任意のコードを呼び出すことを許可すると、「必要性がある可能性がある」要素にエッジが作成されます。そのロックから任意のコードが獲得する可能性のある任意のロックまでをグラフ化します(システム内のすべてのロックではありませんが、それほど遠くはありません)。結果としてサイクルが発生しても、デッドロックが発生するとは限りませんが、... - supercat
  • ...分析が不可能であることを証明するために必要な分析のレベルが大幅に上がります。 - supercat

5

本の中でジェフリーリヒターによるとC#によるCLR正しい方法は次のとおりです。

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

それは参照コピーを強制するからです。 詳細については、本の彼のイベントセクションを参照してください。


  • smthが欠けているかもしれませんが、最初の引数がnullの場合、Interlocked.CompareExchangeはNullReferenceExceptionをスローします。msdn.microsoft.com/ja-jp/library/bb297966.aspx - Kniganapolke
  • Interlocked.CompareExchange何らかの理由でnullが渡された場合は失敗します。refしかし、それは合格したのと同じことではありませんref保管場所(例:NewMail存在し、最初はどれ保持するnull参照 - supercat

4

このデザインパターンを使用して、イベントハンドラが登録解除された後に実行されないようにしています。パフォーマンスプロファイリングは試していませんが、これまでのところ非常にうまく機能しています。

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

私は最近Mono for Androidを使って仕事をしていますが、アクティビティがバックグラウンドに送信された後にViewを更新しようとすると、Androidは気に入らないようです。



1

これは、特定の操作順序を強制することではありません。それは実際にはnull参照例外を回避することです。

競合条件ではなく、null参照例外を気にかけている人々の背後にある推論は、いくつかの深い心理学的研究を必要とするでしょう。これは、null参照問題の修正がはるかに簡単であるという事実と関係があると思います。それが修正されれば、彼らは彼らのコードの上に大きな "Mission達成"バナーを掛け、彼らの飛行スーツを解凍します。

注:競合状態を修正するには、おそらくハンドラーを実行する必要があるかどうかを同期フラグトラックを使用する必要があります


  • その問題の解決策を求めていません。イベント発火に関して余分なコード混乱を回避するためのアドバイスが広く行き渡っている理由を疑問に思っています。検出できない競合状態がまだ存在するときにnull例外を回避するだけの場合です。 - Daniel Earwicker
  • それが私のポイントでした。していませんこれ競合状態について。それらはnull参照例外のみを気にします。それを私の答えに編集します。 - dss539
  • そして私の論点は、null参照例外を気にするのはなぜ理にかなっているのでしょうか。ではない競合状態を気にしますか? - Daniel Earwicker
  • 適切に記述されたイベントハンドラは、処理が追加または削除の要求とオーバーラップする可能性があるイベントを発生させるための特定の要求が、追加または削除されていたイベントを発生させる可能性がある。プログラマが競合状態を気にしないのは、適切に記述されたコード内にあるからです。誰が勝つかは関係ありません。 - supercat
  • @ dss539:保留中のイベント呼び出しが終了するまで購読解除要求をブロックするイベントフレームワークを設計することはできますが、そのような設計はどんなイベントに対しても不可能にします。Unload他のイベントへのオブジェクトの登録を安全にキャンセルするため。不快な。単純に言うと、イベントの購読中止リクエストにより、イベントが定期的に購読解除され、イベントサブスクライバが呼び出されたときに役立つことがあるかどうかを確認する必要があります。 - supercat

1

だから私はここのパーティーには少し遅れます。 :)

サブスクライバのいないイベントを表すためにnullオブジェクトパターンではなくnullを使用する場合は、このシナリオを検討してください。イベントを呼び出す必要がありますが、オブジェクトの作成(EventArgs)は自明ではありません。一般的な場合、イベントにサブスクライバはいません。引数を構築してイベントを呼び出すことに処理の労力を費やす前に、自分がサブスクライバを持っているかどうかを確認するようにコードを最適化できれば、それは有益です。

これを念頭に置いて、解決策は「まあ、ゼロの加入者はnullで表される」と言うことです。次に、高価な操作を実行する前に、単にnullチェックを実行します。別の方法として、Delegate型のCountプロパティを使用することも考えられます。そのため、myDelegate.Count> 0の場合にのみ高価な操作を実行することになります。Countプロパティを使用するのは、元の問題を解決するためのややよいパターンです。また、NullReferenceExceptionを発生させずに呼び出すことができるという優れた特性もあります。

ただし、デリゲートは参照型なので、nullにすることは許可されています。おそらく、この事実を隠蔽してイベントのnullオブジェクトパターンのみをサポートする良い方法はなかったので、代替案は開発者にnullと0の両方のサブスクライバのチェックを強いることでした。それは現在の状況よりもさらに厄介です。

注:これは純粋な推測です。私は.NET言語やCLRには関わっていません。


  • 「...ではなく空のデリゲートを使用する」という意味です。空のデリゲートに初期化されたイベントを使用して、すでに提案したことを実行できます。最初の空のデリゲートがリストの唯一のものである場合、テスト(MyEvent.GetInvocationList()。Length == 1)は真になります。それでも最初にコピーを作成する必要はありません。とはいえ、私があなたが説明するケースはとにかく非常にまれであると思いますが。 - Daniel Earwicker
  • ここでは、参加者やイベントのアイデアをまとめています。自分のクラスにイベントFooがある場合、外部ユーザーがMyType.Foo + = / - =を呼び出すと、実際にはadd_Foo()メソッドとremove_Foo()メソッドが呼び出されます。ただし、Fooが定義されているクラス内からFooを参照すると、実際にはadd_Foo()およびremove_Foo()メソッドではなく、基礎となるデリゲートが直接参照されます。また、EventHandlerListのような型が存在するため、デリゲートとイベントを同じ場所に配置することを義務付けるものは何もありません。これが、「心に留めておく」という意味です。私の応答の段落。 - Levi
  • (続き)私はこれが混乱を招くようなデザインであると認めますが、代替案はもっと悪かったかもしれません。結局のところあなたが持っているのはデリゲートだけです - あなたは直接基礎となるデリゲートを参照するかもしれません、あなたはコレクションからそれを得るかもしれません、あなたはそれをその場でインスタンス化するかもしれません - "チェック以外の何かをサポートすることは技術的に不可能です。 null"の場合パターン。 - Levi
  • イベントの開始について話しているので、ここでは追加/削除アクセサがなぜ重要なのかわかりません。 - Daniel Earwicker
  • @Levi:C#がイベントを処理する方法が嫌いです。私が私のdruthersを持っていたならば、代表はイベントから別の名前を与えられたでしょう。クラスの外からは、イベント名に対する唯一許可されている操作は、+=そして-=。クラス内では、許される操作には、(組み込みのnullチェックを使った)呼び出しも含まれます。null、またはに設定null。それ以外の場合は、イベント名に特定のプレフィックスまたはサフィックスを付けた名前のデリゲートを使用する必要があります。 - supercat

1

C#6以降では、newを使ってコードを単純化することができます。.? operatorのようにTheEvent?.Invoke(this, EventArgs.Empty);

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


0

シングルスレッドのアプリケーションでは、これは問題にはならないことは間違いありません。

ただし、イベントを公開するコンポーネントを作成している場合、そのコンポーネントのコンシューマがマルチスレッド化しないという保証はありません。その場合は、最悪の状況に備える必要があります。

空のデリゲートを使用すると問題は解決しますが、イベントへのすべての呼び出しでパフォーマンスが低下するため、GCに影響を与える可能性があります。

これを実現するために消費者が購読を中止するのは正しいことですが、一時コピーを過ぎた場合は、すでに送信中のメッセージを検討してください。

一時変数を使用せず、空のデリゲートを使用せず、誰かが購読を中止すると、null参照例外が発生します。これは致命的なので、コストに見合うだけの価値があると思います。


0

私は一般的に私の再利用可能なコンポーネント上の静的メソッド(など)におけるこの種の潜在的なスレッド化の悪さから保護するだけであり、静的イベントを作成しないので、これをあまり問題にしません。

私はそれを間違っていますか?


  • 可変状態のクラスのインスタンス(値を変更するフィールド)を割り当ててから、ロックを使用せずに複数のスレッドが同じインスタンスに同時にアクセスできるようにすると、それらのフィールドが同時に2つのスレッドによって変更されるのを防ぐことができます。おそらく間違っているのでしょう。すべてのスレッドに独自のインスタンスがある(何も共有していない)か、すべてのオブジェクトが不変である(一度割り当てられると、そのフィールドの値は変更されない)場合は、おそらく問題ないでしょう。 - Daniel Earwicker
  • 私の一般的なアプローチは、静的メソッドを除いて、同期を呼び出し側に任せることです。私が発信者である場合は、その上位レベルで同期します。 (もちろん、同期アクセスを処理することだけを目的としているオブジェクトは例外です。:)) - Greg D
  • メソッドがどれほど複雑で、どのデータを使用するかによって異なります。それが内部のメンバーに影響を及ぼし、あなたがスレッド化された/タスク化された状態で実行することにした場合、あなたはたくさんのけがをしています - Mickey Perlstein

0

あなたのすべてのイベントを構築時に配線し、それらを一人にしておきます。この投稿の最後の段落で説明するように、Delegateクラスの設計は他の使用法を正しく処理できない可能性があります。

まず第一に、しようとしても意味がありません傍受イベントお知らせあなたのイベントハンドラーするかどうか/どうするかについてはすでに同期した決定をしなければ通知に返信する

通知される可能性があるものはすべて通知する必要があります。あなたのイベントハンドラが通知を適切に処理している(すなわち、それらが正式なアプリケーション状態にアクセスして適切な時にだけ応答する)場合、いつでもそれらに通知し信頼するのが良いでしょう。

イベントが発生したことをハンドラに通知してはいけないのは、イベントが実際に発生していない場合だけです。そのため、ハンドラに通知したくない場合は、イベントの生成を停止します(つまり、コントロールを無効にするか、最初にイベントを検出して存在させる原因となるものは何でも)。

正直なところ、私はDelegateクラスは避けられないと思います。 MulticastDelegateへの合併/移行は、イベントの(有用な)定義を、ある瞬間に発生するものから、ある期間にわたって発生するものに効果的に変更したため、大きな間違いでした。このような変更には、論理的にそれを単一の瞬間に折りたたむことができる同期メカニズムが必要ですが、MulticastDelegateにはそのようなメカニズムがありません。同期は、イベントが発生するまでの期間全体または瞬間を網羅する必要があります。そのため、アプリケーションは、イベントの処理を開始するという同期の決定を下すと、トランザクションの処理を完全に終了します。 MulticastDelegate / Delegateハイブリッドクラスであるブラックボックスでは、これはほぼ不可能です。単一の加入者を使用することを固守するか、ハンドラーチェーンが使用または変更されている間に取り出すことができる同期ハンドルを持つ独自の種類のMulticastDelegateを実装する。。私はこれをお勧めします。代替手段は同期/トランザクションの整合性をすべてのハンドラに冗長に実装することであり、これはばかげて/不必要に複雑になるでしょう。


  • [1]「ある瞬間に」発生する便利なイベントハンドラはありません。すべての操作には期間があります。どの単一のハンドラでも、実行するための重要でない一連のステップを持つことができます。ハンドラのリストをサポートしても何も変わりません。 - Daniel Earwicker
  • [2]イベントが発生している間にロックを保持することは完全な狂気です。それは必然的にデッドロックにつながる。ソースはロックAを取り出し、イベントを発生させ、シンクはロックBを取り出し、2つのロックが保持されます。別のスレッドで何らかの操作を行った結果、逆の順序でロックが解除された場合はどうなりますか?ロックに対する責任が、別々に設計/テストされたコンポーネント(イベントの全体的なポイント)間で分割されている場合、そのような致命的な組み合わせをどのように除外できますか? - Daniel Earwicker
  • [3]これらの問題はいずれも、特にGUIフレームワークにおいて、コンポーネントのシングルスレッド構成における通常のマルチキャストデリゲート/イベントの全面的な有用性を損なうものではありません。このユースケースは、イベントの使用の大部分をカバーしています。自由にスレッド化された方法でイベントを使用することは疑わしい価値があります。これは、それらが意味をなす文脈におけるそれらのデザインまたはそれらの明白な有用性を決して無効にするものではありません。 - Daniel Earwicker
  • [4]スレッド+同期イベントは本質的にはニシンです。キュー非同期通信は、行く方法です。 - Daniel Earwicker
  • [1]計測時間について言及していませんでした...論理的に瞬時に発生するアトミック操作について話していました...そして、同じリソースを使用している間に変更できるものは他にないという意味です。このイベントはロック付きでシリアル化されているため発生しています。 - Triynko

0

こちらをご覧ください。http://www.danielfortunov.com/software/%24daniel_fortunovs_adventures_in_software_development/2009/04/23/net_event_invocation_thread_safetyこれは正しい解決方法であり、他のすべての回避策ではなく常に使用する必要があります。

「何もしない匿名メソッドで初期化することで、内部呼び出しリストに常に少なくとも1人のメンバーが含まれるようにすることができます。外部の者が匿名メソッドへの参照を持つことはできないため、外部の者がメソッドを削除することはできず、デリゲートはnullになることはありません。」      - プログラミング.NETコンポーネント、第2版、JuvalLö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);  
}  


0

質問がc#の "イベント"タイプに限定されているとは思わない。その制限を取り除き、ホイールを少し作り直して、これらの方針に沿って何かをしてはどうでしょうか。

安全にイベントスレッドを上げる - ベストプラクティス

  • 昇給中に任意のスレッドを購読/購読解除する機能(レース 状態が解除されました)
  • 演算子はクラスレベルで+ =と - =をオーバーロードします。
  • 一般的な発信者定義のデリゲート


0

有用な議論をありがとう。私は最近この問題に取り組んでいて、次のクラスを作成しました。これは少し遅くなりますが、破棄されたオブジェクトへの呼び出しを避けることができます。

ここでの重要な点は、イベントが発生しても呼び出しリストを変更できるということです。

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

そして使い方は:

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

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

テスト

次のようにしてテストしました。私はこのようなオブジェクトを作成して破壊するスレッドを持っています:

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

Bar私が購読している(リスナーオブジェクト)コンストラクタSomeEvent(上記のように実装されています)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;
    }

また、ループ内でイベントを発生させるスレッドがいくつかあります。

これらすべてのアクションは同時に実行されます。多くのリスナーが作成および破棄され、イベントが同時に発生します。

競合状態が発生した場合、コンソールにメッセージが表示されるはずですが、空です。しかし、私がいつものようにclrイベントを使うと、警告メッセージがいっぱいになるのがわかります。だから、私はそれがC#でスレッドセーフなイベントを実装することが可能であると結論付けることができます。

どう思いますか?


  • 私にはよさそうだ。私はそれが可能であるかもしれないと思うが(理論的に)disposed = true前に起こるfoo.SomeEvent -= Handlerテストアプリケーションでは、誤検知が発生します。それ以外にも、変更が必要なものがいくつかあります。本当に使いたいtry ... finallyロックのために - これはあなたがこれをスレッドセーフだけでなくアボートセーフにするのを助けるでしょう。あなたがそのばかげたことを取り除くことができることは言うまでもありませんtry catch。渡された代理人を確認していませんAdd/Remove- かもしれないnull(あなたはすぐに捨てるべきですAdd/Remove) - Luaan

リンクされた質問


関連する質問

最近の質問