Sign in to follow this  
Lenox

Deallocation error upon std::map element removal

Recommended Posts

I'm using an std::map to map Windows Messages to function pointers in my Control class. When my Control is deleted, the Control's dtor is called and the elements from my std::map are removed when it goes out of scope. The problem comes when the final element in my std::map is removed; it ends up failing the following assertion in dbgdel.cpp: ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)); So, my question to you guys is: What might be causing this? I'm not doing anything way too strange with my std::map it seems, but I s'pose looks can be deceiving. The relevant code is as follows: Control.h:
#ifndef CONTROL_H
#define CONTROL_H

#include <windows.h>
#include <string>
#include <map>
#include <utility>

#include "../GUIDll.h"

		// warning C4312: 'reinterpret_cast' : conversion from 'LONG' to 'Window *' of greater size
#pragma warning( disable: 4312 )
		// warning C4311: 'reinterpret_cast' : pointer truncation from 'Window *' to 'LONG'
#pragma warning( disable: 4311 )
		// warning C4244: 'argument' : conversion from 'LPARAM' to 'long', possible loss of data
#pragma warning( disable: 4244 )
		// warning C4100: 'lParam' : unreferenced formal parameter
#pragma warning( disable: 4100 )
		// warning C4127: conditional expression is constant
#pragma warning( disable: 4127 )

class Control;
typedef long (* MsgHandlerPtr)(Control*, HWND, WPARAM, LPARAM);
typedef std::map<long, MsgHandlerPtr> MsgMap;
typedef MsgMap::const_iterator MsgIterator;

class Control
{
protected:
	GUILIB_API MsgHandlerPtr RegisterMsgHandler(long msg, MsgHandlerPtr handler);

	HWND					_hwnd;
	MsgMap					_handlers;
	HINSTANCE				_hinst;
	unsigned int			_style;
	wchar_t		*			_name;
	wchar_t		*			_class;
	Control		*			_ctl_parent;
	WNDPROC					_oldwndproc;
	HBRUSH					_brush;
	bool					_registered;
	HMENU					_menu;
	bool					_vscroll;
	bool					_hscroll;
	HICON					_icon;
	anchor_point			_top_left;
	anchor_point			_bottom_right;
	double					_x;
	double					_y;
	double					_width;
	double					_height;

	static LRESULT CALLBACK MsgRouter(HWND, UINT, WPARAM, LPARAM);
	void SetHWND(HWND hWnd) {_hwnd = hWnd; };

public:
	inline bool operator ==(Control &rhs)
	{
		return (_hwnd == rhs._hwnd);
	}

	GUILIB_API Control(HINSTANCE hInst, Control * ctlParent = NULL);
	GUILIB_API virtual ~Control();

	GUILIB_API void Create();
	GUILIB_API void Focus();

	MsgIterator GetMsgHandler(long msg);
	bool IsValidHandler(MsgIterator iter) { return (iter != _handlers.end()); };
	GUILIB_API bool IsControl(HWND hWnd) const { return (hWnd == _hwnd); };
	
	GUILIB_API WNDPROC GetOldWndProc() const { return _oldwndproc; };

	GUILIB_API void SetStyle(int style);
	GUILIB_API void SetClass(char * name);
	GUILIB_API void SetName(char * name);
	GUILIB_API void SetClass(wchar_t * name);
	GUILIB_API void SetName(wchar_t * name);
	GUILIB_API void SetX(double x = CW_USEDEFAULT);
	GUILIB_API void SetY(double y = CW_USEDEFAULT);
	GUILIB_API void SetHeight(double height = CW_USEDEFAULT);
	GUILIB_API void SetWidth(double width = CW_USEDEFAULT);
	GUILIB_API void SetMenu(unsigned int hMenu = 0);
	GUILIB_API void SetMenu(HMENU hMenu = NULL);
	GUILIB_API void SetBrush(HBRUSH hBrush = NULL);
	GUILIB_API void SetRegistered(bool registered = false);
	GUILIB_API void SetVScroll(bool VScroll = false);
	GUILIB_API void SetHScroll(bool HScroll = false);
	GUILIB_API void SetIcon(HICON hIcon);
	GUILIB_API bool HasHandlers() const { return (_handlers.empty() != true); };
	GUILIB_API void Destroy();

