Raising events

  • Thread starter Thread starter CuriousGeorge
  • Start date Start date
C

CuriousGeorge

Can someone explain why this code DOES NOT raise a null reference exception?


//////////////////////////// Program.cs
/////////////////////////////////////////////
using System;
using System.Collections.Generic;
using System.Text;
using CSharpLib;

namespace CSharpConsole
{
class Program
{
private EventHandler handler;
private Class1 obj;

static void Main(string[] args)
{
(new Program()).RunTest();
Console.ReadKey(true);
}

private Program()
{
obj = new Class1();
handler = new EventHandler(DoSomething);
}

private void RunTest()
{
obj.SomeEvent += handler;

obj.RaiseEvent(delegate { obj.SomeEvent -= handler; });
}

private void DoSomething(object sender, EventArgs e)
{
Console.WriteLine("foo");
}
}
}


//////////////////////////// Class1.cs
/////////////////////////////////////////////
using System;
using System.Collections.Generic;
using System.Text;

namespace CSharpLib
{
public class Class1
{
public event EventHandler SomeEvent;

public delegate void Callback();

public Class1()
{
}

public void RaiseEvent(Callback callback)
{
EventHandler temp = SomeEvent;

callback();

if (temp != null)
{
temp(this, EventArgs.Empty);
}
}
}
}
 
CuriousGeorge said:
Can someone explain why this code DOES NOT raise a null reference
exception?
Click to expand...

Because no nulls are referenced?

If you have reason to believe that a null reference exception should occur,
then the least you could do is explain in your post:
a) why it is you want to execute code like this, and
b) why and where it is that you think a null reference exception would
occur

Pete
 
Hi,

Where do you expect a null reference exception to be raised?

This line is checking to make sure temp is not null before it's used in
Class1:

if (temp != null)
 
Because no nulls are referenced?
Hmmm. OK.

a) "Code like this" is the preferred way to raise events in a thread safe
manor - you should be doing it yourself.

b) Events are basically delegates, System.Delegate is a reference type,
there's nothing special about it, it overloads += and -= but other than that
its a pretty vanilla type.

As we know and event with no subscribers is actually a null-reference hence
the check for nullility before invoking the subscribed handlers.

If you in-line the delegate call in my original post you get something like
the following (compiled in Outlook Express)

public void RaiseEvent(Callback callback)
{
EventHandler temp = SomeEvent;

SomeEvent -= theOnlyRegisteredEventHandler
//SomeEvent is now null

if (temp != null)
{
temp(this, EventArgs.Empty);
}
}

The code in my original post draws the understanding reader to the
conclusion that this particular reference type is not obeying standard
reference type semantics. I thought it was pretty self-explanatory.
 
Hi,
a) "Code like this" is the preferred way to raise events in a thread safe
manor - you should be doing it yourself.
Click to expand...

I don't see anything particularly thread safe about your code, are you
talking about checking for null that makes this thread safe? It is safe for
a single thread but once you have multiple threads the really only safe way
is to make sure you have locks in place around adding/removing delegates to
your event and the invocation of that event i.e. the following is an example
of how two threads accessing the same event can cause a null reference
exception:

using System.Threading;

namespace CSharpConsole
{
class Program
{
static void Main(string[] args)
{
TestClass t = new TestClass();
t.Run();
}
}

class TestClass
{
private delegate void MyEventHandler();
private event MyEventHandler MyEvent;

public TestClass()
{
}

public void Run()
{
Thread threadA = new Thread(new ThreadStart(ThreadADoSomething));

//Add listener to the event
MyEvent += new MyEventHandler(DoSomething);

if (MyEvent != null)
{
//got in here, let the other thread run and wait until it
//completes
threadA.Start();
threadA.Join();

//whoops now this is null
MyEvent();
}
}

void ThreadADoSomething()
{
//removes the only listener
MyEvent -= new MyEventHandler(DoSomething);
}

void DoSomething()
{
}
}
}
b) Events are basically delegates, System.Delegate is a reference type,
there's nothing special about it, it overloads += and -= but other than that
its a pretty vanilla type.
Click to expand...

There is a little bit more to this type, hence your posting :-) If you look
at the documentation for the delegate type you will see: "Delegates are
immutable; once created, the invocation list of a delegate does not change."
Hence the reason why when you assign you event (which is really just
syntactic sugar around delgates) to your other delegate type really you have
just made a new copy of that delegate. The contents of the
multicastdelegates invocation list inside the temp variable will therefore
not change when you update the other multicastdelegate through the SomeEvent
enty point.

The code in my original post draws the understanding reader to the
conclusion that this particular reference type is not obeying standard
reference type semantics. I thought it was pretty self-explanatory.
Click to expand...

Why not just ask the question you want to ask :-)
 
"Delegates are immutable; once created, the invocation list of a delegate
does not change."

Word. Thanks Mark, I hadn't seen that bit of doco. The thing is, I did skim
the (managed) rotor source for delegates and saw nothing that suggested
immutability in the implementation. Forgive my ignorance but where in the IL
can I see this? (is it a type decorator)

"Why not just ask the question you want to ask :-)"

Quite simply, I don't have the expertise to express myself very well.

Case in point, immutability, I see it, I hear it, it is said by the oracle,
MSDN, however, the upshot is (to all intents and purposes) that the
assignment operator behaves like it is overloaded such that it works like a
value type. I have taken this as gospel for years with the System.String
class but now I see it here and it is making my head spin!!

Words fail me.

Please help :)


Mark R. Dawson said:
Hi,
a) "Code like this" is the preferred way to raise events in a thread safe
manor - you should be doing it yourself.
Click to expand...

