C# Strange behavior of CustomAttribute

Question

Inside my application I encountered a strange situation with CustomAttributes and Reflection that I can't understand, I tried to reduce the problem. Suppose that I have following custom attributes:

class A : Attribute
{
    public virtual string SayHi()
    {
        return "Hi From A";
    }
}
class B : A
{
    public override string SayHi()
    {
        return "Hi From B";
    }
}

Following classes are decorated with custom attributes:

[A]
class X
{ }

[B]
class Y
{ }

In following method I map each type of classes decorated with "A" attribute to a function that returns the value returned by its custom attribute:

static Dictionary<Type, Func<string>> dic = new Dictionary<Type, Func<string>>();
static void func()
{
    A attr;
    foreach (var type in typeof(Program).Assembly.GetTypes())
    {
        var attrs = type.GetCustomAttributes(typeof(A)).ToList();
        if(attrs.Any())
        {
            attr = attrs.First() as A;
            dic.Add(type, () => attr.SayHi());
        }
    }
}

The function mapped to type of X might return "Hi From A" but strangely the following code prints "Hi From B" to the console!

func();
Console.WriteLine(dic[typeof(X)]());

Am I missing a language feature?

_"Am I missing a language feature?"_ -- you have made the classic "captured the same variable multiple times" error. See marked duplicate. You need to declare `attr` _inside_ the loop, so that each of your lambdas added to the dictionary is using a different variable. — Peter Duniho, Jan 28 '18 at 07:01

StriplingWarrior · Accepted Answer · 2018-01-28T07:04:46.530

1

This behavior has nothing to do with attributes. It's a classic "captured variable" problem. You're declaring the attr variable outside of your foreach loop, and then referring to it inside of a delegate, so every function in the dictionary will end up referring to the last value that attr ends up with after running through the foreach.

A simpler reproduction looks like this:

int x;
var actions = new List<Action>();
for (int i = 0; i < 3; i++)
{
    x = i;
    actions.Add(() => Console.WriteLine(x));
}
foreach (var action in actions)
{
    action();
}

Output:

2
2
2

If you move your declaration into the loop, you'll get the behavior you're looking for.

static void func()
{
    foreach (var type in typeof(Program).Assembly.GetTypes())
    {
        var attrs = type.GetCustomAttributes(typeof(A)).ToList();
        if(attrs.Any())
        {
            A attr = attrs.First() as A;
            dic.Add(type, () => attr.SayHi());
        }
    }
}

edited Jan 28 '18 at 07:04

answered Jan 28 '18 at 06:57

StriplingWarrior

151,543
27
246
315

True attr is unnecessarily outside the loop, but it assigned 'attr = attrs.First() as A;' before being added to the dictionary. – Rob Smyth Jan 28 '18 at 06:58
@Rob: _"it assigned 'attr = attrs.First() as A;'"_ -- yes, and that's exactly the problem. the lambda is what's getting added to the dictionary, not the current value of the variable. The lambda captures the variable, and so each lambda uses the same variable, and so always uses the most recent value. – Peter Duniho Jan 28 '18 at 06:59
@RobSmyth: But `attr` isn't being added to the dictionary: a `Func` *referring* to `attr` is being added, and then `attr` is reassigned on a subsequent pass through the loop. – StriplingWarrior Jan 28 '18 at 07:01
Yes, you are right. Capturing seems odd to to me. BTW doesn't declaring variables outside of loops affect optimization of the code ? – Pharaz Fadaei Jan 28 '18 at 07:07
1

_"doesn't declaring variables outside of loops affect optimization of the code ?"_ -- affect, how, exactly? C# doesn't do (much) optimization at all. The JIT compiler can just as easily reuse a variable slot for variables inside loops as outside. In any case, if you are in general spending time moving variables around in a method to achieve some "optimization", you're focused on the wrong thing. – Peter Duniho Jan 28 '18 at 07:08
@Pharaz: Nope. That's a common misconception. See [this answer](https://stackoverflow.com/a/2388644/120955) of mine for details. You may also find [this question](https://stackoverflow.com/q/8898925/120955) and its answers interesting. – StriplingWarrior Jan 28 '18 at 07:10
Sorry: [this answer](https://stackoverflow.com/a/7383090/120955) is actually much more detailed. – StriplingWarrior Jan 28 '18 at 07:12

C# Strange behavior of CustomAttribute

1 Answers1