Why does this window subclassing code crash?

Question

I am trying to subclass the window that currently has focus. I do this by monitoring for HCBT_ACTIVATE events using a CBT hook, and set and unset the WndProc of the focused and previously focused windows.

The problem is that it only works whenever I have a breakpoint set somewhere in the code.

If there is no breakpoint, once my application exits, all the windows that I have subclassed crashes in order, even though I have removed the subclassing and restored the original WndProc.

I have verified that Unsubclass() is called whenever my application shuts down.

// code extracts
HINSTANCE hInst;
HHOOK hHook;

#pragma data_seg(".shared")
HWND hWndSubclass = 0;
FARPROC lpfnOldWndProc = NULL;
#pragma data_seg()
#pragma comment(linker, "/section:.shared,rws")

void Unsubclass()
{
 // if the window still exists
 if (hWndSubclass != 0 && IsWindow(hWndSubclass))
 {
  SetWindowLongPtr(hWndSubclass, GWLP_WNDPROC, (LPARAM)lpfnOldWndProc);
  hWndSubclass = 0;
 }
}

static LRESULT CALLBACK SubClassFunc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
{
 if (message == WM_MOVING)
 {
  // this is just test code so I can see it works (it does)
  RECT* r = (RECT*)lParam;
  r->right = r->left + 500;
  r->bottom = r->top + 500;
  return TRUE;
 }
 else if (message == WM_DESTROY)
 {
  Unsubclass();
 }
 return CallWindowProc((WNDPROC)lpfnOldWndProc, hWndSubclass, message, wParam, lParam);
}

void SubclassWindow(HWND hWnd)
{
 // remove the subclassing for the old window
 Unsubclass();
 // subclass the new window
 lpfnOldWndProc = (FARPROC)SetWindowLongPtr(hWnd, GWLP_WNDPROC, (LPARAM)SubClassFunc);
 hWndSubclass = hWnd;
}

static LRESULT CALLBACK CBTProc(int nCode, WPARAM wParam, LPARAM lParam)
{
 if (nCode == HCBT_ACTIVATE)
 {
  SubclassWindow((HWND)wParam);
 }
 return 0;
}

// ... code that initializes the CBT proc
__declspec(dllexport) BOOL Setup()
{
 hHook = SetWindowsHookEx(WH_CBT, CBTProc, hInst, 0);
}

__declspec(dllexport) BOOL Teardown()
{
 UnhookWindowsHookEx(hHook);
 Unsubclass();
}

BOOL APIENTRY DllMain( HINSTANCE hInstance, 
        DWORD  Reason, 
        LPVOID Reserved
      )
{
 switch(Reason)
 { 
  case DLL_PROCESS_ATTACH:
   hInst = hInstance;
   return TRUE;
  case DLL_PROCESS_DETACH:
   Unsubclass();
   return TRUE;
 }
 return TRUE;
}

Answer 1

lpfnOldWndProc and hWndSubclass are global pointers. Seems like you've got only one per process. What if a process creates more than one window?

Then you will unsubclass only the last one.

EDIT: Also, why do you tear down in Process DETACH?

Answer 2

You are creating a global system-wide hook in a DLL. You need to store the HHOOK handle and your subclassing information in a block of shared memory so all instances of your DLL in all running processes can have access to them. Your variables are declared global in code, but each individual instance of the DLL will have its own local copy of them, and thus they will not be not initialized in all but 1 of your DLL instances (the one that calls Setup()). They need to be shared globally within the entire system instead.

You also should not be calling TearDown() in DLL_PROCESS_DETACH, either. Every instance of the DLL is going to call TearDown() when their respective processes terminate, but only the single instance that actually called Setup() should be the one to call Teardown().

Answer 3

Your problems hinge on several fronts:

• UnHookWindowsHook does not unload injected dlls, all it does is remove the hook proc. If the dlls need to be unloaded its up to them to invent some kind of unloading mechanism.
• SetWindowLongPtr typically fails when called from a process other than the process that owns the window.

The nett result of this is, its very difficult to safely remove windows hooks. First thing, your OldWindowProc pointer should not be stored in the shared data area. Next, in order to remove the subclass, you need to be able to co-erce the (currently) subclassed process to perform the un-subclassing.

What you could do is, first, register a new unique message id and place it in your shared area using RegisterWindowMessage. WM_REMOVE_HOOK.

UINT idWM_REMOVE_HOOK = RegisterWindowMessage("WM_REMOVE_HOOK");

Now, whenever you need to remove a hook,

SendMessage(hWndSubClass,idWM_REMOVE_HOOK,0,0);

In your subclass proc:

if(uMsg == WM_DESTROY || uMsg == idWM_REMOVE_HOOK)
{
  Unsubclass(hwnd);
}

Remove the call to UnSubClass in DLL_PROCESS_DETATCH. Its a dangerous race condition thats going to cause your dll being unloaded in some random process to trash the hook data of a potentially valid hook in another process.

Answer 4

If the debugger will cause the process to succeed by adding a breakpoint then most likely, this is a timing issue. What possibly happens is that your main application is closing itself and freeing resources just before the subclassed windows get the messages they need to remove the subclass again. You might want to give them a few processing cycles to handle their own messages between the unhooking and the unsubclassing. (In Delphi you could do this by calling Application.ProcessMessages but in your C++ version? Don't know the answer to that.