# DestroyWindow Access Violation

This topic is 4745 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Ok, i dont like this. in my Window::close() function, it does everything ok exept DestroyWindow() i get access violation when i call DestroyWindow, and dont get when i do:
#ifdef hwnd
DestroyWindow(hwnd);
#endif

or
HWND t;
t=this->hwnd;
#ifdef t
DestroyWindow(hwnd);
#endif
but the problem is, it doesn't call DestroyWindow so it looks like nothing happened.. but, i get access violation when i do:
if (hwnd)
DestroyWindow(hwnd);

btw, hwnd = this->hwnd. i already created my window, and i dont get access violation when i call:
	char* h="";
GetClassName(hwnd,h,256);

what should i do? i really dont get how hwnd isn't defined but it is not a zero and i already used it to get the class name..so why i cant use it to destroy itself (the window)? i have tried to call GetLastError() after DestroyWindow() but it wasn't called. thanks, pex.

##### Share on other sites
The problem with pre-processor commads is, that they are executed at compile time and thus cannot do anything at runtime.
You can simply use
if (IsWindow(hwnd)) {    DestroyWindow(hwnd);}

##### Share on other sites
Are you sure you are not calling DestroyWindow twice with the same handle? I would set hwnd to NULL after calling DestroyWindow, then worst case and it is getting called twice it would still be safe.

Also, your #ifdef solutions are simple wrong. #ifdef is for testing preprocessor tokens, not variables for being vaild/null. #ifdef hwnd is always evaluating to false, so the DestroyWindow(hwnd) is not even getting compiled into your program.

The best way to figure this out is to set a breakpoint on the DestroyWindow statement. Check that hwnd is still valid, and also check you dont get to that breakpoint twice.

Alan

##### Share on other sites
ok, i have tried that now (both Alan and darookie tips, thanks).
but i get Access Violation again in same address (cmp dword ptr [eax+58h],0)
i dont have to make hwnd a pointer, right? or i do have?

##### Share on other sites
You don't need to make hwnd a pointer.

I didn't think you could get access violations, or indeed any other type of exceptions, with DestroyWindow. I thought errors were returned in the return value of the function.

The
(cmp dword ptr [eax+58h],0)
may be part of the windows DLLs.

##### Share on other sites
cmp dword ptr [eax+58h],0 can also be a check if hwnd == 0 or something from hwnd == 0..
the errors returned are in GetLastError(), the function only returns 0 or a nonzero number.
Here is what i did:
main.cpp (console.cpp):
// console.cpp : Defines the entry point for the console application.////#include "stdafx.h"#include "c:\\pex\\tribox\\stdafx.h"//#include <stdio.h>#pragma comment(lib,"c:\\pex\\tribox\\release\\tribox.lib")int main(int argc, char* argv[]){	Window win;	win.createFS();	win.toggle();	win.run();	return 0;}