	GUILIB_API unsigned int GetStyle() const { return _style; };
	GUILIB_API HWND GetHWND() const { return _hwnd; };
	GUILIB_API double GetWidth() const { return _width; }
	GUILIB_API double GetHeight() const { return _height; }
	GUILIB_API anchor_point GetTopLeftAnchor() const { return _top_left; };
	GUILIB_API anchor_point GetBottomRightAnchor() const { return _bottom_right; };

	GUILIB_API void SetAnchors();
};

#endif		// CONTROL_H

Control.cpp:
#include "Control.h"
#include "Unicode.h"

Control::Control(HINSTANCE hInst, Control * ctlParent)
: _hwnd(NULL), _hinst(hInst), _style(0), _name(NULL), _class(NULL), _ctl_parent(ctlParent), _oldwndproc(NULL), _brush(NULL),
		_registered(false), _menu(NULL), _vscroll(false), _hscroll(false), _icon(NULL), _x(0), _y(0),
		_width(0), _height(0)
{
};

Control::~Control() {
	if(_name != NULL) {
		delete [] _name;
		_name = NULL;
	}

	if(_class != NULL) {
		delete [] _class;
		_class = NULL;
	}

	if(_ctl_parent != NULL) _ctl_parent = NULL;
}

void Control::Create()
{
	if(!_registered)
	{
		WNDCLASSEX wndClass;
		memset(&wndClass, '\0', sizeof(WNDCLASSEX));
		wndClass.cbSize				=		sizeof(wndClass);
		wndClass.style				=		CS_HREDRAW | CS_VREDRAW;
		wndClass.lpfnWndProc		=		&Control::MsgRouter;
		wndClass.hInstance			=		_hinst;
		wndClass.hCursor			=		LoadCursor(NULL, IDC_ARROW);
		if(!_brush)
		{
			wndClass.hbrBackground	=		GetSysColorBrush(COLOR_APPWORKSPACE);
			_brush					=		wndClass.hbrBackground;
		}
		else
		{
			wndClass.hbrBackground	=		_brush;
		}
		wndClass.lpszMenuName		=		NULL;
		if(_class != NULL) {
#ifndef _UNICODE
			char * ansi = NULL;
			UnicodeToAnsi(_class, &ansi);
			wndClass.lpszClassName	= new char[strlen(ansi) + 1];
			memset(const_cast<LPSTR>(wndClass.lpszClassName), '\0', strlen(ansi) + 1);
			memcpy(const_cast<LPSTR>(wndClass.lpszClassName), ansi, strlen(ansi));
			CoTaskMemFree(ansi);
#else
			wndClass.lpszClassName = new wchar_t[wcslen(_class) + 1];
			wmemset(const_cast<LPWSTR>(wndClass.lpszClassName), '\0', wcslen(_class) + 1);
			wmemcpy(const_cast<LPWSTR>(wndClass.lpszClassName), _class, wcslen(_class));
#endif
		} else {
			wndClass.lpszClassName	=		NULL;
		}
		wndClass.cbClsExtra			=		0;
		wndClass.cbWndExtra			=		0;
		if(_icon != NULL)
		{
			wndClass.hIconSm		=		_icon;
		}
		else
		{
			wndClass.hIconSm		=		reinterpret_cast<HICON>(NULL);
		}
		wndClass.hIcon				=		reinterpret_cast<HICON>(NULL);

		if(!RegisterClassEx(&wndClass)) throw std::runtime_error("Failed to register the class.");
	}

	if(_vscroll) _style |= WS_VSCROLL;
	if(_hscroll) _style |= WS_HSCROLL;

	HWND hWnd = ::CreateWindowW(_class, 
						_name,
						_style,
						_x,
						_y,
						_width,
						_height,
						(_ctl_parent != NULL ? _ctl_parent->_hwnd : NULL),
						_menu,
						_hinst,
						reinterpret_cast<LPVOID>(this));
	if(!hWnd) throw std::runtime_error("Failed to create the window.");
	if(_registered)
	{
		_oldwndproc = reinterpret_cast<WNDPROC>(::GetWindowLong(hWnd, GWL_WNDPROC));
		::SetWindowLong(hWnd, GWL_WNDPROC, reinterpret_cast<LONG>(&Control::MsgRouter));
		::SetWindowLong(hWnd, GWL_USERDATA, reinterpret_cast<LONG>(this));
		SetHWND(hWnd);
	}
}

