Game crashes randomly

(t0mt64=tomt64, spambot prevention)

I use the following libraries with my game:

SDL, SDL_image, SDL_mixer, SDL_ttf, SDL_gfx, and PHYSFS

All seems to work fine, and as far as I can tell I have no memory leaks
(valgrind supports this as of this writing, to the best of my
knowledge). I can’t give the entire code over email so I’m just going
to link you up. http://www.t0mt64.com/fftrader/fftrader-src.zip

The source does not extract to its own directory.

The problem is that in Windows XP, the game crashes randomly. It may
not be SO random, as I can fairly easily reproduce the error by picking
a Fighter Ship and zooming in and out a few times. I also notice that
the memory leaks an MB or two every time I zoom out, then zoom in (x and
z). I have done my best to close/delete all leaks and so on. I have no
idea what’s causing the leaks or the crash, but would appreciate any
help anyone could offer.

On a secondary note I have tried using GLSDL with this game to no
avail. A version of the code that is slightly outdated in which I
attempted to do this is here:
http://www.t0mt64.com/fftrader/fftrader-src-glsdl.zip

Thanks in advance,
TomT64

(t0mt64=tomt64, spambot prevention)

Huh?

The problem is that in Windows XP, the game crashes randomly.

Would it not make more sense to run the game through a debugger and
catch the crash? You should then be able to back trace and find what
function caused the crash to occur :)On Fri, 2004-03-26 at 01:35, TomT64 wrote:


http://www.mattsscripts.co.uk/

  • A great source for free CGI and stuff

Our OS who art in CPU, UNIX be thy name.
Thy programs run, thy syscalls done,
In kernel as it is in user!

Matt Wilson wrote:>On Fri, 2004-03-26 at 01:35, TomT64 wrote:

(t0mt64=tomt64, spambot prevention)

Huh?

The problem is that in Windows XP, the game crashes randomly.

Would it not make more sense to run the game through a debugger and
catch the crash? You should then be able to back trace and find what
function caused the crash to occur :slight_smile:


http://www.mattsscripts.co.uk/

  • A great source for free CGI and stuff

Our OS who art in CPU, UNIX be thy name.
Thy programs run, thy syscalls done,
In kernel as it is in user!


SDL mailing list
SDL at libsdl.org
http://www.libsdl.org/mailman/listinfo/sdl

Believe me, I’ve tried. I figure someone who knows more about the inner
workings of things my program is using than I do would be able to help.

And the links are obfuscated, that’s what that equation is for.

-TomT64

Hi,

did a bit of testing on XP and as a first pass I found a couple of things that
were naughty:

In ‘graphics.cpp’ CenterWindow() crashed in a very unpleasant manner in
release mode only:

#ifdef WIN32
RECT rc;
// HWND hwnd = FindWindow(NULL, “Final Frontier Trader”);
HWND hwnd = info.window;
w=GetSystemMetrics(SM_CXSCREEN);
h=GetSystemMetrics(SM_CYSCREEN);
GetWindowRect(hwnd, &rc);
x = (w - (rc.right-rc.left))/2;
y = (h - (rc.bottom-rc.top))/2;
// MoveWindow(hwnd, x, y, w, h, TRUE);
SetWindowPos(hwnd, NULL, x, y, 0, 0, SWP_NOSIZE|SWP_NOZORDER);
#endif

You should be using the window handle provided in the SDL_SysWMinfo structure
and your MoveWindow() is changing the window size to that of the desktop(!)
SetWindowPos() can just move the window (great function names from MS…).

In ‘main.cpp’ you are using (your) LoadIMG() before the video surface has been
initialized. (Your) LoadIMG() uses SDL_DisplayFormatAlpha() which returns
NULL in this circumstance. I made this function LoadICON():

SDL_Surface *LoadICON(std::string imgfile)
{
PHYSFS_file *temp=PHYSFS_openRead(imgfile.c_str());
if (!temp)
{
std::string temp2="Could not open file ";
temp2+=imgfile;
FPK_Invalid(“Unknown FPK!”, temp2);
}
SDL_RWops *rw=create_rwops(temp);
SDL_Surface *tempsurf=IMG_Load_RW(rw,0);
PHYSFS_close(temp);
SDL_FreeRW(rw);
return tempsurf;
}