now, inside the library (i skip functions that weren't used)
	Window()			{hRC=0;hwnd=0;hDC=0; bits=24;						 width=height=250; x=320; y=250;						 title="Tribox Engine Demo";}int Window::createFS(char* t,...){	char text[256];	enumAll();	va_list ap;	fullscreen=0;	if (!t)		t="Tribox Engine Demo";	va_start(ap,t);		wvsprintf(text,t,ap);	va_end(ap);	fullscreen=1;	title = text;	WNDCLASSEX	wc;	DWORD		dwExStyle;	DWORD		dwStyle;    wc.cbSize        = sizeof(WNDCLASSEX);    wc.style         = 0;    wc.lpfnWndProc   = SWndProc;    wc.cbClsExtra    = 0;    wc.cbWndExtra    = 0;    wc.hInstance     = this->hInstance;    wc.hIcon         = LoadIcon(NULL, IDI_APPLICATION);    wc.hCursor       = LoadCursor(NULL, IDC_ARROW);    wc.hbrBackground = (HBRUSH)(COLOR_WINDOW);    wc.lpszMenuName  = NULL;    wc.lpszClassName = title;    wc.hIconSm       = LoadIcon(NULL, IDI_APPLICATION);	if (!RegisterClassEx(&wc))		return 1;	this->wc = wc;	DEVMODE dmScreenSettings;	ZeroMemory(&dmScreenSettings, sizeof(dmScreenSettings));	dmScreenSettings.dmSize = sizeof(dmScreenSettings);	dmScreenSettings.dmPelsWidth = this->width;	dmScreenSettings.dmPelsHeight = this->height;	dmScreenSettings.dmBitsPerPel = bits;	dmScreenSettings.dmFields = DM_BITSPERPEL|DM_PELSWIDTH|DM_PELSHEIGHT;	long ret = ChangeDisplaySettings(&dmScreenSettings,CDS_FULLSCREEN);	if(ret != DISP_CHANGE_SUCCESSFUL)	{		EnumDisplaySettings(0,ENUM_CURRENT_SETTINGS,&dmScreenSettings);		ret = ChangeDisplaySettings(&dmScreenSettings,CDS_FULLSCREEN);		if (ret != DISP_CHANGE_SUCCESSFUL)			return 0;	}	dwExStyle = WS_EX_APPWINDOW;	dwStyle	  = WS_POPUP;	hideMouse();	this->hwnd=CreateWindowEx(dwExStyle,this->wc.lpszClassName,title,							 WS_CLIPSIBLINGS|WS_CLIPCHILDREN|dwStyle,							 0,0,width,height,0,0,hInstance,0);	if (this->hwnd==0)		return 0;	ShowWindow(this->hwnd,SW_SHOW);	UpdateWindow(this->hwnd);	return 1;}int Window::toggle(){	if (fullscreen)		enumAll();	close();	fullscreen=!fullscreen;	WNDCLASSEX	wc;	DWORD		dwExStyle;	DWORD		dwStyle;	hInstance=0;    wc.cbSize        = sizeof(WNDCLASSEX);    wc.style         = 0;    wc.lpfnWndProc   = SWndProc;    wc.cbClsExtra    = 0;    wc.cbWndExtra    = 0;    wc.hInstance     = this->hInstance;    wc.hIcon         = LoadIcon(NULL, IDI_APPLICATION);    wc.hCursor       = LoadCursor(NULL, IDC_ARROW);    wc.hbrBackground = (HBRUSH)(COLOR_WINDOW);    wc.lpszMenuName  = NULL;    wc.lpszClassName = title;    wc.hIconSm       = LoadIcon(NULL, IDI_APPLICATION);	if (!RegisterClassEx(&wc))		return 0;	this->wc = wc;	if (fullscreen)	{		DEVMODE dmScreenSettings;		ZeroMemory(&dmScreenSettings, sizeof(dmScreenSettings));		dmScreenSettings.dmSize = sizeof(dmScreenSettings);		dmScreenSettings.dmPelsWidth = this->width;		dmScreenSettings.dmPelsHeight = this->height;		dmScreenSettings.dmBitsPerPel = bits;		dmScreenSettings.dmFields = DM_BITSPERPEL|DM_PELSWIDTH|DM_PELSHEIGHT;		long ret = ChangeDisplaySettings(&dmScreenSettings,CDS_FULLSCREEN);		if(ret != DISP_CHANGE_SUCCESSFUL)		{			EnumDisplaySettings(0,ENUM_CURRENT_SETTINGS,&dmScreenSettings);			ret = ChangeDisplaySettings(&dmScreenSettings,CDS_FULLSCREEN);			if (ret != DISP_CHANGE_SUCCESSFUL)				return 0;		}		dwExStyle = WS_EX_APPWINDOW;		dwStyle	  = WS_POPUP;		hideMouse();	}	else	{		dwExStyle = WS_EX_APPWINDOW|WS_EX_WINDOWEDGE;		dwStyle   = WS_OVERLAPPEDWINDOW;		RECT winRect;		winRect.left = 0;		winRect.right = width;		winRect.top = 0;		winRect.bottom = height;		AdjustWindowRectEx(&winRect,dwStyle,FALSE,dwExStyle);	}	this->hwnd=CreateWindowEx(dwExStyle,this->wc.lpszClassName,title,							 WS_CLIPSIBLINGS|WS_CLIPCHILDREN|dwStyle,							 x,y,width,height,0,0,hInstance,0);	if (this->hwnd==0)		return 0;	ShowWindow(this->hwnd,SW_SHOW);	UpdateWindow(this->hwnd);	return 1;}void Window::close(){	if (this->hRC)		removeOpengl();	if (fullscreen)	{		ChangeDisplaySettings(0,0);		showMouse();	}	char* h="";	GetClassName(hwnd,h,256);	if (hwnd)		DestroyWindow(hwnd);	hwnd=0;	UnregisterClass(h,hInstance);}void Window::removeOpengl(){	if (hRC)	{		wglMakeCurrent(0,0);		wglDeleteContext(hRC);		hRC=0;	}	if (hwnd && hDC)	{		ReleaseDC(hwnd,hDC);		hDC=0;	}}

thx, pex.

##### Share on other sites
char text[256];enumAll();va_list ap;fullscreen=0;if (!t)	t="Tribox Engine Demo";va_start(ap,t);	wvsprintf(text,t,ap);va_end(ap);fullscreen=1;title = text;

You leave title pointing to random memory when this function finishes because text is a local array. JUSS (just use std::string)!

Enigma

##### Share on other sites
strcmp(win.getTitle(),"Tribox Engine Demo") returns zero, so what's wrong with title (win.getTitle())?

edit: I just found that I don't get the access violation error if i comment GetClassName(), or if i comment DestroyWindow()
i have an other way to get the class name, so i probably will this way.
but i still cant understand what is the reason for that.
in past i remember i wasn't using GetClassName() but got the access violation error.
weird.

[Edited by - pex22 on December 14, 2004 10:41:26 AM]

##### Share on other sites
Beware the difference between "working" and "correct". Using a pointer to access a variable that no longer exists is undefined behaviour. Undefined behaviour means it may well do exactly what you want... for now. Observe:
#include <iostream>void func(char*& string){	char text[16];	text[0] = '!';	text[1] = '\0';	string = text;}void print(char*& string){	int dummy[4];	std::cout << string << '\n';}int main(){	char* string;	func(string);	print(string);}

This code "works" on every compiler I tested it on. It prints '!'. However, it is not correct. A small change to the print function yields completely different behaviour:
#include <iostream>void func(char*& string){	char text[16];	text[0] = '!';	text[1] = '\0';	string = text;}void print(char*& string){	int dummy[4];	dummy[0] = 48;	std::cout << string << '\n';}int main(){	char* string;	func(string);	print(string);}

It now prints '0' on every compiler I tested it on.

I just spotted another bug in your code:
char* h="";GetClassName(hwnd,h,256);

GetClassName expects you to pass it a buffer and the number of characters it's allowed to write to that buffer. In this case you've told it that it's safe to write upto 256 characters to the buffer, but the buffer you pass is not 256 characters long. It may not even be writeable! It is a constant array of characters of length 1. This code should be e.g.:
char className[256];GetClassName(hwnd, className, 256);

This correctly passes a 256 character writeable buffer.

Enigma

##### Share on other sites
hm..right...
i used strcpy() instead, is that ok?
and i found that i keep a WNDCLASSEX in Window class so i used it instead of GetClassName
so, now i dont get acceess violation error and all functions works fine [smile], exept one :(
i wrote printf("%d\n",GetLastError()); in the WndProc (SWndProc), and in Window::close() i wrote printf(wc.lpszClassName);printf("\n");

when i call win.create(or createFS)() only, in console i see a lot of 0s and the class name ("Tribox Engine Demo"). (then it quits the program, as it suppose to do)
when i add win.toggle(), i get same chars but now i get it twice (because i first created a window, then closed it and now created again.) (then, quits. of course, no main loop.)
however, when i add win.run(), in the middle of the second window i get "Out of loop!" message and program ends up. (this message is called after:
while(GetMessage(&msg, NULL, 0, 0) > 0)
{
TranslateMessage(&msg);
DispatchMessage(&msg);
}
)

However, when i just call win.create(or createFS)() and win.run(), everything run fine.
win.toggle now is just:
int Window::toggle(){	if (!togglerogl()) // called to re-initialize opengl automatically		return 0;  // if opengl was called	fullscreen=!fullscreen;	if (fullscreen)		return createFS(title);	else		return create(title);}

the only non-0 i have seen is when i call just create() and run()--and the number is 183 (i think) which means cannot create the file because its already exists, or something like that. so i guess its not something serious, right? i get it only once (per application with just create() and run()).

pex.

[Edited by - pex22 on December 15, 2004 4:43:20 AM]

##### Share on other sites
Quote:
 Original post by pex22hm..right...i used strcpy() instead, is that ok?

I'm assuming you mean as a replacement for title = text? It's safe provided you spotted all the other changes you would need to make. You must allocate memory for title (or use strdup) and you must free that memory at some point (probably in the destructor). In addition because you are freeing the memory you must ensure that no codepath copies a statically allocated string into title, because such memory cannot be freed. Do you really want to take responsibility for all that? Just use std::string. There is (almost) no reason not to use it. It's safe. It's portable. It's efficient. And trust me, you're not dealing with the one case where std::string may be sub-optimal.

Quote:
 and i found that i keep a WNDCLASSEX in Window class so i used it instead of GetClassNameso, now i dont get acceess violation error and all functions works fine [smile], exept one :(

Again, beware of confusing "works" with "correct". If you missed any of the steps I listed above then your program is not "correct", even if it may "work" at the moment.

Quote:
 i wrote printf("%d\n",GetLastError()); in the WndProc (SWndProc), and in Window::close() i wrote printf(wc.lpszClassName);printf("\n");when i call win.create(or createFS)() only, in console i see a lot of 0s and the class name ("Tribox Engine Demo"). (then it quits the program, as it suppose to do)when i add win.toggle(), i get same chars but now i get it twice (because i first created a window, then closed it and now created again.) (then, quits. of course, no main loop.)however, when i add win.run(), in the middle of the second window i get "Out of loop!" message and program ends up. (this message is called after: while(GetMessage(&msg, NULL, 0, 0) > 0) { TranslateMessage(&msg); DispatchMessage(&msg); })However, when i just call win.create(or createFS)() and win.run(), everything run fine.win.toggle now is just:*** Source Snippet Removed ***the only non-0 i have seen is when i call just create() and run()--and the number is 183 (i think) which means cannot create the file because its already exists, or something like that. so i guess its not something serious, right? i get it only once (per application with just create() and run()).thanks in advance,pex.

Any error is serious, expecially if you don't know what's causing it. Can you post your current source for create, createFS, run and any functions that they call?

Enigma

##### Share on other sites
Your frequent and unnecessary use of this-> is VERY dodgy.
For example the line:
this->wc = wc;
Since this-> is always implicit, you could delete the local variable wc and this line would still compile just fine, but do nothing.

You should almost NEVER have to use this, usually only when passing the class to another function or something like that. In your case I would say that you could remove every occurence of it. This would of course make the line wc = wc; no longer work. But you should never have used the same name to begin with.

I know typing this-> makes that nice little dropdown list of class variables appear in MSVC so you don't have to remember what it's called etc etc. However you should delete the this-> part once you get the variable name.

You might like to instead declare class member variable names with m_ or similiar prefix.

##### Share on other sites
While most of the occurances of this-> are superflous, for the example you picked it is actually necessary:
WNDCLASSEX	wc;// codethis->wc = wc; // assigns the local wc variable to the class member wc variable

Of course he could just use the class wc member directly, but that's another matter. Using this-> is not exactly dodgy. Most people don't use it because it requires extra typing, but it qualifies member variables just as well as any other notation would (i.e. m_ prefix).

Enigma

##### Share on other sites
hm, i removed all the this->'s, used wc directly from the class and used std::string instead. but still same problem.
about the strcpy() thing, before i used it i saw that in the end of the program "log" i saw a jirbish instead of the title that was written in the beginning of the program "log".
so i used strcpy() instead and saw the title instead of the jirbish, so i was thinking that its ok to use that instead of changing the title type. it takes time to change almost everything to .c_str(), and all the rest things. but now i guess it doesn't matter. i forgot to mention that before.
i still couldn't use strcpy() instead, right? ok.
Window constructor:
	Window()			{hRC=0;hwnd=0;hDC=0; bits=24;						 width=height=250; x=320; y=250;						 opengled=oglsbits=ogldbits=oglcbits=0;}

win.createFS():
int Window::createFS(const char* t,...){	char text[256];	enumAll();	va_list ap;	if (!t)		t="Tribox Engine Demo";	va_start(ap,t);		wvsprintf(text,t,ap);	va_end(ap);	fullscreen=1;	title = text;	DWORD		dwExStyle;	DWORD		dwStyle;    wc.cbSize        = sizeof(WNDCLASSEX);    wc.style         = 0;    wc.lpfnWndProc   = SWndProc;    wc.cbClsExtra    = 0;    wc.cbWndExtra    = 0;    wc.hInstance     = hInstance;    wc.hIcon         = LoadIcon(NULL, IDI_APPLICATION);    wc.hCursor       = LoadCursor(NULL, IDC_ARROW);    wc.hbrBackground = (HBRUSH)(COLOR_WINDOW);    wc.lpszMenuName  = NULL;    wc.lpszClassName = title.c_str();    wc.hIconSm       = LoadIcon(NULL, IDI_APPLICATION);	if (!RegisterClassEx(&wc))		return 1;	DEVMODE dmScreenSettings;	ZeroMemory(&dmScreenSettings, sizeof(dmScreenSettings));	dmScreenSettings.dmSize = sizeof(dmScreenSettings);	dmScreenSettings.dmPelsWidth = width;	dmScreenSettings.dmPelsHeight = height;	dmScreenSettings.dmBitsPerPel = bits;	dmScreenSettings.dmFields = DM_BITSPERPEL|DM_PELSWIDTH|DM_PELSHEIGHT;	long ret = ChangeDisplaySettings(&dmScreenSettings,CDS_FULLSCREEN);	if(ret != DISP_CHANGE_SUCCESSFUL)	{		EnumDisplaySettings(0,ENUM_CURRENT_SETTINGS,&dmScreenSettings);		ret = ChangeDisplaySettings(&dmScreenSettings,CDS_FULLSCREEN);		if (ret != DISP_CHANGE_SUCCESSFUL)			return 0;	}	dwExStyle = WS_EX_APPWINDOW;	dwStyle	  = WS_POPUP;	hideMouse();	hwnd=CreateWindowEx(dwExStyle,wc.lpszClassName,title.c_str(),							 WS_CLIPSIBLINGS|WS_CLIPCHILDREN|dwStyle,							 0,0,width,height,0,0,hInstance,0);	if (hwnd==0)		return 0;	ShowWindow(hwnd,SW_SHOW);	UpdateWindow(hwnd);	return 1;}

win.size(100,100):
void Window::size(int w,int h){	width = w;	height = h;}

win.toggle():
int Window::toggle(){	if (!togglerogl())		return 0;	fullscreen=!fullscreen;	if (fullscreen)		return createFS(title.c_str());	else		return create(title.c_str());}

togglerogl():
int Window::togglerogl(){	/* Window::close() */	if (hRC)	{		/* Window::removeOpengl() */		if (hRC)		{			wglMakeCurrent(0,0);			wglDeleteContext(hRC);			hRC=0;		}		if (hwnd && hDC)		{			ReleaseDC(hwnd,hDC);			hDC=0;		}		/* End of Window::removeOpengl() */	}	if (fullscreen)	{		ChangeDisplaySettings(0,0);		showMouse();	}	if (hwnd)	{		wdawdq=0;		DestroyWindow(hwnd);		wdawdq=1;		printf(wc.lpszClassName);printf("\n");		UnregisterClass(wc.lpszClassName,hInstance);		hwnd=0;	}	/* End of Window::close() */	if (opengled)		if (!opengl(opengled,oglcbits,ogldbits,oglsbits))			return 0;	return 1;}

create():
int Window::create(const char* t,...){	char text[256];	va_list ap;	fullscreen=0;	if (!t)		t="Tribox Engine Demo";	va_start(ap,t);		wvsprintf(text,t,ap);	va_end(ap);	title = text;    wc.cbSize        = sizeof(WNDCLASSEX);    wc.style         = 0;    wc.lpfnWndProc   = SWndProc;    wc.cbClsExtra    = 0;    wc.cbWndExtra    = 0;    wc.hInstance     = hInstance;    wc.hIcon         = LoadIcon(NULL, IDI_APPLICATION);    wc.hCursor       = LoadCursor(NULL, IDC_ARROW);    wc.hbrBackground = (HBRUSH)(COLOR_WINDOW);    wc.lpszMenuName  = NULL;    wc.lpszClassName = title.c_str();    wc.hIconSm       = LoadIcon(NULL, IDI_APPLICATION);	if (!RegisterClassEx(&wc))		return 0;	hwnd=CreateWindowEx(WS_EX_CLIENTEDGE,wc.lpszClassName,title.c_str(),							 WS_OVERLAPPEDWINDOW,x,y,width,height,							 0,0,hInstance,0);	if (hwnd==0)		return 0;	ShowWindow(hwnd,SW_SHOW);	UpdateWindow(hwnd);	return 1;}

and finally..
win.run() // which calls setActive():
void Window::setActive(){	if (isActive() && ActiveWindow!=0)		return;	SetForegroundWindow(hwnd);	SetFocus(hwnd);	ActiveWindow = this;	MSG msg;    while(GetMessage(&msg, NULL, 0, 0) > 0)    {        TranslateMessage(&msg);        DispatchMessage(&msg);    }	printf("Out of loop!\n");}

isActive():
int Window::isActive(){	if (this==ActiveWindow)		return 1;	return 0;}

WndProc is:
	LRESULT CALLBACK WndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam){	printf("%d\n",GetLastError());    switch(msg)    {        case WM_CLOSE:            DestroyWindow(hwnd);        break;        case WM_DESTROY:			if (wdawdq)				ActiveWindow->close();			PostQuitMessage(0);        break;        default:            return DefWindowProc(hwnd, msg, wParam, lParam);    }    return 0;}

SWndProc is:
static LRESULT CALLBACK SWndProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam){  return ActiveWindow->WndProc(hwnd,msg,wParam,lParam);}

and before everything:
class Window;static Window* ActiveWindow=0;static int     wdawdq=1;#undef Window

wdawdq means: Window Destroy Active Window Do Quit
i think, im not sure about the last 2 words..i didnt knew how to call it.

thanks,
pex.

[Edited by - pex22 on December 16, 2004 3:39:33 PM]

##### Share on other sites

This topic is 4745 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Create an account

Register a new account

• ### Forum Statistics

• Total Topics
628682
• Total Posts
2984197

• 13
• 12
• 9
• 10
• 10