Sign in to follow this  

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.

If you intended to correct an error in the post then please contact us.

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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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 this post


Link to post
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()).

thanks in advance,
pex.

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

Share this post


Link to post
Share on other sites
Quote:
Original post by pex22
hm..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 this post


Link to post
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 this post


Link to post
Share on other sites
While most of the occurances of this-> are superflous, for the example you picked it is actually necessary:
WNDCLASSEX	wc;
// code
this->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

EDIT: Sorry, should have read your post more thoroughly to start with!

Share this post


Link to post
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 this post


Link to post
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.

If you intended to correct an error in the post then please contact us.

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