but you should probably do a little refactoring here.

Also, to detect memory leaks and trashings in a win32 application I can
recommend using the crtdbg facilites supplied by MS. Make one of the
following the first line in main() and you’ll catch nearly everything (in
debug mode):

#include <crtdbg.h>

int main ( int argc, char argv[] )
{
/
a bit slow, but very useful
_CrtSetDbgFlag(
_CRTDBG_LEAK_CHECK_DF|_CRTDBG_ALLOC_MEM_DF
);
*/

/* very slow, very paranoid
_CrtSetDbgFlag(
_CRTDBG_LEAK_CHECK_DF|_CRTDBG_ALLOC_MEM_DF|_CRTDBG_CHECK_ALWAYS_DF
);
*/

}

Apart from the CenterWindow() problem at startup, I couldn’t get it to go
wrong, but there did seem to be a fair number of memory leaks, and it was
pretty slow,

good luck,
John Popplewell.On Friday 26 March 2004 10:50, TomT64 wrote:

Matt Wilson wrote:

On Fri, 2004-03-26 at 01:35, TomT64 wrote:

(t0mt64=tomt64, spambot prevention)

Huh?

The problem is that in Windows XP, the game crashes randomly.

Would it not make more sense to run the game through a debugger and
catch the crash? You should then be able to back trace and find what
function caused the crash to occur :slight_smile:


http://www.mattsscripts.co.uk/

  • A great source for free CGI and stuff

Our OS who art in CPU, UNIX be thy name.
Thy programs run, thy syscalls done,
In kernel as it is in user!


SDL mailing list
SDL at libsdl.org
http://www.libsdl.org/mailman/listinfo/sdl

Believe me, I’ve tried. I figure someone who knows more about the inner
workings of things my program is using than I do would be able to help.

And the links are obfuscated, that’s what that equation is for.

-TomT64


SDL mailing list
SDL at libsdl.org
http://www.libsdl.org/mailman/listinfo/sdl

Hi,

did a bit of testing on XP and as a first pass I found a couple of things that
were naughty:

In ‘graphics.cpp’ CenterWindow() crashed in a very unpleasant manner in
release mode only:

#ifdef WIN32
RECT rc;
// HWND hwnd = FindWindow(NULL, “Final Frontier Trader”);
HWND hwnd = info.window;
w=GetSystemMetrics(SM_CXSCREEN);
h=GetSystemMetrics(SM_CYSCREEN);
GetWindowRect(hwnd, &rc);
x = (w - (rc.right-rc.left))/2;
y = (h - (rc.bottom-rc.top))/2;
// MoveWindow(hwnd, x, y, w, h, TRUE);
SetWindowPos(hwnd, NULL, x, y, 0, 0, SWP_NOSIZE|SWP_NOZORDER);
#endif

You should be using the window handle provided in the SDL_SysWMinfo structure
and your MoveWindow() is changing the window size to that of the desktop(!)
SetWindowPos() can just move the window (great function names from MS…).

This is another thing I’ve been concerned about, CenterWindow. Isn’t
there a flag or something that centers a window in SDL?

In ‘main.cpp’ you are using (your) LoadIMG() before the video surface has been
initialized. (Your) LoadIMG() uses SDL_DisplayFormatAlpha() which returns
NULL in this circumstance. I made this function LoadICON():

SDL_Surface *LoadICON(std::string imgfile)
{
PHYSFS_file *temp=PHYSFS_openRead(imgfile.c_str());
if (!temp)
{
std::string temp2="Could not open file ";
temp2+=imgfile;
FPK_Invalid(“Unknown FPK!”, temp2);
}
SDL_RWops *rw=create_rwops(temp);
SDL_Surface *tempsurf=IMG_Load_RW(rw,0);
PHYSFS_close(temp);
SDL_FreeRW(rw);
return tempsurf;
}

