473,495 Members | 2,128 Online
Bytes | Software Development & Data Engineering Community
Create Post

Home Posts Topics Members FAQ

Race Conditions in C# Eventing?

Hi,

in VB.Net I fire an event very simply:

RaiseEvent(MyEvent) 'Fire&Forget

The same in C# looks like

if(MyEvent != null) //check, if someone's listening
{
MyEvent(); //Fire
}

What happens in a multithreaded environment? The moment after I checked
MyEvent the last listener could retreat and I would have a
NullReferenceException nonetheless, wouldn't I?

Best regards,
Karsten Schramm

Jul 7 '06 #1
15 2456
On 7 Jul 2006 08:02:51 -0700, Karsten Schramm wrote:
The same in C# looks like

if(MyEvent != null) //check, if someone's listening
{
MyEvent(); //Fire
}

What happens in a multithreaded environment? The moment after I checked
MyEvent the last listener could retreat and I would have a
NullReferenceException nonetheless, wouldn't I?
Yes. See Jon Skeet's article about thread safe events here:
<http://www.yoda.arachsys.com/csharp/threads/lockchoice.shtml>
Jul 7 '06 #2
Hello Karsten,

RaiseEvent in VB.net is not thread-safe, you can see this in MSIL
thus use locking like

SyncLock Me
RaiseEvent(MyEvent)
End SyncLock

But, registration and unregistration is *always* thread-safe.

Now about C#.
The problem is that calls to register and unregister events are thread-safe
if they are called from outside the class with the event

KSin VB.Net I fire an event very simply:
KS>
KSRaiseEvent(MyEvent) 'Fire&Forget
KS>
KSThe same in C# looks like
KS>
KSif(MyEvent != null) //check, if someone's listening
KS{
KSMyEvent(); //Fire
KS}
KSWhat happens in a multithreaded environment? The moment after I
KSchecked MyEvent the last listener could retreat and I would have a
KSNullReferenceException nonetheless, wouldn't I?
KS>
KSBest regards,
KSKarsten Schramm
---
WBR,
Michael Nemtsev :: blog: http://spaces.msn.com/laflour

"At times one remains faithful to a cause only because its opponents do not
cease to be insipid." (c) Friedrich Nietzsche
Jul 7 '06 #3
Michael Nemtsev <ne*****@msn.comwrote:
RaiseEvent in VB.net is not thread-safe, you can see this in MSIL
thus use locking like

SyncLock Me
RaiseEvent(MyEvent)
End SyncLock
That's a really bad idea IMO - holding a lock while you raise an event
could be catastrophic, as the event handler could be doing something
like calling Control.Invoke, which could involve the UI thread waiting
to acquire the same lock.
But, registration and unregistration is *always* thread-safe.
Well, it depends on exactly what you mean. It will always be atomic,
but you won't necessarily see the change from other threads.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too
Jul 7 '06 #4
Karsten,

In C#, the code that you have is actually a bad idea. The current
suggestion out of MS is to do something like this:

// Assign to a local variable.
EventHandler myEvent = MyEvent;

// Check for null.
if (myEvent != null)
{
// Fire.
myEvent();
}

This will handle the case of the delegate list being modified on another
thread (having it nulled out).

However, the CLR can actually optimize the local variable away (through
inlining), and directly access MyEvent still.

Because of this, to be TRULY safe, you want to pass the event through to
a method that is either virtual, or has the MethodImpl attribute attached to
it, passing the MethodImplOptions.NoInlining option. This way, the function
will not be inlined, which results in your parameter being preserved,
instead of being inlined to the original variable.