LRESULT CALLBACK Control::MsgRouter(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
{
	Control * control = NULL;

	if(message == WM_NCCREATE)
	{
		control = reinterpret_cast<Control*>(((LPCREATESTRUCT)lParam)->lpCreateParams);
		if(control == NULL) throw std::runtime_error("The instance was not passed to the message router.");
		::SetWindowLong(hWnd, GWL_USERDATA, reinterpret_cast<LONG>(control));
		control->SetHWND(hWnd);
	}
	else
	{
		control = reinterpret_cast<Control*>(::GetWindowLong(hWnd, GWL_USERDATA));
		if(control == NULL)
		{
			return ::DefWindowProc(hWnd, message, wParam, lParam);
		}
	}

	if(control->HasHandlers())
	{
		MsgIterator iter = control->GetMsgHandler(message);
		if(control->IsValidHandler(iter) == true) return (iter->second)(control, hWnd, wParam, lParam);
	}

	if(control->_oldwndproc != NULL)
	{
		return (control->_oldwndproc)(hWnd, message, wParam, lParam);
	}
	else
	{
		return ::DefWindowProc(hWnd, message, wParam, lParam);
	}
}

MsgIterator Control::GetMsgHandler(long msg)
{
	return _handlers.find(msg); 
}

MsgHandlerPtr Control::RegisterMsgHandler(long msg, MsgHandlerPtr handler)
{
	MsgHandlerPtr prev = NULL;
	MsgIterator iter = _handlers.find(msg);
	if(IsValidHandler(iter)) prev = iter->second;
	_handlers.insert(std::make_pair<long, MsgHandlerPtr>(msg, handler));
	return prev;
}

void Control::SetRegistered(bool registered)
{
	_registered = registered;
}

void Control::SetStyle(int style)
{
	_style = style;
	if(_hwnd) ::SetWindowLong(_hwnd, GWL_STYLE, _style);
}

void Control::SetClass(wchar_t * name)
{
	if(!name) return;
	if(_class != NULL) {
		delete [] _class;
		_class = NULL;
	}
	_class = new wchar_t[wcslen(name) + 1];
	wmemset(_class, '\0', wcslen(name) + 1);
	wmemcpy(_class, name, wcslen(name));
	if(_hwnd) throw std::runtime_error("Can't change the class of a control after it has been created!");
}

void Control::SetName(wchar_t * name)
{
	if(!name) return;
	if(_name != NULL) {
		delete [] _name;
		_name = NULL;
	}
	_name = new wchar_t[wcslen(name) + 1];
	wmemset(_name, '\0', wcslen(name) + 1);
	wmemcpy(_name, name, wcslen(name));
	if(_hwnd) ::SendMessageW(_hwnd, WM_SETTEXT, NULL, reinterpret_cast<LPARAM>(_name));
}

void Control::SetClass(char * name)
{
	if(!name) return;
	if(_class != NULL) {
		delete [] _class;
		_class = NULL;
	}
	wchar_t * uni = NULL;
	AnsiToUnicode(name, &uni);
	_class = new wchar_t[wcslen(uni) + 1];
	wmemset(_class, '\0', wcslen(uni) + 1);
	wmemcpy(_class, uni, wcslen(uni));
	CoTaskMemFree(uni);
	if(_hwnd) throw std::runtime_error("Can't change the class of a control after it has been created!");
}

void Control::SetName(char * name)
{
	if(!name) return;
	if(_name != NULL) {
		delete [] _name;
		_name = NULL;
	}
	wchar_t * uni = NULL;
	AnsiToUnicode(name, &uni);
	_name = new wchar_t[wcslen(uni) + 1];
	wmemset(_name, '\0', wcslen(uni) + 1);
	wmemcpy(_name, uni, wcslen(uni));
	CoTaskMemFree(uni);
	if(_hwnd) ::SendMessageW(_hwnd, WM_SETTEXT, NULL, reinterpret_cast<LPARAM>(_name));
}

void Control::SetX(double x)
{
	_x = x;
	if(_hwnd) ::MoveWindow(_hwnd, _x, _y, _width, _height, TRUE);
}

void Control::SetY(double y)
{
	_y = y;
	if(_hwnd) ::MoveWindow(_hwnd, _x, _y, _width, _height, TRUE);
}

void Control::SetHeight(double height)
{
	_height = height;
	if(_hwnd) ::MoveWindow(_hwnd, _x, _y, _width, _height, TRUE);
}

void Control::SetWidth(double width)
{
	_width = width;
	if(_hwnd) ::MoveWindow(_hwnd, _x, _y, _width, _height, TRUE);
}

void Control::Focus()
{
	if(_hwnd && !_ctl_parent)
	{
		::SetForegroundWindow(_hwnd);
	}
	else if(_hwnd && _ctl_parent)
	{
		::SetForegroundWindow(_ctl_parent->_hwnd);
		::SetFocus(_hwnd);
	}
}

void Control::SetMenu(unsigned int hMenu)
{
	if(hMenu != NULL && _hinst != NULL) {
		_menu = ::LoadMenu(_hinst, MAKEINTRESOURCE(hMenu));
		if(_hwnd != NULL) {
			::SetMenu(_hwnd, _menu);
		}
	}
}

void Control::SetMenu(HMENU hMenu)
{
	if(hMenu != NULL && _hwnd != NULL) {
		::SetMenu(_hwnd, _menu);
	}
}

void Control::SetBrush(HBRUSH hBrush)
{
	_brush = hBrush;
	if(_hwnd) throw std::runtime_error("Cannot modify the brush at runtime.");
}

void Control::SetVScroll(bool VScroll)
{
	_vscroll = VScroll;
	if(_hwnd) throw std::runtime_error("Cannot set VScroll at runtime.");
}

void Control::SetHScroll(bool HScroll)
{
	_hscroll = HScroll;
	if(_hwnd) throw std::runtime_error("Cannot set HScroll at runtime.");
}

void Control::SetIcon(HICON hIcon)
{
	_icon = hIcon;
	if(_hwnd) ::SendMessage(_hwnd, WM_SETICON, ICON_SMALL, reinterpret_cast<LPARAM>(hIcon));
}

void Control::Destroy()
{
	if(_hwnd) ::DestroyWindow(_hwnd);
}

void Control::SetAnchors() {
	if(!_ctl_parent) return;
	HWND hWnd = _ctl_parent->GetHWND();
	RECT rect;
	GetClientRect(hWnd, &rect);
	double width = rect.right;
	double height = rect.bottom;
	if(_x != 0 && _y != 0) {
		_top_left.SetAnchor((_x / width), (_y / height));
	} else if(_x == 0 && _y != 0) {
		_top_left.SetAnchor(0, (_y / height));
	} else if(_x != 0 && _y == 0) {
		_top_left.SetAnchor((_x / width), 0);
	} else if(_x == 0 && _y == 0) {
		_top_left.SetAnchor(0, 0);
	}

	if((_x + _width) != 0 && (_y + _height) != 0) {
		_bottom_right.SetAnchor(((_x + _width) / width), ((_y + _height) / height));
	} else if((_x + _width) == 0 && (_y + _height) != 0) {
		_bottom_right.SetAnchor(0, ((_y + _height) / height));
	} else if((_x + _width) != 0 && (_y + _height) == 0) {
		_bottom_right.SetAnchor(((_x + _width) / width), 0);
	} else if((_x + _width) == 0 && (_y + _height) == 0) {
		_bottom_right.SetAnchor(0, 0);
	}
}


Any help would be greatly appreciated. Thanks. -- Lenox

Share this post


Link to post
Share on other sites
Look up the call stack when that assertion occurs - what line in your code is triggering it?

EDIT: Also, you don't seem to be passing the WM_NCCREATE message along to DefWindowProc() (Although it's probably unrelated).

