Crashs after 3 restarts

[Ghoti]

Hi folks,

In my game i have a main menu, through this menu i can run the game. Ingame i can quit the game to the main menu.
then i can restart the game again and then quit again. Then another time i can run the game and quit again but the fourth time i try to run the game it always crashes.

This phenomona is not due to direct code i guess because it does it correctly 3 times. So i guess it is a memory leak or something? How exactly can i indentify an memory leak? ( haven't had it ever so i don't really what it is) and can it be also something different?

greets ghoti

[weak]

educated guess: you're not freeing all of the memory you allocate when either a) loading the menu b) loading the map

just check the amount of free memory after load and you'll see which part (menu/game) eats your memory. then go through your allocs and make sure you free them properly before you load new stuff ...

edit: of course you can keep stuff you'll need again in memory. just make sure you don't load it again. otherwise you'll most likey lose the pointer to the previously allocated memory -> memory hole

[Ghoti]

:) still need to learn C badly i see :)

well i have these image variables which i refill everytime with different images (a theme thing) i have now put code in place to first free the image from memory. And in those lines it crashes.

Code: Select all

printTextScreen(10, 120, "stap 2", 16777215);
		flipScreen();

		freeImage(Blocks[1]);
		freeImage(Blocks[2]);
		freeImage(Blocks[3]);
		freeImage(Blocks[4]);
		freeImage(Blocks[5]);
		freeImage(Blocks[6]);
		freeImage(Blocks[7]);
		freeImage(GameBackground);
		freeImage(Pause);
		freeImage(Line);
		freeImage(Message[1]);
		freeImage(Message[2]);
		freeImage(Message[3]);
		freeImage(Message[4]);
		freeImage(Message[5]);
		freeImage(Message[6]);
		freeImage(Message[7]);
		freeImage(Message[8]);
		freeImage(Fx[1]);
		freeImage(Fx[2]);
		freeImage(Fx[3]);
		freeImage(Fx[4]);
		freeImage(Fx[5]);
		
		printTextScreen(10, 130, "stap 3", 16777215);
		flipScreen();

the text stap 3 never reaches the screen soo i guess the problem is in the freeImage codes but they work 3 times in a row and the fourth suddenly they won't anymore

but come to think of it free the images of the themes but i don't with the menu images... going to look if that is the problem

[weak]

no idea what "freeImage" does, but you can't free undefined stuff. so your pointers must at least be initialized with NULL and reset to NULL after freeing. free(NULL) will just generate a NOP and shouldn't affect your code.

[Ghoti]

