haskell / win32

Haskell support for the Win32 API
http://hackage.haskell.org/package/Win32
Other
97 stars 62 forks source link

setWindowClosure/createWindowEx leaks memory #142

Closed tmcl closed 4 years ago

tmcl commented 4 years ago

setWindowClosure leaks memory. It creates a function pointer using "wrapper", but then it seems to forget about it. Every function pointer created with "wrapper" must be freed, since the runtime has to keep a reference to everything which is closed over in order for the code to run whenever outsiders call.

I believe a viable solution is to change defWindowProc to something like:

defWindowProc :: Maybe HWND -> WindowMessage -> WPARAM -> LPARAM -> IO LRESULT
defWindowProc mb_wnd msg w l = do
  when (msg == wM_NCDESTROY) (maybe (return ()) freeWindowProc mb_wnd)
  c_DefWindowProc (maybePtr mb_wnd) msg w l 

freeWindowProc :: HWND -> IO ()
freeWindowProc hwnd = do
   fp <- c_GetWindowLongPtr hwnd GWLP_USERDATA
   unless (fp == nullPtr) $ 
      freeHaskellFunPtr $ castPtrToFunPtr . intPtrToPtr . fromIntegral $ fp

We aren't strictly guaranteed the user data will still be what we want but I suppose genericWndProc will have broken by then already. According to Raymond Chen https://devblogs.microsoft.com/oldnewthing/20050726-00/?p=34803 WM_NCDESTROY is the last message a window receives before it dies.

I have attached a program which demonstrates the difference Main.hs. You can track the memory in the Task Manager. The program creates a series of windows. Each window closure holds a reference to big pack of data which it prints out when it gets WM_DESTROY. A weak reference is also created and returned. Once the last window has been output, the weak references are inspected. If you're bored or discover you are using too much memory and want to abort early - Ctrl-C doesn't effectively quit the process. Instead, minimise the window. The WM_SIZE handler will immediately terminate the process if the window is minimised.

Using Graphics.Win32.defWindowProc in the otherwise case of mainWndProc, each weak reference contains the strings and the memory increases; for me it usually ends up in the range 60-120 MB. With my proposed replacement, Main.defWindowProc, in the otherwise case of mainWndProc, only the final weak reference contains a string at the end - the rest print out "all's well" to indicate deRefWeak returned Nothing - and the memory stays low, alternating between about 10-13 MB each iteration after the first.

It's way too late but I can probably throw together a pr tomorrow.

tmcl commented 4 years ago

I have made my changes but I can't build it. cabal sandbox init doesn't exist any more and cabal new-build fails because hsc2hs has a dependency on a bunch of things which have a dependency on the installed version of Win32. Any hint? (I'm using chocolatey. I recently upgraded from Haskell Platform although there now seem to be actual instructions for how to do it and not go crazy.)

Mistuke commented 4 years ago

I believe this fixed by #143, thanks @tmcl !