I don't see anything particularly thread safe about your code, are you
talking about checking for null that makes this thread safe? It is safe
for
a single thread but once you have multiple threads the really only safe
way
is to make sure you have locks in place around adding/removing delegates
to
your event and the invocation of that event i.e. the following is an
example
of how two threads accessing the same event can cause a null reference
exception:

using System.Threading;

namespace CSharpConsole
{
class Program
{
static void Main(string[] args)
{
TestClass t = new TestClass();
t.Run();
}
}

class TestClass
{
private delegate void MyEventHandler();
private event MyEventHandler MyEvent;

public TestClass()
{
}

public void Run()
{
Thread threadA = new Thread(new
ThreadStart(ThreadADoSomething));

//Add listener to the event
MyEvent += new MyEventHandler(DoSomething);

if (MyEvent != null)
{
//got in here, let the other thread run and wait until it
//completes
threadA.Start();
threadA.Join();

//whoops now this is null
MyEvent();
}
}

void ThreadADoSomething()
{
//removes the only listener
MyEvent -= new MyEventHandler(DoSomething);
}

void DoSomething()
{
}
}
}
b) Events are basically delegates, System.Delegate is a reference type,
there's nothing special about it, it overloads += and -= but other than
that
its a pretty vanilla type.
Click to expand...

There is a little bit more to this type, hence your posting :-) If you
look
at the documentation for the delegate type you will see: "Delegates are
immutable; once created, the invocation list of a delegate does not
change."
Hence the reason why when you assign you event (which is really just
syntactic sugar around delgates) to your other delegate type really you
have
just made a new copy of that delegate. The contents of the
multicastdelegates invocation list inside the temp variable will therefore
not change when you update the other multicastdelegate through the
SomeEvent
enty point.

The code in my original post draws the understanding reader to the
conclusion that this particular reference type is not obeying standard
reference type semantics. I thought it was pretty self-explanatory.
Click to expand...

Why not just ask the question you want to ask :-)


--
http://www.markdawson.org
Posted from Windows Vista RC1



CuriousGeorge said:
Hmmm. OK.

a) "Code like this" is the preferred way to raise events in a thread safe
manor - you should be doing it yourself.

b) Events are basically delegates, System.Delegate is a reference type,
there's nothing special about it, it overloads += and -= but other than
that
its a pretty vanilla type.

As we know and event with no subscribers is actually a null-reference
hence
the check for nullility before invoking the subscribed handlers.

If you in-line the delegate call in my original post you get something
like
the following (compiled in Outlook Express)

public void RaiseEvent(Callback callback)
{
EventHandler temp = SomeEvent;

SomeEvent -= theOnlyRegisteredEventHandler
//SomeEvent is now null

if (temp != null)
{
temp(this, EventArgs.Empty);
}
}

The code in my original post draws the understanding reader to the
conclusion that this particular reference type is not obeying standard
reference type semantics. I thought it was pretty self-explanatory.
Click to expand...
Click to expand...
 
CuriousGeorge said:
a) "Code like this" is the preferred way to raise events in a thread safe
manor - you should be doing it yourself.
Click to expand...

I disagree. As Mark points out, there's nothing thread-safe about the code
that you posted.
[...]
The code in my original post draws the understanding reader to the
conclusion that this particular reference type is not obeying standard
reference type semantics. I thought it was pretty self-explanatory.
Click to expand...

Well, hopefully at this point, given the responses you've seen, it should be
clear to you that it was NOT "pretty self-explanatory".

On the bright side, it looks like you did finally get the answer to your
question. :)

Pete
 
CuriousGeorge said:
Hmmm. OK.

a) "Code like this" is the preferred way to raise events in a thread safe
manor - you should be doing it yourself.
Click to expand...

It's not, actually. Your code isn't genuinely thread-safe. See
http://www.pobox.com/~skeet/csharp/threads/lockchoice.shtml for a
genuinely thread-safe pattern.
b) Events are basically delegates
Click to expand...

No they're not. Events are pairs of methods. Field-like events declare
both an event and a delegate.
System.Delegate is a reference type,
there's nothing special about it, it overloads += and -= but other than that
its a pretty vanilla type.
Click to expand...

Actually, it *doesn't* overload += and -=. Instead, the C# compiler
calls Delegate.Combine or Delegate.Remove, both of which are static
methods which can take null arguments.

See http://www.pobox.com/~skeet/csharp/events.html for more
information.
 
Peter Duniho said:
I disagree. As Mark points out, there's nothing thread-safe about the code
that you posted.
Click to expand...

In fairness to the OP, I wouldn't go that far. It's not *quite* thread-
safe, but it's safer than the normal pattern of:

if (EventName != null)
{
EventName (...);
}

where "EventName" is a field-like event.

The problem with the above is that another thread could change the
value of myEventName to null between the check and the call. The OP's
code tries to avoid that by using a local variable. Now, there's some
debate as to the exact guarantees of behaviour of local variables in
the memory model, but it's at least a reasonable start. However, the
lack of locking and volatility means that if one thread adds a load of
event handlers, they may never be noticed by the thread raising the
event, which isn't generally what's wanted.
 
Jon, your arachsys stuff is, quite simply, GOLD

Jon Skeet said:
It's not, actually. Your code isn't genuinely thread-safe. See
http://www.pobox.com/~skeet/csharp/threads/lockchoice.shtml for a
genuinely thread-safe pattern.


No they're not. Events are pairs of methods. Field-like events declare
both an event and a delegate.


Actually, it *doesn't* overload += and -=. Instead, the C# compiler
calls Delegate.Combine or Delegate.Remove, both of which are static
methods which can take null arguments.

See http://www.pobox.com/~skeet/csharp/events.html for more
information.
Click to expand...
 
Back
Top