CallbackOnCollectedDelegate was detected

Water Cooler v2 picture Water Cooler v2 · Jan 31, 2011 · Viewed 16k times · Source

I am subclassing an application. My subclassed Window procedure is within a DLL. My subclassing code inside the DLL looks somewhat like this (stripped down, removed other non-related parts).

class FooBar
{
  private delegate int WndProcDelegateType(IntPtr hWnd, int uMsg, 
                                           int wParam, int lParam);

  private const int GWL_WNDPROC = (-4);
  private static IntPtr oldWndProc = IntPtr.Zero;
  private static WndProcDelegateType newWndProc = new 
                                                  WndProcDelegateType(MyWndProc);

  internal static bool bHooked = false;

  [DllImport("user32.dll")]
  private static extern IntPtr SetWindowLong(IntPtr hWnd, int nIndex, 
                                             WndProcDelegateType dwNewLong);

  [DllImport("user32.dll")]
  private static extern IntPtr SetWindowLong(IntPtr hWnd, int nIndex, 
                                             IntPtr dwNewLong);


  [DllImport("user32")]
  private static extern int CallWindowProc(IntPtr lpPrevWndFunc, IntPtr hWnd, 
                                           int Msg, int wParam, int lParam);

  private static int MyWndProc(IntPtr lhWnd, int Msg, int wParam, int lParam)
  {
    switch (Msg)
    {
      // the usual stuff


      // finally
      return CallWindowProc(oldWndProc, lhWnd, Msg, wParam, lParam);
    }


  internal static void Hook()
  {
    oldWndProc = SetWindowLong(hWnd, GWL_WNDPROC, newWndProc);
    bHooked = oldWndProc != IntPtr.Zero;
  }

  internal static void Unhook()
  {
    if (bHooked) SetWindowLong(hWnd, GWL_WNDPROC, oldWndProc);
  }
}

Now, even though I am holding a strong reference to the WndProc in a class-level static instance variable of the delegate, I get this error.

CallbackOnCollectedDelegate was detected

Message: A callback was made on a garbage collected delegate of type 'PowerPointAddIn1!FooBar+WndProcDelegateType::Invoke'. This may cause application crashes, corruption and data loss. When passing delegates to unmanaged code, they must be kept alive by the managed application until it is guaranteed that they will never be called.

What am I doing wrong?

Answer

Hans Passant picture Hans Passant · Jan 31, 2011
oldWndProc = SetWindowLong(hWnd, GWL_WNDPROC, MyWndProc);

That forces C# to create a delegate object on-the-fly. It translates the code to this:

oldWndProc = SetWindowLong(hWnd, GWL_WNDPROC, new WndProcDelegateType(MyWndProc));

which is a problem, that delegate object isn't referenced anywhere. The next garbage collection is going to destroy it, pulling the rug out from under the unmanaged code. You already did the proper thing in your code, you just forgot to use it. Fix:

oldWndProc = SetWindowLong(hWnd, GWL_WNDPROC, newWndProc);

Deriving your own class from NativeWindow and uses its AssignHandle() method is the better mousetrap btw. Call ReleaseHandle() when you see the WM_DESTROY message.