Handy delegate shortcut hides important details: The hidden delegate

One of my colleagues was having trouble with a little tool he wrote.

I installed a low-level keyboard hook following the code in this article, but it crashes randomly. Here's what I know so far:

  • I spawn a new STA thread to register the hook, so that it can run a message pump, which is a requirement for low-level hooks.

  • After setting the hook, the program waits on a Manual­Reset­Event with Wait­One(). Since this is being called from an STA thread, it will pump messages while waiting, which is what we want.

  • The event is signaled by another part of the program when the hook is no longer needed, at which point the thread unregisters the hook before exiting normally.

The crash happens inside Wait­One() immediately after keyboard activity occurs. The debugger tells me that it is crashing trying to dispatch a call into a managed stub via the message pump, but that's all I was able to extract.

I took a look at the article that my colleague referenced and observed that there was a subtlety in the code that not obvious, and which may have been lost in translation. I shared my observation in the form a psychic prediction.

My psychic powers tell me that you did not prevent the delegate from getting GCd. The next time GC runs, the delegate will get collected, and the next attempt to fire the callback will AV because its calling into memory that has been freed.

The sample code from the blog avoids this problem by putting the delegate in a private static, which makes it a GC root, ineligible for collection.

private static LowLevelKeyboardProc _proc = HookCallback;

This is subtle because the private static is decoupled from Set­Hook. If you copied Set­Hook but not the private static, then you inadvertently created a bug because local variables can get optimized out.

Either put it in a static, like the sample does, or explicitly extend the delegates lifetime by calling GC.Keep­Alive() after you unhook the hook.

LowLevelKeyboardProc proc = HookCallback;
IntPtr hookId = SetHook(proc);
GC.KeepAlive(proc); // keep the proc alive until this line is reached

My colleague realized that was the problem.

I'd actually thought of that (mostly). I made my callback method itself a static, thinking that this was enough. What I forgot is that C# wraps that in an instance of the delegate automatically, and it was this hidden delegate that was getting GC'd not the callback function itself. This explains why I could always inspect the callback method and see that it was alive and well, yet we were still jumping into space when invoking the callback.

Explicitly calling out the assignment reminded me of the details of delegates. Thanks!

The classical notation for creating a delegate is

    DelegateType d = new DelegateType(o.Method);
    DelegateType d = new DelegateType(Method); // this.Method

C# version 2.0 added delegate inference which lets you omit the new DelegateType most of the time. The compiler will automatically convert the method name (and optional this object) into a delegate.

    DelegateType d = o.Method;
    DelegateType d = Method; // this.Method

This shorthand is so old, you may not even remember (or realize) that it is a shorthand for a hidden delegate.

In my colleague's program, the line

    IntPtr hookId = SetHook(HookCallback);

was shorthand for

    LowLevelKeyboardProc temp = HookCallback;
    IntPtr hookId = SetHook(temp);

Once the delegate was made explicit rather than hidden, the issue became clear: Since there was nothing keeping the delegate alive, the delegate disappeared at the next GC, and the unmanaged function pointer disappeared with it.

And now CLR Week will disappear until next time.

Comments (13)
  1. Joshua says:

    Been here.There used to be a comment "Do not optimize. Compiler bugs lurk." Until we understood what was going on. Incidentally, in debug compiles the variable would always live long enough because debug builds keep variables alive until they leave scope.

  2. Some guy says:

    Not knowing any C# myself, I wonder:

    is there any magic in "GC.KeepAlive" or is it basically just a NOP which could be replaced with anything else, as long as it references the local "proc" variable?

  3. Joshua says:

    It's a function call the JIT can't inline.

  4. Raphael says:

    Yep, it's just a NOP in a method marked as Do-Not-Inline: referencesource.microsoft.com

  5. poizan42 says:

    Uhm so unless it's special-cased in the jitter, GC.KeepAlive actually has a small performance overhead (at least a call+return)?

  6. 12BitSlab says:

    Raymond, thanks for the managed code week. Again, I learned from your posts.

  7. voo says:

    @poizan42 It also inhibits useful optimisations since the JIT cannot make any assumptions about global state of the program after the function is executed.

    But all in all that's still pretty negligible on a larger scale.

  8. DWalker says:

    I like how your colleague actually responds to you, and gives you thanks, and mentions that he forgot an important point.  He doesn't blame you for what happened.  That's nice!

  9. Anonymous Cow Herd says:

    "a private static"? Not "a private static *field*"?

    Reminds me of the newbies asking in IRC "Hey, I declared main as a void, but someone tells me it should be an int instead?"

  10. Anonymous Cow Herd says:

    @Raphael: I like how they censored the word "races", referring to race conditions.

  11. Medinoc says:

    Weirdly, the documentation for KeepAlive mentions a problem that's not supposed to happen because according to Dispose's documentation, you're not supposed to dispose managed resources when GC'd, only when explicitly disposed.

  12. Medinoc says:

    ETA: How foolish I am. Of course that situation can easily happen as soon as the stream is an unmanaged one.

    Fortunately, explicit disposal should prevent it (since it references the object).

  13. Joshua says:

    @Anonymous Cow Herd: You mean like this?


    (different function, same consequence)

Comments are closed.

Skip to main content