:S sorry but please in a bit more plain english (i'm not english)

but the image i pass in this function:

Code: Select all

void freeImage(Image* image)
{
	free(image->data);
	free(image);
}

are not empty because otherwise my game would have crashed earlier in the first 3 runs because it wanted to draw an image that has not been defined so i guess that is not the problem. however since it is in that code there should be an error there, how can i be certain that the image that is passed is not nothing?

[weak]

that code does not set the pointer to NULL after calling free(). so a consecutive call to free(foobar) will crash because foobar is most likely pointing in the wild.

edit: and how is Image defined? are you sure you're not trying to free a pointer to a struct?

[Ghoti]

hi,

well the problem is only with this line of code (narrowed it down)

Code: Select all

freeImage(Pause);

so it is my pause image. But before i free it i load it through this code:

Code: Select all

sprintf(sBuffer, "Themes/%i/pause.png", iCurrentThemes);
		Pause = loadImage(sBuffer);

and image is: (the image loading functions are not my code it is from the graphics.c from the yelarb tutorials) (searching.....

Code: Select all

typedef struct
{
	int textureWidth;  // the real width of data, 2^n with n>=0
	int textureHeight;  // the real height of data, 2^n with n>=0
	int imageWidth;  // the image width
	int imageHeight;
	Color* data;
} Image;

this is the code for the image if i am correct

the pause image is 480x272 but the background image is also so big so that should not be the problem

[weak]

(without knowing the code for "loadImage") it seems like the memory for the struct is dynamically allocated. so you should change the above code to:

Code: Select all

void freeImage(Image* image)
{
   free(image->data);
   image->data = NULL;  // obsolete
   free(image);
   image = NULL;
}

just to make sure you have no dangling pointers. and i think you're loading something into 'Pause', freeing it, and then you try to free it again without resetting the pointer or making it point to new data.

[Ghoti]

Hi,

i tried that but that didn't do the trick. I will try to figure out to display the memory looking if that is the problem ( i think it is because when i comment out the line of freeImage(Pause); then it crashes a few freeImages later (images after pause are a lot smaller so that looks like a memory bug when it crashes a little later after the images together are as big as the pause

or am i wrong?

[groepaz]

that code does not set the pointer to NULL after calling free(). so a consecutive call to free(foobar) will crash because foobar is most likely pointing in the wild.

please, only because this works for you (for whatever reason), dont make it look like good or evem common practise. free() does _NOT_ handle null pointers.

[Ghoti]

Hi folks, back again

I have managed to check the total free memory by using this function:

Code: Select all

sprintf(sBuffer, "Free Memory 2: %i", sceKernelTotalFreeMemSize());	
printTextScreen(10,120, sBuffer, 16777215);
		flipScreen();

however it says that i have like 700000 bytes of free memory left so i guess a memory leak is also not the case :S (this is frustrating)

[weak]

please, only because this works for you (for whatever reason), dont make it look like good or evem common practise. free() does _NOT_ handle null pointers.

at least you can check if the pointer is NULL before calling free. and as far as i know free(NULL) does nothing:

[groepaz]

as far as i know free(NULL) does nothing.

no, the behaviour is undefined.

[weak]

Unix Manual Page for free():

If ptr is a null pointer, no action occurs. If a random number is passed to free(), the results are undefined.

maybe that's wrong, never looked at the assembly of free(NULL), but if that's wrong someone should change the wiki entry of malloc and a load of other sources...

wiki entry for malloc:

To avoid this, some programmers set pointers to NULL after freeing them, as free(NULL) is safe—it does nothing.

[Ghoti]

well the code you gave didn't work so i guess that was not the problem do you have any idea what i can be?

[groepaz]

funny....i had some code crashing on my last nite because i was accidently doing exactly that, passing null to free :=P i'd rather not rely on it :)

[Jim]

7.20.3.2 of C99 standard says free(NULL) does nothing. "if ptr is a null pointer, no action occurs".

Jim

[weak]

thx for making that clear Jim.

@Ghoti:

hard to say what's exactly going wrong without seeing the whole source. but i'd suggest to read some stuff about the common errors with malloc on the wiki page. it should help you to get an idea what could be going wrong.

[dot_blank]

texture is aligned to 16?
static unsigned char __attribute__((aligned(16))) tex[*]{*};

and your mallocing like thus
tex.alloc = malloc(tex->width * tex->height + 16);

then freeing should have no problems if you memset after free
free(tex.alloc);
memset(tex, 0, sizeof(tex));

[groepaz]

Code: Select all

7.20.3.2 of C99 standard says free(NULL) does nothing. "if ptr is a null pointer, no action occurs".

mmmh is that new in C99 ?

[Jim]

No. It's been like that since K&R.

edit: I just checked newlib and although there're a lot of macros in there, the first thing it does is check for free(NULL) and returns right away.

Jim

[groepaz]

mmmh

guess i am (again) falling for my experience with crappy embedded compilers (and their libs) :=P

[Ghoti]

Hi back again,

I have looked at the malloc thingy and it is still a bit difficult for me but it is good for me to learn so i'll dig it further out :)

What i was wondering though, since my memory has always 700000 bytes free, why is the freeimage still causing problems? is it maybe because i do it multiple times over the same image container? and it was meant for only once?

and why does it work for 3 times, it can only be some memory thing if it crashes the same time after 3 reruns?

[memon]

Is the 1-based index to the arrays intentional? That is usually the first weapon which people coming from Lua world use to shoot their feet ;)

For example if the Message is defined like this:

Code: Select all

Image* Message[8];

You access the array from 0 to 7. Using index 8 may work, but it will write over some other data, which is not good :)

Also you could try to improve freeImage to clear all the pointers:

Code: Select all

void freeImage(Image** image)
{
   if(image == NULL || *image == NULL)
      return;
   free(*image->data);
   image->data = NULL;
   free(*image);
   *image = NULL;
}

In that case the call to freeImage becomes:

Code: Select all

freeImage(&Message[0]);

Setting the image to NULL inside freeImage in Weak's example does not change the actual image pointer (like the contents of Message[0]).

[Ghoti]

Hi,

I have never used LUA :) and the indices are correct, i like it that way (i know it is not the correct way to do but i just like it better this way)

I will try your new code but i have however two questions:

1. Using the sceKernelTotalFreeMemSize i can display my free memory, and during the crash it doesn't go below the 700000 bytes, is it still possible that it is the problem of that freeimage code?

2. Secondly I free the image, reload the image and display, when i restart the game i do it another time etc etc. the fourth time it crashes on the same code that worked for three times. I have also used the images that worked correctly as the fourth but that didn't gave the error also, can it still be that it is what ou think it is?

3. Is there an other way of seeing the memory or detecting if there will be a leak?

greets ghoti

[memon]

I have never used LUA :) and the indices are correct, i like it that way (i know it is not the correct way to do but i just like it better this way)

I call those "it's nice to keep that armed shotgun around" coding habits ;)

Do you check the return value of malloc and stop if it returns NULL?

Also, in a simple test program does it work if you just load and free a sigle image three times in a row? Also what happens to the memory consumption in that case, does the image freed correctly. Then what happens when you load all your images.

