What's Wrong With This Code (#5)

Joe Developer is working on a bowling program (again). Joe wrote the following code.

using System;
using System.Collections.Generic;

[
Serializable]
class Bowlers
{
    
List<string> _bowlerList = new List<string>();

    
public void AddBowler(string name)
    {
        _bowlerList.Add(name);

        
EventHandler<BowlerAddedEventArgs> handler = BowlerAdded;
        
if (handler != null)
        {
            handler(
this, new BowlerAddedEventArgs(name));
        }
    }

    
public event EventHandler<BowlerAddedEventArgs> BowlerAdded;

    
// ...
}

[
Serializable]
class BowlerAddedEventArgs : EventArgs
{
    
public BowlerAddedEventArgs(string name)
    {
        Name = name;
    }

    
public string Name;
}

Joe unit tested the code to within an inch of its life, so he was surprised when another developer wrote the following program, which throws an exception.

using System;
using System.IO;
using System.Runtime.Serialization.Formatters.Binary;

class Test
{
    
public static void Main()
    {
        
Bowlers bowlers = new Bowlers();

        
string addedMessage = "Added bowler: {0}";
        bowlers.BowlerAdded +=
            
delegate(object sender, BowlerAddedEventArgs e)
            {
                
Console.WriteLine(addedMessage, e.Name);
            };

        bowlers.AddBowler(
"Bob");
        bowlers.AddBowler(
"Jan");
        bowlers.AddBowler(
"Ann");

        
using (MemoryStream stream = new MemoryStream())
        {
            
BinaryFormatter formatter = new BinaryFormatter();
            formatter.Serialize(stream, bowlers);
        }
    }
}  

What's wrong?

Hint: the exception is a strange looking serialization exception.

posted on Monday, September 11, 2006 9:25 PM by scott

Comments

Monday, September 11, 2006 8:24 PM by Mike Powell

# re: What's Wrong With This Code (#5)

I don't know what the exception is exactly, but I bet it has to do with the BinaryFormatter trying to serialize the anonymous delegate you added to your Bowlers instance. The solution would be to decorate the BowlerAdded event with [NonSerialized].
Monday, September 11, 2006 9:02 PM by Joseph

# re: What's Wrong With This Code (#5)

When the class is attempted to be serialized with a non-serialisable subscriber, an serialization error will occur.

To get around it you have to have a private event that is marked as [NonSerialized] and then have a public add/remove.

If the subscriber is serializable, then it will be serialized in the object graph, which is usually not what is wanted.
Monday, September 11, 2006 9:07 PM by cmyworld

# re: What's Wrong With This Code (#5)

I beleive exception is being thrown because event delegate reference is getting serialize and this reference in turn contains reference to the Test class in some way,and has not been marked serializable.
Monday, September 11, 2006 10:30 PM by Jason Kemp

# re: What's Wrong With This Code (#5)

I had to run the code to figure it out, but the anonymous delegate in this case is a "hard anonymous method", as phrased by Raymond Chen, because of the reference to the addedMessage variable of the enclosing method. A hard anonymous method causes the compiler to create a helper class, a class that isn't marked as Serializable. Because one has been added to the BowlerAdded event of the Bowlers class, the serializer will attempt to serialize the instance of the compiler-generated helper class. Exception!

So how do you solve it?

Two ways:
1. Inline the bowlerAdded message in the WriteLine call (the thing to do in apps); or
2. Mark the event as [field: NonSerialized] in the Bowlers class (the thing to do in frameworks)
Monday, September 11, 2006 11:47 PM by enis

# re: What's Wrong With This Code (#5)

Problem is in anonymous method delegate.
Tuesday, September 12, 2006 12:04 AM by Ido Samuelson

# re: What's Wrong With This Code (#5)

The problem is that anonymous delegate create a nested class with the delegate method. The problem is that this class is not marked as serializable thus you will not able to serialize Bowler.

Tuesday, September 12, 2006 12:05 AM by inoodle

# re: What's Wrong With This Code (#5)

oo oo I might know this one :) *fingers crossed*

I'd guess it's to do with the fact that the compiler generated class for the anonymous delegate is NOT marked as serializable by the compiler ?
Tuesday, September 12, 2006 12:09 AM by Ido Samuelson

# re: What's Wrong With This Code (#5)

One mistake though, I wrote Bowler nested class which is in fact Test nested class.
Well, am still not a compiler :)
Tuesday, September 12, 2006 1:10 AM by Wayne Howarth

# re: What's Wrong With This Code (#5)

The 'BowlerAdded' event field should ideally be marked as NonSerializable. This will prevent all objects that contain a method attached to the event handler from being serialized.

I'm no expert but in this example the delegate is anonymous and is contained within a static method which is possibly causing the problem.
Tuesday, September 12, 2006 3:01 AM by R. Aaron Zupancic

# re: What's Wrong With This Code (#5)

The main issue has to do with what's being serialized. By default, event handlers are internally represented by a compile-time generated field. This field holds a reference to the delegate(s) to be invoked when the event is raised.

Using your example above, the exception is caused because you're using an anonymous method that accesses resources beyond its defined scope. Under the covers, a class (probably called "<>c__DisplayClass1") is created to represent the anonymous method. This method doesn't get marked with the [Serializable()] attribute. When you attempt to serialize your object it attempts to serialize its fields and the exception is thrown.

You can fix your code in one of several ways:

If you want to maintain serialization on the event (which is on by default for a Serializable class), the easiest thing to do is to move your 'addedMessage' variable into the anonymous method so that it doesn't access any local variables in the containing scope.

If serialization of the event isn't important to you, you can declare your event field manually, marking it with the [NonSerialized()] attribute and then use the add and remove accessors on the event block to manage delegate references.
Tuesday, September 12, 2006 4:21 AM by Israel Aece

# re: What's Wrong With This Code (#5)

Hello Scott,

I believe that the problem is in anonymous method because after compilation, it generate a class and function that aren't marked with Serializable attribute.

Regards,
Tuesday, September 12, 2006 6:21 AM by scott

# re: What's Wrong With This Code (#5)

The real problem is, as some pointed out, that the C# compiler generates a class (with a name like "<>C_DisplayClass1" for the anonymous method. This class is referenced by the Bowlers class since we hook up the anon delegate as an event handler. Since the class is not marked as serializable, the binary formatter throws an exception!

As Jason and Aaron pointed out, using [NonSerialized] or [field: NonSerialized] on the event field would prevent this problem.
Tuesday, September 12, 2006 7:17 PM by Christopher Steen

# Link Listing - September 12, 2006

&quot;Atlas&quot; 1.0 Naming and Roadmap [Via: ScottGu ] BetaMarker.com - Early adopters welcome [Via: RoyOsherove...
Tuesday, September 19, 2006 3:44 AM by Atilla

# re: What's Wrong With This Code (#5)

I am not sure but, "Name" property in the BowlerAddedEventArg is not declared.