Object gdi hbrush destroy

Question

1.00/5 (2 votes)

See more:

When I resize my window software crashes because hbrush objcet isn't destoyed, I wrote this:

C++

HBRUSH CDlgEst::OnCtlColor(CDC* pDC, CWnd* pWnd, UINT nCtlColor)
{
	HBRUSH hbr = CDialog::OnCtlColor(pDC, pWnd, nCtlColor);
	//HBRUSH hbrN = 
	HBRUSH hBrush = CreateSolidBrush(RGB(240, 240, 240));
	HBRUSH hBrush1 = CreateSolidBrush(RGB(240, 240, 240));
	//pDC->SetBkColor(0x000000FF);
	int n_color;
	RGBTRIPLE rgb;
	double red;
	double green;
	double blue;
	COLORREF Color;
	CBrush Brush;
	CSize csSizeOfText;
	CString cs;
	switch (pWnd->GetDlgCtrlID())
   {
	case IDC_STATIC_TIME1:
	case IDC_STATIC_TIME2:
	case IDC_STATIC_TIME3:
	case IDC_STATIC_TIME4:
	case IDC_ST_ESTS:
	case IDC_TIMESEC1:
	case IDC_TIMESEC2:
	case IDC_TIMESEC3:
	case IDC_TIMESEC4:
		pDC->SetBkColor(RGB(255, 255, 255));// for text background  
		pDC->SetTextColor(RGB(0, 128, 0));
		return (HBRUSH)(m_WhiteBrush);//your brush 
	case IDC_ST_COLOR1:
		if (m_aColor2.GetSize() > 0)
		{
			n_color = m_aColor2[0];
			rgb.rgbtRed = GetRValue(n_color);
			rgb.rgbtGreen = GetGValue(n_color);
			rgb.rgbtBlue = GetBValue(n_color);

			red = rgb.rgbtRed;
			green = rgb.rgbtGreen;
			blue = rgb.rgbtBlue;
			pDC->SetBkColor(RGB(red, green, blue));

			Color = RGB(red, green, blue);
			Brush.CreateSolidBrush(Color);
			hBrush1 = CreateSolidBrush(RGB(red, green, blue));
			return (HBRUSH)(hBrush1);
		}
		else
			return hBrush1;

	case IDC_ST_COLOR2:
		if (m_aColor2.GetSize() > 1)
		{
			n_color = m_aColor2[1];
			rgb.rgbtRed = GetRValue(n_color);
			rgb.rgbtGreen = GetGValue(n_color);
			rgb.rgbtBlue = GetBValue(n_color);

			red = rgb.rgbtRed;
			green = rgb.rgbtGreen;
			blue = rgb.rgbtBlue;
			pDC->SetBkColor(RGB(red, green, blue));
			hBrush1 = CreateSolidBrush(RGB(red, green, blue));

			Color = RGB(red, green, blue);
			Brush.CreateSolidBrush(Color);
			return (HBRUSH)(hBrush1);
		}
		else
			return hBrush1;
	case IDC_ST_COLOR3:
		if (m_aColor2.GetSize() > 2)
		{
			n_color = m_aColor2[2];
			rgb.rgbtRed = GetRValue(n_color);
			rgb.rgbtGreen = GetGValue(n_color);
			rgb.rgbtBlue = GetBValue(n_color);

			red = rgb.rgbtRed;
			green = rgb.rgbtGreen;
			blue = rgb.rgbtBlue;
			pDC->SetBkColor(RGB(red, green, blue));
			hBrush1 = CreateSolidBrush(RGB(red, green, blue));

			Color = RGB(red, green, blue);
			Brush.CreateSolidBrush(Color);
			return (HBRUSH)(hBrush1);
		}
		else
			return hBrush1;
	case IDC_ST_COLOR4:
		if (m_aColor2.GetSize() > 3)
		{
			n_color = m_aColor2[3];
			rgb.rgbtRed = GetRValue(n_color);
			rgb.rgbtGreen = GetGValue(n_color);
			rgb.rgbtBlue = GetBValue(n_color);

			red = rgb.rgbtRed;
			green = rgb.rgbtGreen;
			blue = rgb.rgbtBlue;
			pDC->SetBkColor(RGB(red, green, blue));
			hBrush1 = CreateSolidBrush(RGB(red, green, blue));

			Color = RGB(red, green, blue);
			Brush.CreateSolidBrush(Color);
			return (HBRUSH)(hBrush1);
		}
		else
			return hBrush1;
	default:
		return CDialog::OnCtlColor(pDC, pWnd, nCtlColor);
	}	

	DeleteObject((HBRUSH)hbr);
	DeleteObject((HBRUSH)hBrush);
	DeleteObject((HBRUSH)hBrush1);

	return hbr;
}