but you should probably do a little refactoring here.

Implemeted this, and temporarily removed the CenterWindow calls until i
read up on what you suggested, but it didn’t solve the problem. They
are good changes though.

Also, to detect memory leaks and trashings in a win32 application I can
recommend using the crtdbg facilites supplied by MS. Make one of the
following the first line in main() and you’ll catch nearly everything (in
debug mode):

#include <crtdbg.h>

int main ( int argc, char argv[] )
{
/
a bit slow, but very useful
_CrtSetDbgFlag(
_CRTDBG_LEAK_CHECK_DF|_CRTDBG_ALLOC_MEM_DF
);
*/

/* very slow, very paranoid
_CrtSetDbgFlag(
_CRTDBG_LEAK_CHECK_DF|_CRTDBG_ALLOC_MEM_DF|_CRTDBG_CHECK_ALWAYS_DF
);
*/

}

Apart from the CenterWindow() problem at startup, I couldn’t get it to go
wrong, but there did seem to be a fair number of memory leaks, and it was
pretty slow,

good luck,
John Popplewell.

Ok, once I get those lines into the program, how do I use it exactly?

Thanks for your help.

-TomT64

Hi,

did a bit of testing on XP and as a first pass I found a couple of things
that were naughty:

In ‘graphics.cpp’ CenterWindow() crashed in a very unpleasant manner in
release mode only:

#ifdef WIN32
RECT rc;
// HWND hwnd = FindWindow(NULL, “Final Frontier Trader”);
HWND hwnd = info.window;
w=GetSystemMetrics(SM_CXSCREEN);
h=GetSystemMetrics(SM_CYSCREEN);
GetWindowRect(hwnd, &rc);
x = (w - (rc.right-rc.left))/2;
y = (h - (rc.bottom-rc.top))/2;
// MoveWindow(hwnd, x, y, w, h, TRUE);
SetWindowPos(hwnd, NULL, x, y, 0, 0, SWP_NOSIZE|SWP_NOZORDER);
#endif

You should be using the window handle provided in the SDL_SysWMinfo
structure and your MoveWindow() is changing the window size to that of
the desktop(!) SetWindowPos() can just move the window (great function
names from MS…).

This is another thing I’ve been concerned about, CenterWindow. Isn’t
there a flag or something that centers a window in SDL?

Yes there is, but it doesn’t seem to apply to windib or directx. See
SDL_VIDEO_CENTERED here: http://sdldoc.csn.ul.ie/sdlenvvars.php

I had a quick look through the source and this seems to be the case.

In ‘main.cpp’ you are using (your) LoadIMG() before the video surface has
been initialized. (Your) LoadIMG() uses SDL_DisplayFormatAlpha() which
returns NULL in this circumstance. I made this function LoadICON():

SDL_Surface *LoadICON(std::string imgfile)
{
PHYSFS_file *temp=PHYSFS_openRead(imgfile.c_str());
if (!temp)
{
std::string temp2="Could not open file ";
temp2+=imgfile;
FPK_Invalid(“Unknown FPK!”, temp2);
}
SDL_RWops *rw=create_rwops(temp);
SDL_Surface *tempsurf=IMG_Load_RW(rw,0);
PHYSFS_close(temp);
SDL_FreeRW(rw);
return tempsurf;
}

but you should probably do a little refactoring here.

Implemeted this, and temporarily removed the CenterWindow calls until i
read up on what you suggested, but it didn’t solve the problem. They
are good changes though.

I think it was probably a quirk of the video drivers I’m using.

Also, to detect memory leaks and trashings in a win32 application I can
recommend using the crtdbg facilites supplied by MS. Make one of the
following the first line in main() and you’ll catch nearly everything (in
debug mode):

#include <crtdbg.h>

int main ( int argc, char argv[] )
{
/
a bit slow, but very useful
_CrtSetDbgFlag(
_CRTDBG_LEAK_CHECK_DF|_CRTDBG_ALLOC_MEM_DF
);
*/

/* very slow, very paranoid
_CrtSetDbgFlag(
_CRTDBG_LEAK_CHECK_DF|_CRTDBG_ALLOC_MEM_DF|_CRTDBG_CHECK_ALWAYS_DF
);
*/

}

