sobota, 10 października 2009

I am the key to the lock in your house or some thoughts about reflection and Singleton pattern

I wanted to present some thoughts about the Singleton pattern and assumptions you can make about it. This post also shows that using reflection is like going to the darkside ;)

Say, we have a class SingletonFoo that manipulates some PreciousResource. Access to PreciousResource must be synchronized and manipulation should be performed in critical section.
public class SingletonFoo : ObjectWithIdentifier
{
    private static int instancesCount;
    private static PreciousResource preciousResource = new PreciousResource();
    private static SingletonFoo instance = new SingletonFoo();        

    // Explicit static constructor to tell C# compiler
    // not to mark type as beforefieldinit
    static SingletonFoo() { }
    
    private SingletonFoo()
    {
        instancesCount++;
    }

    public static SingletonFoo Instance { get { return instance; } }        

    public void ManipulatePreciousResource()
    {
        // no need for locking, we are in a singleton!            
        Console.WriteLine("{0} is accessing the resource {1}", this, preciousResource);
        preciousResource.Manipulate();
    }

    public override string ToString()
    {
        return String.Format("{0} [instances count: {1}]", base.ToString(), instancesCount);
    }
}
For clarification here is ObjectWithIdentifier:
public class ObjectWithIdentifier
{
    private Guid uniqueId = Guid.NewGuid();
    public Guid UniqueId { get { return uniqueId; } }

    public override string ToString()
    {
        return String.Format("{0}, id: {1}", this.GetType().Name, uniqueId);
    }
}
ObjectWithIdentifier is only necessary for giving more meaningful output from the application.
We have a situation when someone creates Singleton and assumes that he may be sure that only one instance of this class exists in the system, so he doesn't have to synchronize access to some PreciousResource:
public class PreciousResource : ObjectWithIdentifier
{
    public void Manipulate()
    {
        //do something
    }
}
However, here comes the evil doer:
public static class EvilBar
{
    const int REPEAT_COUNT = 2;

    public static void BeNice()
    {
        Console.WriteLine("Now I'm nice");
        for (int i = 0; i < REPEAT_COUNT; i++)
        {
            SingletonFoo.Instance.ManipulatePreciousResource();
        }
    }

    public static void BeEvil()
    {
        Console.WriteLine("Now I'm evil!");
        for (int i = 0; i < REPEAT_COUNT; i++)
        {
            ((SingletonFoo) Activator.CreateInstance(typeof(SingletonFoo), true)).ManipulatePreciousResource();
        }
    }
}
When EvilBar is nice we get following output:
Now I'm nice

SingletonFoo, id: 7dbccd1c-4e74-4e1d-b762-104f4cbcb82b [instances count: 1] is accessing the resource PreciousResource, id: c2b4a8b7-80e8-46cf-b616-a3f45f0612d7
SingletonFoo, id: 7dbccd1c-4e74-4e1d-b762-104f4cbcb82b [instances count: 1] is accessing the resource PreciousResource, id: c2b4a8b7-80e8-46cf-b616-a3f45f0612d7
But what happens if EvilBar is trully evil:
Now I'm evil!

SingletonFoo, id: 5a829a65-580f-4cc2-b17b-2650f68e464b [instances count: 2] is accessing the resource PreciousResource, id: 8417a76b-a40b-4221-bd73-0b507ff059c9
SingletonFoo, id: 744eba58-9aa9-4ee2-97aa-ffd815c3832f [instances count: 3] is accessing the resource PreciousResource, id: 8417a76b-a40b-4221-bd73-0b507ff059c9
We have three instances of our Singleton class. One for singleton and two instance created with Activator.
Ok, the class was intentionally written to be evil. But was XmlSerializer written to be evil? Let's add another method to EvilBar:
public static void BeUnwittlinglyEvil(SingletonFoo foo)
 {
    Console.WriteLine("Now I'm unwittingly evil");
    Console.WriteLine("Foo before operations: {0}", foo);

    XmlSerializer serializer = new XmlSerializer(typeof(SingletonFoo));

    StringBuilder serialized = new StringBuilder();
    using (StringWriter sw = new StringWriter(serialized))
    {                
        serializer.Serialize(sw, foo);
    }

    SingletonFoo result;
    using (StringReader sr = new StringReader(serialized.ToString()))
    {
        result = (SingletonFoo) serializer.Deserialize(sr);
    }

    result.ManipulatePreciousResource();
}
After executing the method EvilBar.BeUnwittlinglyEvil(SingletonFoo.Instance), we get following output:
Now I'm unwittingly evil

Foo before operations: SingletonFoo, id: 5c4724bb-ac02-4300-9be6-70c86c5629fc [instances count: 1]
SingletonFoo, id: 58612540-6d6a-458a-8c0c-1f723117f9b6 [instances count: 2] is accessing the resource PreciousResource, id: e7a55b7f-4e62-4d1f-878c-499ec0387a31
The implementation looks weird, but that just an example what happens if someone serializes this class and then unserializes it. If the code is executed in different threads, you will run into synchronization issues.
In Pragmatic Programmer there is a nice quiz. Which of these things should never happen (answers are presented below the post):
1. A month with fewer than 28 days
2. stat("." ,&sb) == -1 (that is, can't access the current directory)
3. In C++: a = 2; b = 3; if (a + b != 5) exit(1);
4. A triangle with an interior angle sum \u8800 180°
5. A minute that doesn't have 60 seconds
6. In Java: (a + 1) <= a

I'm adding another thing to the list: singleton with more than one instance ;)
The point is, you always have to think about the things you can be sure about your code and about the ways people are using your code. The less you assume, the better.


List of things that should never happen:
1. September, 1752 had only 19 days. This was done to synchronize calendars as part of the Gregorian Reformation.
2. The directory could have been removed by another process, you might not have permission to read it, &sb might be invalid—you get the picture.
3. We sneakily didn't specify the types of a and b. Operator overloading might have defined +, =, or ! = to have unexpected behavior. Also, a and b may be aliases for the same variable, so the second assignment will overwrite the value stored in the first.
4. In non-Euclidean geometry, the sum of the angles of a triangle will not add up to 180°. Think of a triangle mapped on the surface of a sphere.
5. Leap minutes may have 61 or 62 seconds.
6. Overflow may leave the result of a + 1 negative (this can also happen in C and C++).

2 komentarze:

  1. based on this Singleton definition: "Ensure a class has only one instance and provide a global point of access to it." singleton gives you a class with two main responsibilities... and that's bad already, isn't it ?

    OdpowiedzUsuń
  2. I wonder if it is possible to divide these two responsibilities. Creating class instances and setting permissions to class methods (in this case constructor) cannot be achieved outside the given class. It could be possible in C++ using friend keyword, but I don't know how could it be possible in C# without using such tricks as described in this post.

    OdpowiedzUsuń