« Notes on Puzzle Quest | Main | Stupid C# Tricks - readonly reflection - Continued »

May 23, 2007

Stupid C# Tricks

We're cracking down on static variables in Schizoid so we can feel more comfortable that there aren't lurking thead safety issues in there.  Some of these statics are effectively constants, but we made them non-constant to ease unit testing.  For example, we might set a unit cap at 1 in a test, create two units, and make sure the cap is being respected.

Unfortunately, now there's no way to make sure that an Evil Programmer doesn't violate the effective const-ness of this variable...and possibly endanger our thread safety.

(In case you're about to say "Why not make it private and add Get/Set accessors", shame on you for parrotting rote "good style" guidelines that don't actually do anything to protect you.  One could then use the Set and violate our thread safety that way.)  In a better world, C# would have friends.  We could make the test fixture a friend of the class with the static variable and walk away happy. 

You can have friend DLL's - I could just see myself breaking my project down into dozens of small DLL's just to get the kind of granularity of friendliness that I want.  A lot of DLL's seems to hurt our build time, however.

I came across a possible answer in *CLR via C#* today - "Note that reflection can be used to modify a readonly field," Richter says.

That's good enough, I thought.  I'll make my consts readonly, use reflection to modify them in the test program, and assume that no programmer will be So Evil that they do the same thing in the core code.  Too much work!  The forces of Evil, after all, are lazy.

Unfortunately, it didn't work.  Maybe a C# guru in the audience can tell me why.

Here's the code. Enemy is the class which has the static readonly field maxEnemies.

            Type enemyType = typeof(Enemy); 
            FieldInfo maxEnemiesField = enemyType.GetField("maxEnemies");
            maxEnemiesField.SetValue(null, newMax);
            Assert.AreEqual(newMax, Enemy.maxEnemies);

When I step through this code in the debugger, Enemy.maxEnemies appears to correctly change to newMax after executing line 3.  But then the assertion on line 4 fires, telling me that it's still 100.

WTF?  The debugger sees one thing, the code another?

No doubt this is a subtle intracacy of the CLR that somebody out there understands?  Googling turned up a lot of noise for me, unfortunately.

BTW - this is all just academic.  We don't have any Evil Programmers, really.  We don't normally waste a lot of time to protect ourselves from bugs that nobody on the team is actually going to write.  But now I'm curious, damn it.





Comments

I seems like its because via reflection, you're creating and manipulating a separate instance of the enemy static class. Sounds weird, but assert on maxEnemiesField.GetValue instead of Enemy.maxEntries. You won't get the assertion. The weird thing is when stepping though in Visual Studio, watching Enemy.maxEnemies shows 4.

Someone else have a more concise explaination?

It appears to not work on statics. Quick test program.

using System;
using System.Reflection;
using System.Collections.Generic;
using System.Text;

namespace ReflectionTest
{
class Enemy
{
readonly public int maxEnemies = 4;
readonly public static int sMaxEnemies = 5;
}

class Program
{
static void Main( string[] args )
{
Enemy enemy = new Enemy();

Console.WriteLine( Enemy.sMaxEnemies.ToString() );
Console.WriteLine( enemy.maxEnemies.ToString() );


Type enemyType = typeof( Enemy );
FieldInfo maxEnemiesField = enemyType.GetField( "maxEnemies" );
FieldInfo sMaxEnemiesField = enemyType.GetField( "sMaxEnemies" );
maxEnemiesField.SetValue( enemy, 100 );
sMaxEnemiesField.SetValue( null, 100 );

Console.WriteLine( Enemy.sMaxEnemies.ToString() );
Console.WriteLine( enemy.maxEnemies.ToString() );
Console.ReadLine();
}
}
}

It's doing something weird with the field when you use the readonly keyword. Want an interesting twist? Change the assert to this:

Assert.AreEqual(newMax, maxEnemiesField.GetValue(null));

I'm far from a C# expert, but here is what I would look at.

Based upon this:
http://msdn2.microsoft.com/en-us/library/system.reflection.bindingflags.aspx

- GetField is described as "Specifies that the value of the specified field should be returned." So it may be returning by value rather than reference.

- Try GetProperty(). There is a difference between fields and properties in C# and I've been bitten by that before. I'm not exactly sure what the difference is in terms of language legalese, but there is a difference.

Why not just put said values into assembly resources or use build-time dependent values for the cases where you want your testing version different?

As for why your example doesn't work: In my experience you generally need to send the right BindingFlags using the long form of SetValue to do things via Reflection that you wouldn't normally be allowed to do, or to do things with static fields. Try using BindingFlags.Static | BindingFlags.NonPublic. BindingFlags is an ugly, ugly part of Reflection:

http://blogs.msdn.com/brada/archive/2005/11/09/475090.aspx

For toying with readonly things you may even need to use the SetValueDirect, which is waters I've never tread...

Did you try with 'Assert.IsTrue(newMax == Enemy.maxEnemies, "Eureka!");'?