Hope this helps.
--
- Nicholas Paldino [.NET/C# MVP]
- mv*@spam.guard.caspershouse.com

"Karsten Schramm" <Ne**********************@yajirobi.dewrote in message
news:11**********************@75g2000cwc.googlegro ups.com...
Hi,

in VB.Net I fire an event very simply:

RaiseEvent(MyEvent) 'Fire&Forget

The same in C# looks like

if(MyEvent != null) //check, if someone's listening
{
MyEvent(); //Fire
}

What happens in a multithreaded environment? The moment after I checked
MyEvent the last listener could retreat and I would have a
NullReferenceException nonetheless, wouldn't I?

Best regards,
Karsten Schramm

Jul 7 '06 #5
Hello Jon Skeet [C# MVP],

JMichael Nemtsev <ne*****@msn.comwrote:
>RaiseEvent in VB.net is not thread-safe, you can see this in MSIL
thus use locking like
SyncLock Me
RaiseEvent(MyEvent)
End SyncLock
JThat's a really bad idea IMO - holding a lock while you raise an
Jevent could be catastrophic, as the event handler could be doing
Jsomething like calling Control.Invoke, which could involve the UI
Jthread waiting to acquire the same lock.

Yep, but we need to realize what are we doing.

Jon, I see the problem in RaiseEvent that MSIL doesn't guarantee that it
wont be interupted somewhere in the middle before Invoke call and in that
case, for example we unreg our event, return to Invoke and ta-da got NullException
>But, registration and unregistration is *always* thread-safe.
JWell, it depends on exactly what you mean. It will always be atomic,
Jbut you won't necessarily see the change from other threads.

I meant that for VB MSIL always mark reg/unreg as syncronized (translating
call to add_xxx remove_xxx methods)
But for C# it works in some cases - only when these calls are in separate
class

---
WBR,
Michael Nemtsev :: blog: http://spaces.msn.com/laflour

"At times one remains faithful to a cause only because its opponents do not
cease to be insipid." (c) Friedrich Nietzsche
Jul 7 '06 #6
Nicholas Paldino [.NET/C# MVP] <mv*@spam.guard.caspershouse.comwrote:
In C#, the code that you have is actually a bad idea. The current
suggestion out of MS is to do something like this:

// Assign to a local variable.
EventHandler myEvent = MyEvent;

// Check for null.
if (myEvent != null)
{
// Fire.
myEvent();
}

This will handle the case of the delegate list being modified on another
thread (having it nulled out).
It won't make sure that any changes made by a different thread are used
in the first place though.
However, the CLR can actually optimize the local variable away (through
inlining), and directly access MyEvent still.
I'm sure I've read something about this, but that it turned out not to
be true. I've a sneaking suspicion it was on some blog or other. I
found this:
http://blogs.msdn.com/grantri/archiv...07/226355.aspx
but I'm not sure it's the same thing.
Because of this, to be TRULY safe, you want to pass the event through to
a method that is either virtual, or has the MethodImpl attribute attached to
it, passing the MethodImplOptions.NoInlining option. This way, the function
will not be inlined, which results in your parameter being preserved,
instead of being inlined to the original variable.
A better solution is to make it thread-safe, IMO.

See http://www.pobox.com/~skeet/csharp/t...ckchoice.shtml for my
preferred way of doing things.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too
Jul 7 '06 #7
Michael Nemtsev <ne*****@msn.comwrote:
JThat's a really bad idea IMO - holding a lock while you raise an
Jevent could be catastrophic, as the event handler could be doing
Jsomething like calling Control.Invoke, which could involve the UI
Jthread waiting to acquire the same lock.

Yep, but we need to realize what are we doing.

Jon, I see the problem in RaiseEvent that MSIL doesn't guarantee that it
wont be interupted somewhere in the middle before Invoke call and in that
case, for example we unreg our event, return to Invoke and ta-da got NullException
So do it the C# instead of using RaiseEvent in the first place. It does
seem that RaiseEvent is pretty broken.
But, registration and unregistration is *always* thread-safe.
JWell, it depends on exactly what you mean. It will always be atomic,
Jbut you won't necessarily see the change from other threads.

I meant that for VB MSIL always mark reg/unreg as syncronized (translating
call to add_xxx remove_xxx methods)
But for C# it works in some cases - only when these calls are in separate
class
The autogenerated add/remove calls are indeed synchronized, but unless
you *also* lock when accessing the delegate, you don't get the
equivalent of volatility. The changes will be effectively volatile, but
the read may not be - you could read a value which is "stale",
basically.

(There's also the problem that the autogenerated methods lock on
"this", which is generally a bad idea in the first place...)

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too
Jul 7 '06 #8
Hello Jon Skeet [C# MVP],

JMichael Nemtsev <ne*****@msn.comwrote:
>JThat's a really bad idea IMO - holding a lock while you raise an
Jevent could be catastrophic, as the event handler could be doing
Jsomething like calling Control.Invoke, which could involve the UI
Jthread waiting to acquire the same lock.

Yep, but we need to realize what are we doing.

Jon, I see the problem in RaiseEvent that MSIL doesn't guarantee that
it wont be interupted somewhere in the middle before Invoke call and
in that case, for example we unreg our event, return to Invoke and
ta-da got NullException
JSo do it the C# instead of using RaiseEvent in the first place. It
Jdoes seem that RaiseEvent is pretty broken.

Exactly. It's what I'm talking about
>>But, registration and unregistration is *always* thread-safe.
JWell, it depends on exactly what you mean. It will always be
atomic, Jbut you won't necessarily see the change from other
threads.

I meant that for VB MSIL always mark reg/unreg as syncronized
(translating call to add_xxx remove_xxx methods) But for C# it works
in some cases - only when these calls are in separate class
JThe autogenerated add/remove calls are indeed synchronized, but
Junless you *also* lock when accessing the delegate, you don't get the
Jequivalent of volatility. The changes will be effectively volatile,
Jbut the read may not be - you could read a value which is "stale",
Jbasically.

Don't quite understand about equivalent of volatility and why read may be
dirty in that case?
Could you explait in a bit?

It's more important to remember that we must *always" lock reg/unreg within
the class with event.

J(There's also the problem that the autogenerated methods lock on
J"this", which is generally a bad idea in the first place...)
J>
---
WBR,
Michael Nemtsev :: blog: http://spaces.msn.com/laflour

"At times one remains faithful to a cause only because its opponents do not
cease to be insipid." (c) Friedrich Nietzsche
Jul 7 '06 #9
It won't make sure that any changes made by a different thread are used
in the first place though.
Yes you are right, the code only will handle the case if the event is
null or not. It doesn't handle preserving the list itself. If you access
the event directly, there is a chance that in between the check for null,
and the raising of the event, you could get a null reference exception
because another thread has nulled out the list (removed the last event
listener).
>
> However, the CLR can actually optimize the local variable away
(through
inlining), and directly access MyEvent still.

I'm sure I've read something about this, but that it turned out not to
be true. I've a sneaking suspicion it was on some blog or other. I
found this:
http://blogs.msdn.com/grantri/archiv...07/226355.aspx
but I'm not sure it's the same thing.
Yes, this is a similar example. However, you can't place the volatile
keyword on an event, so the solution posted there won't work.
> Because of this, to be TRULY safe, you want to pass the event through
to
a method that is either virtual, or has the MethodImpl attribute attached
to
it, passing the MethodImplOptions.NoInlining option. This way, the
function
will not be inlined, which results in your parameter being preserved,
instead of being inlined to the original variable.

A better solution is to make it thread-safe, IMO.

See http://www.pobox.com/~skeet/csharp/t...ckchoice.shtml for my
preferred way of doing things.
I don't think that this is a good way to do this. You have to end up
placing this code at the call site and the declaration of every single event
that you have. This quickly becomes a maintinence issue.

In using a method that will handle the firing of all of your events for
you (or at least the ones that follow the suggested pattern of two
parameters, object o and EventArgs e), and declaring that the method can't
be inlined, you can easily provide nearly the same level of protection
(neither your solution nor mine actually preserves the delegate chain, only
preventing it from being set to null in a multi-threaded environment), and
only have to modify the call site for actually raising the event.

With your way of doing it, though, you gain the advantage of
encapsulating the lock object that is used on the event. In delcaring the
event normally, you end up synchronizing on this, or the Type of the object.
I would say that it is a judgement call whether or not you want to maintain
that much code for that benefit.

--
- Nicholas Paldino [.NET/C# MVP]
- mv*@spam.guard.caspershouse.com

>
--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

Jul 7 '06 #10
Nicholas Paldino [.NET/C# MVP] <mv*@spam.guard.caspershouse.comwrote:
It won't make sure that any changes made by a different thread are used
in the first place though.

Yes you are right, the code only will handle the case if the event is
null or not. It doesn't handle preserving the list itself. If you access
the event directly, there is a chance that in between the check for null,
and the raising of the event, you could get a null reference exception
because another thread has nulled out the list (removed the last event
listener).
No, that's not an issue because delegates are immutable. Nothing can
actually remove the event listener from the list - they would only
change the variable to hold a reference to a different delegate.

However, you could find that event handlers have been subscribed (or
removed) from other threads but that you never see them.
A better solution is to make it thread-safe, IMO.

See http://www.pobox.com/~skeet/csharp/t...ckchoice.shtml for my
preferred way of doing things.

I don't think that this is a good way to do this. You have to end up
placing this code at the call site and the declaration of every single event
that you have. This quickly becomes a maintinence issue.
It's a lot of work, but that's what you get when you want to make a
class thread-safe. Of course, if you don't need to make it thread-safe
in the first place (if you can demand that all subscriptions and
unsubscriptions take place on the same thread that the event is raised
on) then there's less of an issue. Admittedly in a strict mood I'd
still want to wire the add/remove myself to avoid using an implicit
lock, but that's a different matter.
In using a method that will handle the firing of all of your events for
you (or at least the ones that follow the suggested pattern of two
parameters, object o and EventArgs e), and declaring that the method can't
be inlined, you can easily provide nearly the same level of protection
(neither your solution nor mine actually preserves the delegate chain, only
preventing it from being set to null in a multi-threaded environment), and
only have to modify the call site for actually raising the event.
Mine is safe and correct by the spec rather than just by the current
implementation. Inlining and thread safety are only related by virtue
of the current implementation, IMO.
With your way of doing it, though, you gain the advantage of
encapsulating the lock object that is used on the event. In delcaring the
event normally, you end up synchronizing on this, or the Type of the object.
I would say that it is a judgement call whether or not you want to maintain
that much code for that benefit.
Well, I'm kinda anal about thread safety...

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too
Jul 7 '06 #11
"Nicholas Paldino [.NET/C# MVP]" <mv*@spam.guard.caspershouse.com>
wrote:
In C#, the code that you have is actually a bad idea. The current
suggestion out of MS is to do something like this:

// Assign to a local variable.
EventHandler myEvent = MyEvent;

// Check for null.
if (myEvent != null)
{
// Fire.
myEvent();
}

This will handle the case of the delegate list being modified on another
thread (having it nulled out).
I think this pattern brings back the same old Java "synchronized"
problem of designing *every* class as if it was OK to be used
simultaneously on lots of different threads. It was wrong for Java (it
wasn't included in C#) and I think it's wrong to use *all* the time for
raising events too. In carefully selected classes, it may be justified.

What's more, it will break if the compiler implements copy propagation.
To get it to work reliably, you either need to use a backing variable
(i.e. "add" and "remove" which manipulate the backing variable) which is
marked volatile, or lock on some object while reading the event:

---8<---
EventHandler myEvent;
lock (_someObject)
myEvent = MyEvent;
// ...
--->8---

The compiler can't and won't use copy propagation here, since it's aware
of threads and locks. (It's also one of the reasons that the C++
standard's agnosticism on threads is wrong-headed, but that's another
different story...)

As Jon links to, Grant (working on the x64 compiler) ran into trouble
implementing copy propagation, since it broke BCL source code
assumptions.

-- Barry

--
http://barrkel.blogspot.com/
Jul 7 '06 #12
"Nicholas Paldino [.NET/C# MVP]" <mv*@spam.guard.caspershouse.com>
wrote:
See http://www.pobox.com/~skeet/csharp/t...ckchoice.shtml for my
preferred way of doing things.

I don't think that this is a good way to do this. You have to end up
placing this code at the call site and the declaration of every single event
that you have. This quickly becomes a maintinence issue.
You don't have a lot of choice *if* you want to make the class
thread-safe.

A third way (other than volatile or locking) is to use a memory read
barrier between the read and the subsequent uses:

---8<---
EventHandler myEvent = MyEvent;

Thread.MemoryBarrier();

if (myEvent != null)
myEvent();
--->8---

-- Barry

--
http://barrkel.blogspot.com/
Jul 7 '06 #13
Jon,

See inline:
Mine is safe and correct by the spec rather than just by the current
implementation. Inlining and thread safety are only related by virtue
of the current implementation, IMO.
Using the MethodImpl attribute is spec-safe as well. In Partition II,
Section 15.4.3, the definition for the flags for ImplAttr (which is mapped
from the MethodImplOptions enumeration passed to MethodImplAttribute)
defines noinlining as:

The runtime shall not expand the method inline.

Furthermore, in the same partition, Section 15.4.3.3 ("Implementation
information"), it goes on to define the "noinlining" flag:

noinlining specifies that the body of this method should not be included
into the code of any caller methods, by a CIL-to-native-code compiler; it
shall be kept as a separate routine.

Continuing in the "synchronized" section:

noinlining specifies that the runtime shall not inline this method. Inlining
refers to the process of replacing the call instruction with the body of the
called method. This can be done by the runtime for optimization purposes.

Basically, the CLI spec indicates that a valid implementation of the CLR
will honor the attribute, and it is not an implementation detail. That
being said, it will honor the copy of the parameter to a method that is
inlined, and produce the same effect with less code.
> With your way of doing it, though, you gain the advantage of
encapsulating the lock object that is used on the event. In delcaring
the
event normally, you end up synchronizing on this, or the Type of the
object.
I would say that it is a judgement call whether or not you want to
maintain
that much code for that benefit.

Well, I'm kinda anal about thread safety...
Yeah, well, no need to remind me =P
--
- Nicholas Paldino [.NET/C# MVP]
- mv*@spam.guard.caspershouse.com
>
--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too

Jul 8 '06 #14
Liewe
2 New Member
It is probably a big sin to just catch the NullReferenceException isnt it?
Jul 8 '06 #15
Nicholas Paldino [.NET/C# MVP] <mv*@spam.guard.caspershouse.comwrote:
Mine is safe and correct by the spec rather than just by the current
implementation. Inlining and thread safety are only related by virtue
of the current implementation, IMO.

Using the MethodImpl attribute is spec-safe as well. In Partition II,
Section 15.4.3, the definition for the flags for ImplAttr (which is mapped
from the MethodImplOptions enumeration passed to MethodImplAttribute)
defines noinlining as:

The runtime shall not expand the method inline.
<snip>
Basically, the CLI spec indicates that a valid implementation of the CLR
will honor the attribute, and it is not an implementation detail. That
being said, it will honor the copy of the parameter to a method that is
inlined, and produce the same effect with less code.
It will honour the method not being inlined, but that doesn't
necessarily mean that there will be memory barriers. There's nothing in
the memory model which specifies that there must be a memory barrier at
method call boundaries, as far as I'm aware.

--
Jon Skeet - <sk***@pobox.com>
http://www.pobox.com/~skeet Blog: http://www.msmvps.com/jon.skeet
If replying to the group, please do not mail me too
Jul 8 '06 #16

This thread has been closed and replies have been disabled. Please start a new discussion.

Similar topics

5
15673
by: GIMME | last post by:
One of my coworkers insists that one should never use static methods because race conditions exist. Thinking along the lines that all variable assignments are assignments to static variables. ...
18
5831
by: Urs Vogel | last post by:
Hi I wrote an application server (a remoting sinlgeton), where processes must be stopped in very rare cases, done thru a Thread.Abort(). Occasionally, and only after a Thread.Abort(), this...
2
4933
by: John Jenkins | last post by:
Does anyone have any introductory references to either WS-Addressing, or WS-Eventing, with perhaps worked examples?? Thanks
1
7539
by: Stefan Lischke | last post by:
hi there, I have a lot of problems generating code for the new "WS-Eventing" Specification http://schemas.xmlsoap.org/ws/2004/08/eventing/ Is it possible, that after IBM joined this spec....
1
1462
by: rawCoder | last post by:
Hi, How does WS Eventing works around the problem of the subscriber client being a Winform Application and behind firewall / Proxy server? In Other Words ... If I have a Windows Form...
0
2019
by: Stefan Lischke | last post by:
Hi, I'm really desperate using code generation(wsdl.exe) from wsdl files for latest WS-Eventing(including WS-Addressing) Specs. I'm writing my diploma about "publish subscribe systems based on...
2
3632
by: manuelg | last post by:
Here is a code fragment, where I am trying to copy a file, avoiding overwrites and race conditions. The filename gets a '02','03','04' etc appended to the end if a file with that name already...
0
903
by: MichaelPhillipson | last post by:
Hello, I am planning and researching an application in which a given number of client applications would register interest with a web service, this web service would then monitor a value and...
5
2262
by: mars | last post by:
I use TurboGears to do some web service. TurboGears use cherrypy. When web browser access this site, the cherrypy will call my python program. So my program looks like a lib. When web browser...
0
1807
by: moltendorf | last post by:
I've been trying to find a suitable method for preventing race conditions in my own code. Currently I'm using a file and the flock function to prevent code in other threads from executing at the...
0
7120
marktang
by: marktang | last post by:
ONU (Optical Network Unit) is one of the key components for providing high-speed Internet services. Its primary function is to act as an endpoint device located at the user's premises. However,...
0
6991
by: Hystou | last post by:
Most computers default to English, but sometimes we require a different language, especially when relocating. Forgot to request a specific language before your computer shipped? No problem! You can...
0
7160
Oralloy
by: Oralloy | last post by:
Hello folks, I am unable to find appropriate documentation on the type promotion of bit-fields when using the generalised comparison operator "<=>". The problem is that using the GNU compilers,...
1
6878
by: Hystou | last post by:
Overview: Windows 11 and 10 have less user interface control over operating system update behaviour than previous versions of Windows. In Windows 11 and 10, there is no way to turn off the Windows...
0
7373
tracyyun
by: tracyyun | last post by:
Dear forum friends, With the development of smart home technology, a variety of wireless communication protocols have appeared on the market, such as Zigbee, Z-Wave, Wi-Fi, Bluetooth, etc. Each...
0
5456
agi2029
by: agi2029 | last post by:
Let's talk about the concept of autonomous AI software engineers and no-code agents. These AIs are designed to manage the entire lifecycle of a software development project—planning, coding, testing,...
1
4897
isladogs
by: isladogs | last post by:
The next Access Europe User Group meeting will be on Wednesday 1 May 2024 starting at 18:00 UK time (6PM UTC+1) and finishing by 19:30 (7.30PM). In this session, we are pleased to welcome a new...
0
3088
by: TSSRALBI | last post by:
Hello I'm a network technician in training and I need your help. I am currently learning how to create and manage the different types of VPNs and I have a question about LAN-to-LAN VPNs. The...
0
1405
by: 6302768590 | last post by:
Hai team i want code for transfer the data from one system to another through IP address by using C# our system has to for every 5mins then we have to update the data what the data is updated ...

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.