You could also try to make your own version of malloc/free which does some more logging, and checks if the freed memory is allocated or freed already. There is propably bunch of these already available, I have used the one from Fluid Studios in past:
http://www.fluidstudios.com/

[Ghoti]

Hi,

thanks for the reply,

I have now done it so that when i choose a menu item in my game it loads and free the same picture 7 times, it did not crash, after that i pressed like 20 times the X button to choose the menu item and still no crash (7 times 20 loads and frees)

also i have drawn the free memory and it was always the same as the last time i pressed the x button to choose the menu item.

Therefor i guess this is not the problem.

here is my free code and my load code please point to me where i have to deal with the malloc since i have never used this

Code: Select all

Image* loadImage(const char* filename)
{
	png_structp png_ptr;
	png_infop info_ptr;
	unsigned int sig_read = 0;
	png_uint_32 width, height;
	int bit_depth, color_type, interlace_type, x, y;
	u32* line;
	FILE *fp;
	Image* image = (Image*) malloc(sizeof(Image));
	if (!image) return NULL;

	if ((fp = fopen(filename, "rb")) == NULL) return NULL;
	png_ptr = png_create_read_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
	if (png_ptr == NULL) {
		free(image);
		fclose(fp);
		return NULL;;
	}
	png_set_error_fn(png_ptr, (png_voidp) NULL, (png_error_ptr) NULL, user_warning_fn);
	info_ptr = png_create_info_struct(png_ptr);
	if (info_ptr == NULL) {
		free(image);
		fclose(fp);
		png_destroy_read_struct(&png_ptr, png_infopp_NULL, png_infopp_NULL);
		return NULL;
	}
	png_init_io(png_ptr, fp);
	png_set_sig_bytes(png_ptr, sig_read);
	png_read_info(png_ptr, info_ptr);
	png_get_IHDR(png_ptr, info_ptr, &width, &height, &bit_depth, &color_type, &interlace_type, int_p_NULL, int_p_NULL);
	if (width > 512 || height > 512) {
		free(image);
		fclose(fp);
		png_destroy_read_struct(&png_ptr, png_infopp_NULL, png_infopp_NULL);
		return NULL;
	}
	image->imageWidth = width;
	image->imageHeight = height;
	image->textureWidth = getNextPower2(width);
	image->textureHeight = getNextPower2(height);
	png_set_strip_16(png_ptr);
	png_set_packing(png_ptr);
	if (color_type == PNG_COLOR_TYPE_PALETTE) png_set_palette_to_rgb(png_ptr);
	if &#40;color_type == PNG_COLOR_TYPE_GRAY && bit_depth < 8&#41; png_set_gray_1_2_4_to_8&#40;png_ptr&#41;;
	if &#40;png_get_valid&#40;png_ptr, info_ptr, PNG_INFO_tRNS&#41;&#41; png_set_tRNS_to_alpha&#40;png_ptr&#41;;
	png_set_filler&#40;png_ptr, 0xff, PNG_FILLER_AFTER&#41;;
	image->data = &#40;Color*&#41; memalign&#40;16, image->textureWidth * image->textureHeight * sizeof&#40;Color&#41;&#41;;
	if &#40;!image->data&#41; &#123;
		free&#40;image&#41;;
		fclose&#40;fp&#41;;
		png_destroy_read_struct&#40;&png_ptr, png_infopp_NULL, png_infopp_NULL&#41;;
		return NULL;
	&#125;
	line = &#40;u32*&#41; malloc&#40;width * 4&#41;;
	if &#40;!line&#41; &#123;
		free&#40;image->data&#41;;
		free&#40;image&#41;;
		fclose&#40;fp&#41;;
		png_destroy_read_struct&#40;&png_ptr, png_infopp_NULL, png_infopp_NULL&#41;;
		return NULL;
	&#125;
	for &#40;y = 0; y < height; y++&#41; &#123;
		png_read_row&#40;png_ptr, &#40;u8*&#41; line, png_bytep_NULL&#41;;
		for &#40;x = 0; x < width; x++&#41; &#123;
			u32 color = line&#91;x&#93;;
			image->data&#91;x + y * image->textureWidth&#93; =  color;
		&#125;
	&#125;
	free&#40;line&#41;;
	png_read_end&#40;png_ptr, info_ptr&#41;;
	png_destroy_read_struct&#40;&png_ptr, &info_ptr, png_infopp_NULL&#41;;
	fclose&#40;fp&#41;;
	return image;
&#125;

Code: Select all

void freeImage(Image* image)
{
		free(image->data);
		free(image);
}

it crashes with the free code

[Raphael]

Code: Select all

void freeImage(Image* image)
{
      free(image->data);
      free(image);
}

This will crash if image is NULL. So put in a check for that case before the image->data access like this:

Code: Select all

void freeImage(Image* image)
{
      if (image==NULL) return;
      free(image->data);
      free(image);
}