but software crashes...

What I have tried:

I wrote code in this way but software crashes when I resize

Posted 9-Apr-24 3:04am

Member 14594285

Updated 9-Apr-24 19:06pm

KarstenK

v2

Add a Solution

Comments

Richard Deeming 9-Apr-24 9:15am

It's highly unlikely that not destroying a resource would cause the code to crash. You'd end up with with a resource leak, which might eventually cause problems; but not an immediate crash.

But since you've already decided that this must be the cause of the crash, you've neglected to include any actual details of the crash itself. Which means it's also unlikely that anyone will be able to help you.

Member 14594285 9-Apr-24 9:18am

when I want to resize my window software crashes

Richard Deeming 9-Apr-24 9:20am

And you have done precisely nothing to diagnose why it crashes.

Or, if you have tried to diagnose it, you haven't shared that information with us.

All you've done is say it crashes "because the brush isn't destroyed". Which is both unlikely, and uninformative.

Member 14594285 9-Apr-24 9:31am

software crashes in this point :

Gdiplus::GdiplusStartupInput gdiplusStartupInput;
Gdiplus::GdiplusStartup(&m_gdiplusToken, &gdiplusStartupInput, NULL);

INT_PTR nResponse = dlg.DoModal();

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Pete O'Hanlon · Answer 1 · 2024-04-09T04:12:00

Solution 1

Something to look at in your code. You are returning early from your switch statements, so the brushes aren't removed. You are leaking resources in that code. You are leaking a lot of resources. What's even more interesting is - if you somehow manage to get to the DeleteObject section at the bottom, you return hbr after it's been deleted.

Posted 9-Apr-24 4:12am

Pete O'Hanlon

Comments

Member 14594285 9-Apr-24 10:18am

I know, but I don't know gdi and I don't know how to correct this code

Pete O'Hanlon 9-Apr-24 11:00am

Okay, don't repeatedly create brushes. Store them as members instead and don't forget to destroy the object in the PostNcDestroy method.

Member 14594285 9-Apr-24 11:15am

and Must I write code in this way?:

DeleteObject((HBRUSH)hbr);
DeleteObject((HBRUSH)hBrush);
DeleteObject((HBRUSH)hBrush1);

Pete O'Hanlon 9-Apr-24 11:26am

A simple thing to do is write a class to manage the brush like this:
#include <windows.h>

class BrushHandle {
public:
BrushHandle(HBRUSH brush = nullptr) : brush_(brush) {}

~BrushHandle() {
if (brush_) {
DeleteObject(brush_);
}
}

operator HBRUSH() const {
return brush_;
}

private:
HBRUSH brush_;
};

Then, you can use the brush like this to create the brush:

BrushHandle brush(CreateSolidBrush(RGB(240, 240, 240)));

Member 14594285 9-Apr-24 11:55am

I prefere to use CBrush..but I don't know how I can do conversion:

CBrush m_hbr;

(HBRUSH)(m_hbr) = CDialog::OnCtlColor(pDC, pWnd, nCtlColor);

Rick York 9-Apr-24 13:29pm

Here is documentation on CBrush : https://learn.microsoft.com/en-us/cpp/mfc/reference/cbrush-class?view=msvc-170

Have you tried the FromHandle method?

Member 14594285 9-Apr-24 11:35am

Or Can I use CBrush for CTlColor?

CPallini · Answer 2 · 2024-04-10T08:06:00

Try to closely mimic the behaviour this MSDN piece of code[^].
Namely, DO NOT create (and destroy) brushes (or other GDI object) inside the OnCtlColor handler. On the contrary create the brushes once, in your initialization code, and release them once, in your cleanup code. The message handler should just change the CDC parameters and return the handle of the proper (existing) brush.