Apart from the CenterWindow() problem at startup, I couldn’t get it to go
wrong, but there did seem to be a fair number of memory leaks, and it was
pretty slow,

good luck,
John Popplewell.

Ok, once I get those lines into the program, how do I use it exactly?

Using Visual Studio, build the program (and libraries) in debug mode and then
run in debug mode using F5. Make sure the output window is visible. When the
program exists you get a list of all the memory leaks e.g.

Detected memory leaks!
Dumping objects ->
{160443} normal block at 0x04BAE940, 8 bytes long.
{160442} normal block at 0x04BAE8D8, 32 bytes long.
{160441} normal block at 0x04BB8070, 38416 bytes long.
{160440} normal block at 0x04BAE870, 40 bytes long.

The number in {} is related to the order in which the allocations occur and if
you can arrange it so that this number is the same each run, then, using e.g.
_CrtSetBreakAlloc(160443) just after the other _Crt* lines, will break
execution at the point where the lost memory is allocated allowing you to
identify it. The spaces inside < > and the hex digits are the first few
bytes of the block which can be useful. The length can also be useful for
tracking down the leak.

Of course, this works best if you use it throughout development. It is much
harder to find problems like this in a large body of partially debugged code.

It also helps find double-free bugs and stray-pointers that are writing where
they shouldn’t. The extra flag in the second set causes the checks for memory
over-writing to be performed at every allocation which slows things down a
lot but finds corrupted heap problems quite effectively. I only use this
occasionally.

If you have MSDN look it up there, it has quite a good explanation. Or this
very long link might work:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vsdebug/html/_core_solving_buffer_overwrites_and_memory_leaks.asp

Thanks for your help.

No problem. Here are the memory leaks I found so far:

  • call SDL_FreeSurface() after drawing the splash screen.
  • call Mix_CloseAudio() after freeing the sounds.
  • free dispimg in Unit::~Unit()
  • call SDL_FreeSurface(bg) in:
    AskToQuit(), YesNoBox(), ExitToMenuBox(), GameOverBox(),
    AskToQuitGame(), InGameMenu()
    (perhaps the background should be part of the GUI?)

cheers,
John.On Saturday 27 March 2004 05:44, TomT64 wrote:
Data: < > 00 00 00 00 00 00 00 00
Data: < > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Data: < > FF FF FF 00 FF FF FF 00 FF FF FF 00 FF FF FF 00
Data: < > 00 00 00 00 20 04 00 00 00 00 10 08 00 18 00 00

-TomT64


SDL mailing list
SDL at libsdl.org
http://www.libsdl.org/mailman/listinfo/sdl

No problem. Here are the memory leaks I found so far:

  • call SDL_FreeSurface() after drawing the splash screen.
  • call Mix_CloseAudio() after freeing the sounds.
  • free dispimg in Unit::~Unit()
  • call SDL_FreeSurface(bg) in:
    AskToQuit(), YesNoBox(), ExitToMenuBox(), GameOverBox(),
    AskToQuitGame(), InGameMenu()
    (perhaps the background should be part of the GUI?)

Hey I sent a reply to this from the wrong email address, so it is being
screened. It should post soon though. Basically I implemented all this
and uploaded the changes to CVS and the zips I mentioned, and though I
found the cause for the crashes (libpng) I am still leaking on a zoom
cycle (about 2MB per cycle, zoom out twice, then in twice). Further
details on the libpng problem are in the reply I sent the first time.

-TomT64

No problem. Here are the memory leaks I found so far:

  • call SDL_FreeSurface() after drawing the splash screen.
  • call Mix_CloseAudio() after freeing the sounds.
  • free dispimg in Unit::~Unit()
  • call SDL_FreeSurface(bg) in:
    AskToQuit(), YesNoBox(), ExitToMenuBox(), GameOverBox(),
    AskToQuitGame(), InGameMenu()
    (perhaps the background should be part of the GUI?)

