EasyTabs icon indicating copy to clipboard operation
EasyTabs copied to clipboard

Potential GDI+ resource leaks in `WindowsSizingBoxes` class

Open astrohart opened this issue 9 months ago • 0 comments

@lstratman Great library! I love it. I just have a niggle, though.

You really should not, as a best practice, IMHO, use fields such as _closeButtonHighlight in the WindowsSizingBox class to hold a System.Drawing.Brush instance for the life of the object. Such as in the code snippet that is shown in Listing 1.

public class WindowsSizingBoxes
{
    /* ... */

    protected Brush _closeButtonHighlight =
        new SolidBrush(Color.FromArgb(232, 17, 35));
}

Listing 1. Storing a System.Drawing.Brush in a field.

This is because Brush is a GDI+ resource, of which there are only finitely many available to the operating system.

The Brush might be automatically garbage-collected at some point (maybe; it's not deterministic), but, in the meantime, the code is hogging precious GDI+ resources, making them not available to other operating system objects. Plus, this code is probably called very often. I can see the potential desire to save CPU cycles, but I think saving RAM is better.

A best practice in Win32 GDI programming is to use resources such as Brushs, Regions, and Pens only as needed and to immediately release them when drawn. Otherwise you risk leaking lots of memory.

I would recommend the WindowsSizingBoxes class be updated to only store the Colors of the various screen elements (and maybe even make them public properties so that they can be customized, e.g. with themes or skins), and then, in the drawing routines, do:

using (var brush = new SolidBrush(_closeButtonHighlightColor))
{
    /* ... use the brush here ... */
}

Listing 2. Writing a using statement for drawing with a Brush.

astrohart avatar May 15 '24 15:05 astrohart