JAM-Software / RibbonFramework

Delphi wrapper and standalone Designer for the Windows Ribbon Framework
Other
204 stars 59 forks source link

GDI object leak #108

Closed luca-vari closed 5 years ago

luca-vari commented 5 years ago

Hi, The current version of the ribbon framework leaks Windows GDI objects. You can monitor GDI object usage in the Windows Task Manager (e.g. see here).

I can reproduce the issue on Windows 10 using the Ribbon Designer WordPad template:

  1. Launch the Ribbon Designer.
  2. Select File>New menu option. The New Ribbon Document dialog will display.
  3. Select the Create a new Ribbon document using the WordPand template radio button.
  4. Enter a directory in the Save the document in the following directory edit box.
  5. Select the OK button.
  6. Select the Project>Preview menu option. The Ribbon Preview will display.
  7. Note the number of GDI objects reported in the Task Manager for the RibbonDesigner.exe application.
  8. Close the Ribbon Preview.
  9. Repeat as many times as you like from step 6.

You will note that the number of GDI objects used consistently increases but it should remain roughly the same on each execution of the Ribbon Preview. When the GDI object usage hits the application limit (10,000) the Ribbon Designer will crash.

The reason that that this is happening is that the IUIFramework::Destroy method is never called from the TUIRibbon class. It looks like it was called in previous versions of the framework but removed in commits df9da3a and 75dfaab.

For df9da3a it possible these lines were inadvertently removed:

if Assigned(FFramework) then
    FFramework.Destroy;

For 75dfaab may be the change introduced in df9da3a was copied...?

Note that the TUIRibbon.DestroyWnd procedure is not called as a consequence of the TUIRibbon.Destroy destructor being called so that a call to IUIFramework::Destroy would be required in both code paths. This was surprising to me; it is a consequence of the TWinControl.WMNCDestroy procedure (the handler for the WM_NCDESTROY windows message) clearing the windows handle of the ribbon before TUIRibbon.Destroy is called. The TUIRibbon.DestroyWnd procedure doesn't get called if the control doesn't have a handle...

Cheers, Luca

SaschaSchaefer commented 5 years ago

Added call to IUIFramework::Destroy again.

It was removed because it caused occasional AVs. I did not notice the leaks since I never used the Ribbon in an application that creates a ribbon multiple times.

The AVs could be solved by calling IUIFramework::Destroy before freeing the commands. I'm still not sure why this particular order is necessary though.

luca-vari commented 5 years ago

Thank you!