更新
C#6では、この質問に対する答えは次のとおりです。
SomeEvent?.Invoke(this, e);
私は以下のアドバイスをよく聞いたり読んだりします。
確認する前に、必ずイベントのコピーを作成してください。null
そしてそれを発射する。これにより、イベントが発生した場所でのスレッド処理に関する潜在的な問題が解消されます。null
nullをチェックした場所とイベントを発生させた場所の間の場所。
// 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年には行われるはずだったのでしょうか。
条件のため、JITはあなたが最初の部分で話している最適化を実行することを許可されていません。私はこれがしばらく前に幽霊として提起されたことを知っています、しかしそれは有効ではありません。 (私はしばらく前にJoe DuffyかVance Morrisonのどちらかでそれをチェックした。どちらを覚えているのかわからない。)
volatile修飾子がないと、取得したローカルコピーが古くなる可能性がありますが、それだけです。それは引き起こしませんNullReferenceException
。
確かに、競合状態があります - しかし、常にそうなるでしょう。コードを次のように変更したとします。
TheEvent(this, EventArgs.Empty);
そのデリゲートの呼び出しリストに1000のエントリがあるとします。別のスレッドがリストの末尾近くでハンドラの登録を解除する前に、リストの先頭のアクションが実行されている可能性は十分にあります。ただし、そのハンドラは新しいリストになるため、引き続き実行されます。私が見ることができる限りでは、これは避けられません。
空のデリゲートを使用しても、無効性チェックは確実に回避されますが、競合状態は修正されません。また、常に変数の最新の値を「見る」という保証もありません。
私はこれを行う拡張方法に向かっている人がたくさんいるのを見ます...
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() );
また、ローカルコピーはメソッド呼び出し時にキャプチャされるため、ローカルコピーも削除されます。
これは、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イベント呼び出しスレッドの安全性この質問が出される直前の日に私が発表したこと(!)
(私のテストの設定には欠陥があるかもしれませんので、ソースコードをダウンロードして自分で調べてください。どんなフィードバックでも大歓迎です。)
Delegate.Combine
/Delegate.Remove
イベントに0人、1人、または2人の加入者がいる場合のペア。同じデリゲートインスタンスを繰り返し追加および削除した場合、ケース間のコストの違いは特に顕著になります。Combine
引数の1つがであるとき、速い特別な場合の振る舞いをしますnull
(もう一方を返すだけ)Remove
2つの引数が等しい場合は非常に高速です(単にnullを返します)。 - supercat
私はこの読書を本当に楽しんだ - そうではない!イベントと呼ばれる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では、作成時にロックフラグを設定して、呼び出されるたびに、実行中にそのサブスクリプションとアンサブスクライブをすべてロックします。
結論、
現代の言語は私達のためにこのような問題を解決することになっていませんか?
本の中でジェフリーリヒターによると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);
それは参照コピーを強制するからです。 詳細については、本の彼のイベントセクションを参照してください。
Interlocked.CompareExchange
何らかの理由でnullが渡された場合は失敗します。ref
しかし、それは合格したのと同じことではありませんref
保管場所(例:NewMail
存在し、最初はどれ保持するnull参照 - supercat
このデザインパターンを使用して、イベントハンドラが登録解除された後に実行されないようにしています。パフォーマンスプロファイリングは試していませんが、これまでのところ非常にうまく機能しています。
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は気に入らないようです。
これは、特定の操作順序を強制することではありません。それは実際にはnull参照例外を回避することです。
競合条件ではなく、null参照例外を気にかけている人々の背後にある推論は、いくつかの深い心理学的研究を必要とするでしょう。これは、null参照問題の修正がはるかに簡単であるという事実と関係があると思います。それが修正されれば、彼らは彼らのコードの上に大きな "Mission達成"バナーを掛け、彼らの飛行スーツを解凍します。
注:競合状態を修正するには、おそらくハンドラーを実行する必要があるかどうかを同期フラグトラックを使用する必要があります
Unload
他のイベントへのオブジェクトの登録を安全にキャンセルするため。不快な。単純に言うと、イベントの購読中止リクエストにより、イベントが定期的に購読解除され、イベントサブスクライバが呼び出されたときに役立つことがあるかどうかを確認する必要があります。 - supercat
だから私はここのパーティーには少し遅れます。 :)
サブスクライバのいないイベントを表すためにnullオブジェクトパターンではなくnullを使用する場合は、このシナリオを検討してください。イベントを呼び出す必要がありますが、オブジェクトの作成(EventArgs)は自明ではありません。一般的な場合、イベントにサブスクライバはいません。引数を構築してイベントを呼び出すことに処理の労力を費やす前に、自分がサブスクライバを持っているかどうかを確認するようにコードを最適化できれば、それは有益です。
これを念頭に置いて、解決策は「まあ、ゼロの加入者はnullで表される」と言うことです。次に、高価な操作を実行する前に、単にnullチェックを実行します。別の方法として、Delegate型のCountプロパティを使用することも考えられます。そのため、myDelegate.Count> 0の場合にのみ高価な操作を実行することになります。Countプロパティを使用するのは、元の問題を解決するためのややよいパターンです。また、NullReferenceExceptionを発生させずに呼び出すことができるという優れた特性もあります。
ただし、デリゲートは参照型なので、nullにすることは許可されています。おそらく、この事実を隠蔽してイベントのnullオブジェクトパターンのみをサポートする良い方法はなかったので、代替案は開発者にnullと0の両方のサブスクライバのチェックを強いることでした。それは現在の状況よりもさらに厄介です。
注:これは純粋な推測です。私は.NET言語やCLRには関わっていません。
+=
そして-=
。クラス内では、許される操作には、(組み込みのnullチェックを使った)呼び出しも含まれます。null
、またはに設定null
。それ以外の場合は、イベント名に特定のプレフィックスまたはサフィックスを付けた名前のデリゲートを使用する必要があります。 - supercat
C#6以降では、newを使ってコードを単純化することができます。.? operator
のようにTheEvent?.Invoke(this, EventArgs.Empty);
シングルスレッドのアプリケーションでは、これは問題にはならないことは間違いありません。
ただし、イベントを公開するコンポーネントを作成している場合、そのコンポーネントのコンシューマがマルチスレッド化しないという保証はありません。その場合は、最悪の状況に備える必要があります。
空のデリゲートを使用すると問題は解決しますが、イベントへのすべての呼び出しでパフォーマンスが低下するため、GCに影響を与える可能性があります。
これを実現するために消費者が購読を中止するのは正しいことですが、一時コピーを過ぎた場合は、すでに送信中のメッセージを検討してください。
一時変数を使用せず、空のデリゲートを使用せず、誰かが購読を中止すると、null参照例外が発生します。これは致命的なので、コストに見合うだけの価値があると思います。
私は一般的に私の再利用可能なコンポーネント上の静的メソッド(など)におけるこの種の潜在的なスレッド化の悪さから保護するだけであり、静的イベントを作成しないので、これをあまり問題にしません。
私はそれを間違っていますか?
あなたのすべてのイベントを構築時に配線し、それらを一人にしておきます。この投稿の最後の段落で説明するように、Delegateクラスの設計は他の使用法を正しく処理できない可能性があります。
まず第一に、しようとしても意味がありません傍受イベントお知らせあなたのイベントハンドラーするかどうか/どうするかについてはすでに同期した決定をしなければ通知に返信する。
通知される可能性があるものはすべて通知する必要があります。あなたのイベントハンドラが通知を適切に処理している(すなわち、それらが正式なアプリケーション状態にアクセスして適切な時にだけ応答する)場合、いつでもそれらに通知し信頼するのが良いでしょう。
イベントが発生したことをハンドラに通知してはいけないのは、イベントが実際に発生していない場合だけです。そのため、ハンドラに通知したくない場合は、イベントの生成を停止します(つまり、コントロールを無効にするか、最初にイベントを検出して存在させる原因となるものは何でも)。
正直なところ、私はDelegateクラスは避けられないと思います。 MulticastDelegateへの合併/移行は、イベントの(有用な)定義を、ある瞬間に発生するものから、ある期間にわたって発生するものに効果的に変更したため、大きな間違いでした。このような変更には、論理的にそれを単一の瞬間に折りたたむことができる同期メカニズムが必要ですが、MulticastDelegateにはそのようなメカニズムがありません。同期は、イベントが発生するまでの期間全体または瞬間を網羅する必要があります。そのため、アプリケーションは、イベントの処理を開始するという同期の決定を下すと、トランザクションの処理を完全に終了します。 MulticastDelegate / Delegateハイブリッドクラスであるブラックボックスでは、これはほぼ不可能です。単一の加入者を使用することを固守するか、ハンドラーチェーンが使用または変更されている間に取り出すことができる同期ハンドルを持つ独自の種類のMulticastDelegateを実装する。。私はこれをお勧めします。代替手段は同期/トランザクションの整合性をすべてのハンドラに冗長に実装することであり、これはばかげて/不必要に複雑になるでしょう。
こちらをご覧ください。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);
}
質問がc#の "イベント"タイプに限定されているとは思わない。その制限を取り除き、ホイールを少し作り直して、これらの方針に沿って何かをしてはどうでしょうか。
有用な議論をありがとう。私は最近この問題に取り組んでいて、次のクラスを作成しました。これは少し遅くなりますが、破棄されたオブジェクトへの呼び出しを避けることができます。
ここでの重要な点は、イベントが発生しても呼び出しリストを変更できるということです。
/// <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
EventName(arguments)
null以外の場合にのみデリゲートを呼び出すようにするのではなく、無条件でイベントのデリゲートを呼び出す(nullの場合は何もしない)。 - supercat