Hey I sent a reply to this from the wrong email address, so it is being
screened. It should post soon though. Basically I implemented all this
and uploaded the changes to CVS and the zips I mentioned, and though I
found the cause for the crashes (libpng) I am still leaking on a zoom
cycle (about 2MB per cycle, zoom out twice, then in twice). Further
details on the libpng problem are in the reply I sent the first time.

Looking forward to receiving that.

I didn’t use any of the .dll files you supplied in the first .zip, since you
didn’t supply the required, matching, header files. Maybe that’s why I
couldn’t reproduce the fault.

Another surface leak (these matter if you are using hardware surfaces which
are often a severely limited resource):

void StaticAnim::LoadFrames(std::string sanim)
{
int i;
animbase=sanim;
SDL_Surface *temp;
char send[60];
for(i=1; i<13; i++)
{
sprintf(send,"%s%02d.png",sanim.c_str(), i);
temp=LoadIMAGE(send);
if (anim[i-1]) SDL_FreeSurface(anim[i-1]);
anim[i-1]=rotozoomSurface(temp, 0, zoom, SMOOTH);
SDL_FreeSurface(temp); // <--------------------
if (i==1)
{
w=anim[i-1]->w;
h=anim[i-1]->h;
}
}
//SDL_FreeSurface(temp);
}

BTW: should really be like all the other loops:

void StaticAnim::LoadFrames(std::string sanim)
{
int i;
SDL_Surface *temp;
char send[60]; // shiver … could try using snprintf()?

animbase=sanim;
for(i=0; i<12; i++) // this ‘12’ should be a #define everywhere
{
sprintf(send,"%s%02d.png",sanim.c_str(), i+1); // +1 here only
temp=LoadIMAGE(send);
if (anim[i]) SDL_FreeSurface(anim[i]);
anim[i]=rotozoomSurface(temp, 0, zoom, SMOOTH);
SDL_FreeSurface(temp);
if (i==0)
{
w=anim[i]->w;
h=anim[i]->h;
}
}
}

There are lots more leaks, but I think that’s the last of the easy ones.

Sorry, but the memory allocation schemes in ‘units.cpp’ is too confused for me
to work out what’s wrong with it :slight_smile: If I were you, I would consider a quick
re-write, now that you know what you want it to do.

Where the internals of objects are concerned, everytime you allocate a block
of memory or a resource, code up the one and only place where it is freed
(hint: the destructor can call the same function).

You can get rid of the booleans indicating that a resource is loaded by
testing the pointer to the resource directly and making sure that it is
initialised to 0 and set to 0 after it has been freed:

#define UNITS_NUM_FRAMES 72

Unit::Unit()
{
Reset();
}

Unit::~Unit()
{
Delete();
}

void Unit::Reset() // probably should be private
{
int i;
for (i=0; i<UNITS_NUM_FRAMES; i++)
{
mask[i]=0; smask[i]=0; image[i]=0; shield[i]=0;
}
dispimg=0;
…etc…
}

void Unit::Delete() // probably not a great choice of name, unless private
{
int i;
for (i=0; i<UNITS_NUM_FRAMES; i++)
{
if (mask[i]) { bitmask_free(mask[i]); mask[i]=0; }
if (smask[i]) { bitmask_free(smask[i]); smask[i]=0; }
if (image[i]) { SDL_FreeSurface(image[i]); image[i]=0; }
if (shield[i]) { SDL_FreeSurface(shield[i]); shield[i]=0; }
}
if ( dispimg ) { SDL_FreeSurface(dispimg); dispimg=0; }
…etc…
}

bool Unit::IsLoaded(int pic)
{
return bool(image[pic]!=0);
}

bool Unit::ShieldIsLoaded(int pic)
{
return bool(shield[pic]!=0);
}

This (widely-used) technique handles tricky cases where e.g. object are
part-constructed and the clean-up routines are called more than once (see
some Windows GUI programming …),

Good Luck with it,

