I just found (the hard way) a nasty bug in managed DirectX (all versions I think). Why nasty? Because it happens more or less randomly after certain amount of time (2-10hrs) on some computers and it doesn’t say much when exception is thrown (Invalid call or something). So, where is the problem?
Imagine you have a shader Effect defined somewhere in your code and you set its properties via SetValue method using string as a parameter name:
Effect someEffect = ... someEffect.SetValue("SomeEffectParameter", someValue);
Can you spot the problem? Of course not, neither could I. It is just the fact that application crashes randomly on SetValue invocation for no apparent reason. Granted, you have to call it many many times, but this is normal when using effects – you would set properties on every render pass.
Why does it crash? Good question, it took me quite a long time to find out – note that there was no good explanation plus a lengthy process to make it crash. I suspected hardware, graphic card driver and, of course, (managed) direct X. Having a faulty hardware is a possibility but you can pretty fast rule it out by switching hardware and re-test it. The problem was that crash was random so you couldn’t be 100% sure. I 99% ruled out graphic card drivers by testing various versions. This is a very slim chance as gamers would notice problems in driver before you.
That leaves us with DirectX. Or better, Managed DirectX. I soon realized that the only chance to understand what’s going on is by using Reflector. I first look at definition of SetValue method. It is a method of inherited BaseEffect class. It contains plenty of overloads (an overload per value type). The interesting fact that immediately drawn my attention is that no overload method accepts string as parameter name. Instead all methods do accept a EffectHandler class. How is it possible to pass a string instead then? EffectHandler implement = operator overloading that converts string to EffectHandle instance based on passed string. So far, so good. A look on EffectHandle implementation revealed that it consumes unmanaged memory. See where this is getting? Furthermore EffectHandle doesn’t implement IDisposable thus you can’t release unmanaged memory when you want (it implements a finalizer though). It is released if/when Garbage Collector’s is in the mood of running in its finalizer – IOW randomly. But let’s go back to EffectHandle’s = operator overload. Here is what actually happens when you call SetValue with string argument as parameter name.
Implicitly, using = operator overloading, an instance of EffectHandle is crated for you and passed to SetValue. The reference to it is never stored and thus it becomes an orphan as soon as execution leaves SetValue method or even sooner. And within SetValue method it isn’t disposed, simply because it can’t be since it doesn’t implement IDisposable, nor unmanaged memory released in any other way. It is left wandering in memory waiting for GC collection. So the pile keeps growing and GC doesn’t even know that it consumes unmamanged memory (due to GC implementation in .net 1.1 – in .net 2.0 you can notify GC about how much unmanaged memory your instance uses – which is not implemented by EffectHandle in any way, too). After a while computer crashes…or not. Randomly, the worst developer’s nightmare.
The workaround is actually pretty simple. Create proper EffectHandle instances for each property you need at effect’s creation time and strictly use them for setting effect’s parameters. Never ever use strings instead.
Here is proper code.
Effect someEffect = ... // both lines below should run only when effect is created EffectHandle someEffectParameter = "SomeEffectParameter"; // implicit creation EffectHandle someEffectParameter = EffectHandle.FromString("SomeEffectParameter"); // explicit creation - I preffer this one someEffect.SetValue(someEffectParameter, someValue);
Perhaps [MS] did never want us to use SetValue with string parameter. Not really, even Tom Miller suggested this approach in his Managed DirectX 9 Kick Start book. I think that not implementing IDisposable interface on a class that uses unmanaged code is a huge design error. Combined with not releasing that memory within SetValue is a wicked combination that should be fixed asap. Or not, since Managed DirectX is no more. I hope that people behind XNA, successor to Managed DirectX, are more careful.