The C# compiler is optimizing the readonly field as if it was a constant. Try it with a struct:


using System;
using System.Reflection;

class Program
{
public static readonly TimeSpan SomeSpan = TimeSpan.FromSeconds(1);

static void Main(string[] args)
{
Type type = typeof(Program);
FieldInfo someSpanField = type.GetField("SomeSpan");

Console.WriteLine(SomeSpan);
Console.WriteLine(someSpanField.GetValue(null));

someSpanField.SetValue(null, TimeSpan.FromSeconds(2));

Console.WriteLine(SomeSpan);
Console.WriteLine(someSpanField.GetValue(null));

Console.ReadLine();
}
}

Hey,

Your post got me interested. I get a System.FieldAccessException {"Cannot set a constant field."} on the SetValue line, in what I wrote to try to reproduce what your seeing.

I've seen some very weird things in .NET, but this is extremely weird...

The reason I'm interested, is because I would assume that the constant value would end up in an internal symbol table somewhere in the CLR. Meaning (IMO, from what I know) it shouldn't let you mess with it's internal symbol table - so shouldn't let you change a constant value.

The code I used couldn't get much simpler...
static void Main(string[] args) {


Type t = typeof(Enemy);

FieldInfo f = t.GetField("MaxEnemies");
f.SetValue(null, 10);

if (Enemy.MaxEnemies != 10)
throw new Exception();
}
}

public class Enemy {
public const int MaxEnemies = 200;
}

I'd love to know whats going on...

I haven't taken the time to test this, but I believe that 'const' values are/can be inlined by the compiler. (Not sure how you'd actually access the 'real' value other than via reflection then).

My first guess at a fix would be to try making the fields 'readonly' the difference being that readonly fields are not know at compile time and therefore I don't believe they can be inlined, unless the CLR can do it during JIT.

I would imagine the reason is that the compiler has rolled the constant as an immediate into the generated code, and you are just changing an in-memory version of the constant that is only used when accessed through a reference. Ideally, if you never take a reference to that constant, it could be stripped out of the final executable, but I guess the possibility of indirect access via reflection prevents this from being done. I don't know much about CLR either (or C# for that matter), but these are the things I would expect to happen in C++. It's also possible that I am completely misunderstanding the problem - I was bummed enough when I found out about the
mess of const-ness in C#.

On another note, accessors are there precisely to give you the chance to do additional stuff when a member is accessed. Plain blind Get/Set accessors that simple return or assign the member are not useful at all in C#, since they are 100% identical to just making the member public. As a style, they probably come from C++ where accessor methods must have a different name from the member variable itself.

That's proably because the compiler inlined the value of Enemy.maxEnemies, i.e. it generates the same code as if you had written a numeric literal. I know that Java compilers do this, so I guess C# compilers do it too. It can give a big performance boost in some circumstances, but it will bite you in special cases like this.

that SetField() should take the owner object in question: SetField(Enemy, newMax);

the field info is pulled from the type, so it doesn't have any instance info itself.

one other option:

contained classes should have access to their container class's protected variables so:
* declare a UnitTest class inside of the EnemyType class,
* provide gets to your variables via the EnemyType class,
* provide sets via the UnitTest class,
* add a single method on the EnemyType class to get an instance of EnemyType.UnitTest
* wrap that EnemyType.GetUnitTest() with a preprocessor #if UNIT_TESTS_ENABLED ( or some such ).

Test app: DEBUG, Jit Optimisations disabled, IL optimisations dissabled

class Program {
public static readonly int Foo = 1;
static void Main(string[] args) {
Console.WriteLine(Foo);
}
}

Dissasembly:


class Program {
public static readonly int Foo = 1;
static void Main(string[] args) {
00000000 push ebp
00000001 mov ebp,esp
00000003 push edi
00000004 push esi
00000005 push ebx
00000006 sub esp,30h
00000009 xor eax,eax
0000000b mov dword ptr [ebp-10h],eax
0000000e xor eax,eax
00000010 mov dword ptr [ebp-1Ch],eax
00000013 mov dword ptr [ebp-3Ch],ecx
00000016 cmp dword ptr ds:[00918868h],0
0000001d je 00000024
0000001f call 7943113E
00000024 nop

Console.WriteLine(Foo);
00000025 mov ecx,1
0000002a call 78757DDC

0000002f nop
}
00000030 nop
00000031 lea esp,[ebp-0Ch]
00000034 pop ebx
00000035 pop esi
00000036 pop edi
00000037 pop ebp
00000038 ret

the line : 00000025 mov ecx,1
clearely shows what's happening

Obviously the Jit compiler is verry smart and it simply reads the value when it jits the method and doesn't emit mem read opcode.

One ugly solution of using readonly and unitesting coud be using conditonal statemets around readonly :

#define UnitTest
...
public static
#if !UnitTest
readonly
#endif
int Foo = 4;

Post a comment

If you have a TypeKey or TypePad account, please Sign In