cheers,
John.On Sunday 28 March 2004 06:18, TomT64 wrote:

-TomT64


SDL mailing list
SDL at libsdl.org
http://www.libsdl.org/mailman/listinfo/sdl

void StaticAnim::LoadFrames(std::string sanim)
{
int i;
SDL_Surface *temp;
char send[60]; // shiver … could try using snprintf()?

animbase=sanim;
for(i=0; i<12; i++) // this ‘12’ should be a #define everywhere
{
sprintf(send,"%s%02d.png",sanim.c_str(), i+1); // +1 here only
temp=LoadIMAGE(send);
if (anim[i]) SDL_FreeSurface(anim[i]);
anim[i]=rotozoomSurface(temp, 0, zoom, SMOOTH);
SDL_FreeSurface(temp);
if (i==0)
{
w=anim[i]->w;
h=anim[i]->h;
}
}
}

Thanks, can’t believe I missed that one!

Sorry, but the memory allocation schemes in ‘units.cpp’ is too confused for me
to work out what’s wrong with it :slight_smile: If I were you, I would consider a quick
re-write, now that you know what you want it to do.

Don’t know what you mean, unless you mean what I see below. In that
case, I fixed it.

Where the internals of objects are concerned, everytime you allocate a block
of memory or a resource, code up the one and only place where it is freed
(hint: the destructor can call the same function).

You can get rid of the booleans indicating that a resource is loaded by
testing the pointer to the resource directly and making sure that it is
initialised to 0 and set to 0 after it has been freed:

#define UNITS_NUM_FRAMES 72

Unit::Unit()
{
Reset();
}

Unit::~Unit()
{
Delete();
}

void Unit::Reset() // probably should be private
{
int i;
for (i=0; i<UNITS_NUM_FRAMES; i++)
{
mask[i]=0; smask[i]=0; image[i]=0; shield[i]=0;
}
dispimg=0;
…etc…
}

void Unit::Delete() // probably not a great choice of name, unless private
{
int i;
for (i=0; i<UNITS_NUM_FRAMES; i++)
{
if (mask[i]) { bitmask_free(mask[i]); mask[i]=0; }
if (smask[i]) { bitmask_free(smask[i]); smask[i]=0; }
if (image[i]) { SDL_FreeSurface(image[i]); image[i]=0; }
if (shield[i]) { SDL_FreeSurface(shield[i]); shield[i]=0; }
}
if ( dispimg ) { SDL_FreeSurface(dispimg); dispimg=0; }
…etc…
}

bool Unit::IsLoaded(int pic)
{
return bool(image[pic]!=0);
}

bool Unit::ShieldIsLoaded(int pic)
{
return bool(shield[pic]!=0);
}

This (widely-used) technique handles tricky cases where e.g. object are
part-constructed and the clean-up routines are called more than once (see
some Windows GUI programming …),

Good Luck with it,

cheers,
John.

I applied your changes and uploaded to CVS. I will upload to my server
and the links mentioned earlier in a while when my server gets back up.
Since my email server is on the same server, by the time this message
posts they should be uploaded. Thanks!

-TomT64

No problem. Here are the memory leaks I found so far:

  • call SDL_FreeSurface() after drawing the splash screen.
  • call Mix_CloseAudio() after freeing the sounds.
  • free dispimg in Unit::~Unit()
  • call SDL_FreeSurface(bg) in:
    AskToQuit(), YesNoBox(), ExitToMenuBox(), GameOverBox(),
    AskToQuitGame(), InGameMenu()
    (perhaps the background should be part of the GUI?)

Thank you! I fixed all this. Also I discovered finally that the
problem with zooming is an access violation to libpng1.dll. So I
relinked SDL_image with a newer libpng (1.0.15) and put both DLLs in the
game dir, and haven’t had problems since, even with stuff that was
compiled before the change!

I guess the old libpng had a problem with Windows XP/NT somehow. Anyway
that’s all fixed, and I’m checking for leaks again. So far I still have
about 2MB of leaks while making a zoom cycle. I uploaded the changes to
CVS and the old links.

-TomT64