What's Wrong With This Code (#5)

Tuesday, September 12, 2006

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.


Comments
Mike Powell Tuesday, September 12, 2006
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].
Joseph Tuesday, September 12, 2006
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.
cmyworld Tuesday, September 12, 2006
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.
Jason Kemp Tuesday, September 12, 2006
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)
enis Tuesday, September 12, 2006
Problem is in anonymous method delegate.
Ido Samuelson Tuesday, September 12, 2006
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.

inoodle Tuesday, September 12, 2006
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 ?
Ido Samuelson Tuesday, September 12, 2006
One mistake though, I wrote Bowler nested class which is in fact Test nested class.
Well, am still not a compiler :)
Wayne Howarth Tuesday, September 12, 2006
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.
R. Aaron Zupancic Tuesday, September 12, 2006
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.
Israel Aece Tuesday, September 12, 2006
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,
scott Tuesday, September 12, 2006
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.
Atilla Tuesday, September 19, 2006
I am not sure but, "Name" property in the BowlerAddedEventArg is not declared.
Comments are now closed.
by K. Scott Allen K.Scott Allen
My Pluralsight Courses
The Podcast!