EDIT #2: How do you destroy the control? It's possible that the control is being destroyed before the window, which causes weird pointer errors when the wndproc is called (Since it gets the stored pointer and assumes it's valid).

Share this post


Link to post
Share on other sites
Quote:
Original post by Lenox
So, my question to you guys is: What might be causing this?


Barely skimmed your code, but memory corruption may be a problem. Have you considered using std::(w)string to represent text? You then wouldn't need a destructor at all (unless there's something else you aren't doing in the dtor that you should).

Share this post


Link to post
Share on other sites
First off, just a bunch of comments on the posted code:

Don't check for NULL pointers before deleting them. That check is already the first thing done inside delete itself, so you're just duplicating it. As mentioned, you're better off using std::string anyway.
Also, in a destructor there is no point setting the pointers to NULL as the object is being deleted and those pointers are about the cease existing anyway.

Those pragma warning disables look a little worrying. It looks like you must have 64-bit compatibility turned on, and then use the pragmas to ignore warnings related to that. Just go into the project settings and turn off the 64-bit compatibility warnings.

To get rid of warning 4244, simply comment out the name of unused variables (but leave their type info there)

operator == should take a "const Control&" as the right-hand side.
Shouldn't IsValidHandler be const, and how about taking a const iterator reference instead of passing by value.

This is a rather odd way of doing a strcpy:
			memset(const_cast<LPSTR>(wndClass.lpszClassName), '\0', strlen(ansi) + 1);
memcpy(const_cast<LPSTR>(wndClass.lpszClassName), ansi, strlen(ansi));
You've used the same thing a few more times later on. Just use strcpy and wcscpy.


Now, once you've dealt with that, would you mind posting the call stack at the point of the the assertion please?

Share this post


Link to post
Share on other sites
Quote:
Original post by iMalc
First off, just a bunch of comments on the posted code:

Don't check for NULL pointers before deleting them. That check is already the first thing done inside delete itself, so you're just duplicating it. As mentioned, you're better off using std::string anyway.
Also, in a destructor there is no point setting the pointers to NULL as the object is being deleted and those pointers are about the cease existing anyway.

Those pragma warning disables look a little worrying. It looks like you must have 64-bit compatibility turned on, and then use the pragmas to ignore warnings related to that. Just go into the project settings and turn off the 64-bit compatibility warnings.

To get rid of warning 4244, simply comment out the name of unused variables (but leave their type info there)

operator == should take a "const Control&" as the right-hand side.
Shouldn't IsValidHandler be const, and how about taking a const iterator reference instead of passing by value.

This is a rather odd way of doing a strcpy:
			memset(const_cast<LPSTR>(wndClass.lpszClassName), '\0', strlen(ansi) + 1);
memcpy(const_cast<LPSTR>(wndClass.lpszClassName), ansi, strlen(ansi));
You've used the same thing a few more times later on. Just use strcpy and wcscpy.


Now, once you've dealt with that, would you mind posting the call stack at the point of the the assertion please?


Thanks for the input.

On the checking for NULL and setting to NULL in my dtors: I did this for safe measure- I don't like to assume that something's going to be valid when I delete it. It just makes me feel better inside.

About the operator== and IsValidHandler- Yeah, you're right. My bad.

Yeah, I prefer using memset/memcpy. Not quite sure why. I think I'll look into the ramifications of using strcpy over memcpy.

My call stack at the time of the assertion looks like this:


> GUI.dll!operator delete(void * pUserData=0x00ec1068) Line 52 + 0x51 bytes C++
GUI.dll!RichEditCtl::`vector deleting destructor'() + 0x7d bytes C++
GSEMaster.exe!MainWindow::~MainWindow() Line 19 + 0x6f bytes C++
GSEMaster.exe!MainWindow::`scalar deleting destructor'() + 0x2b bytes C++


~MainWindow() looks like so:


MainWindow::~MainWindow() {
if(OutputREC != NULL) {
delete OutputREC; /* This line is the one in question */
OutputREC = NULL;
}

if(InputREC != NULL) {
delete InputREC;
InputREC = NULL;
}

if(client != NULL) {
delete client;
client = NULL;
}
}





And ~Control() now looks like this, after going through a few of the other suggestions:


Control::~Control() {
if(_ctl_parent) _ctl_parent->Destroy();

if(_name) {
delete [] _name;
_name = NULL;
}

if(_class) {
if(!_registered) UnregisterClass(_class, _hinst);
delete [] _class;
_class = NULL;
}

this->Destroy();
}





Quote:
Original post by Evil Steve
Look up the call stack when that assertion occurs - what line in your code is triggering it?

EDIT: Also, you don't seem to be passing the WM_NCCREATE message along to DefWindowProc() (Although it's probably unrelated).

EDIT #2: How do you destroy the control? It's possible that the control is being destroyed before the window, which causes weird pointer errors when the wndproc is called (Since it gets the stored pointer and assumes it's valid).


Ah, WM_NCCREATE is being passed along to DefWindowProc (or the control's old wndproc, if it had one) unless an exception occurs while I'm trying to handle it. And, after your second edit, I realized I wasn't destroying my controls (Whoops.) and added a few calls to Control::Destroy() in there, which just calls ::DestroyWindow.

Quote:
Original post by Zahlman
Barely skimmed your code, but memory corruption may be a problem. Have you considered using std::(w)string to represent text? You then wouldn't need a destructor at all (unless there's something else you aren't doing in the dtor that you should).


Using wchar_t* is just a preference, albeit a foolish one. I'll see what I can do about switching over, though.

[EDIT]

After I started actually deleting the window properly, the assertion magically disappeared. Thanks, all.

[Edited by - Lenox on January 5, 2008 1:08:20 AM]

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this