Allegro.cc - Online Community

Allegro.cc Forums » Programming Questions » freeing memory

This thread is locked; no one can reply to it. rss feed Print
freeing memory
shadyvillian
Member #12,426
December 2010

Do I have to call al_destroy_bitmap every time I do this:

img1 = img2

When ever I do it increases my memory usage everytime I do that unless I call that on it. And when I do use it, it crashes but it isn't empty.

Software Engineer by day, hacker by night.

Thomas Fjellstrom
Member #476
June 2000
avatar

If img1 is an ALLEGRO_BITMAP, that creates a leak, so yes, you want to call al_destroy_bitmap on img1. But you have to make sure you don't call al_destroy_bitmap on the same address twice. Also note, al_destroy_bitmap does not set the variable you pass to it to 0 or NULL. If you want that, you have to set it yourself.

--
Thomas Fjellstrom - [website] - [email] - [Allegro Wiki] - [Allegro TODO]
"If you can't think of a better solution, don't try to make a better solution." -- weapon_S
"The less evidence we have for what we believe is certain, the more violently we defend beliefs against those who don't agree" -- https://twitter.com/neiltyson/status/592870205409353730

shadyvillian
Member #12,426
December 2010

Oh I didn't know that. I'll give it a shot.

EDIT: Its still crashing.

1. image is null
2. assign image to image
3. destroy it(if image != NULL) - crashes here
4. make it null
5. assign another image to image

Software Engineer by day, hacker by night.

Edgar Reynaldo
Member #8,592
May 2007
avatar

Every ALLEGRO_BITMAP* you create, you should destroy. It doesn't matter how or when you assign it to another pointer as long as you don't lose a reference to a created bitmap before you destroy it.

Ex:

#SelectExpand
1ALLEGRO_BITMAP* created1 = al_create_bitmap(800,600); 2ALLEGRO_BITMAP* created2 = al_load_bitmap("hero.png"); 3 4ALLEGRO_BITMAP* shallow_reference1 = 0; 5 6shallow_reference1 = created1;// fine, does not create or destroy memory 7shallow_reference1 = created2;// fine, same thing 8 9// This next line would be bad : 10created1 = created2;// OOPS we just leaked the bitmap referenced by created1 11 12// Clean up when done 13al_destroy_bitmap(created1); 14al_destroy_bitmap(created2); 15 16// This would be bad : 17al_destroy_bitmap(created1);// Already did this, it will crash 18 19created2 = 0; 20al_destroy_bitmap(created2);// Could be fine, depends on what Allegro does with a null pointer.

shadyvillian
Member #12,426
December 2010

So how to do I make it not crash? I always destroy a bitmap when I assign it if its not null.

Software Engineer by day, hacker by night.

Arthur Kalliokoski
Second in Command
February 2005
avatar

I'm probably missing something here, but you don't want to destroy it until you're done displaying it or whatever.

“Throughout history, poverty is the normal condition of man. Advances which permit this norm to be exceeded — here and there, now and then — are the work of an extremely small minority, frequently despised, often condemned, and almost always opposed by all right-thinking people. Whenever this tiny minority is kept from creating, or (as sometimes happens) is driven out of a society, the people then slip back into abject poverty. This is known as "bad luck.”

― Robert A. Heinlein

shadyvillian
Member #12,426
December 2010

When I monitor my programs memory usage, everytime I reassign an ALLEGRO_BITMAP my memory usage goes up. So I destroy the bitmap before I reassign it, but sometimes it crashes and I don't know why.

Software Engineer by day, hacker by night.

Arthur Kalliokoski
Second in Command
February 2005
avatar

So I destroy the bitmap before I reassign it, but sometimes it crashes and I don't know why.

When you destroy a bitmap, the memory becomes available for other uses. When it doesn't crash, it hasn't been reused and overwritten to something else yet.

“Throughout history, poverty is the normal condition of man. Advances which permit this norm to be exceeded — here and there, now and then — are the work of an extremely small minority, frequently despised, often condemned, and almost always opposed by all right-thinking people. Whenever this tiny minority is kept from creating, or (as sometimes happens) is driven out of a society, the people then slip back into abject poverty. This is known as "bad luck.”

― Robert A. Heinlein

Edgar Reynaldo
Member #8,592
May 2007
avatar

Don't destroy the bitmaps until you are done using them, and only do it once. This crap about memory use going up when you assign a pointer is ridiculous. Assigning a pointer takes exactly zero memory to perform. Whatever you're using to measure your memory use is not accurate.

bamccaig
Member #7,536
July 2006
avatar

shadyvillian
Member #12,426
December 2010

I'm getting complete random crashes when I go to draw the image sometimes without an errors going off.

#SelectExpand
1struct MemoryStruct chunk; 2 ALLEGRO_FILE *file = NULL; 3 CURLcode error; 4 5 if(imgBuffer != NULL) 6 { 7 al_destroy_bitmap(imgBuffer); 8 } 9 10 imgBuffer = NULL; 11 12 chunk.memory = (char*)malloc(1); 13 chunk.size = 0; 14 15 curl_easy_setopt(curl, CURLOPT_USERPWD, "user:password"); 16 curl_easy_setopt(curl,CURLOPT_URL, url.c_str()); 17 curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteMemoryCallback); 18 curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&chunk); 19 error = curl_easy_perform(curl); 20 21 if(error != 0) 22 { 23 Framework::ShowMessageBox("Error", "Failed to download image", ALLEGRO_MESSAGEBOX_ERROR); 24 } 25 26 file = al_open_memfile(chunk.memory, chunk.size, "rw"); 27 28 if(file == NULL) 29 { 30 cout << "failed to open file" << endl; 31 } 32 33 imgBuffer = al_load_bitmap_f(file, ".jpg"); 34 35 if(imgBuffer == NULL) 36 { 37 cout << "failed to load bitmap" << endl; 38 } 39 40 al_fclose(file); 41 free(chunk.memory); 42 43 return imgBuffer;

Software Engineer by day, hacker by night.

bamccaig
Member #7,536
July 2006
avatar

I meant all of the code, or at least a complete program that reproduces the issue. :) The more I speculate on what is going on here the more wrong I become. :-/

shadyvillian
Member #12,426
December 2010

The program is like 5000 lines though.... I could attach it if you want to see it. I hate when errors like this happen because my memory isn't increasing anymore but the bitmap is becoming invalid or something and it randomly crashes on draw.

Software Engineer by day, hacker by night.

Schyfis
Member #9,752
May 2008
avatar

Is the program single-threaded?
Can we see the draw function?

Try searching for imgBuffer in all your code, and make sure you're not accidentally destroying it somewhere or setting it to NULL.

________________________________________________________________________________________________________
[freedwill.us]
[unTied Games]

shadyvillian
Member #12,426
December 2010

ImgBuffer isn't being drawn at all its just to hold the bitmap that al_load_bimtap_f loads because if I just return al_load_bitmap_f my memory usage goes up everytime(I think not using imgBuffer stops the crashing maybe). I'm just confused on what to do. If I used imgBuffer I get Random crashes if I don't i get a memory leak...

Software Engineer by day, hacker by night.

Neil Roy
Member #2,229
April 2002
avatar

When I use bitmaps with Allegro 5, I do the following:

#SelectExpand
1// Make sure you assign it NULL to initialize it first 2ALLEGRO_BITMAP *bitmap = NULL; 3 4bitmap= al_create_bitmap(800,600); 5if(!bitmap) { // always check return values 6 //failed, print error message and exit here 7} 8 9// now if I want to load something else into that bitmap 10// I will destroy it first and set it to NULL before using it again 11al_destroy_bitmap(bitmap); 12bitmap = NULL; 13 14bitmap = al_load_bitmap("myimage.png"); 15if(!bitmap) { 16 //failed, print error message and exit here 17}

Basically, don't assume, check all return values, set the initial value to NULL, if you destroy a bitmap, even if you plan to reuse it, reset it to NULL, otherwise if you check to see if it failed to load, and it contains the address of the last bitmap still, your check won't be accurate and you'll have problems. This way you know it is NULL unless it loads safely because that is what you set it to.

Edit: I am curious, how do you check for memory leaks? I use CodeBlocks with MinGW, never done that before but want to.

Don't destroy the bitmaps until you are done using them, and only do it once. This crap about memory use going up when you assign a pointer is ridiculous. Assigning a pointer takes exactly zero memory to perform. Whatever you're using to measure your memory use is not accurate.

It seems to me that if you assign a new address to a pointer without first freeing up the memory that it is currently pointing to, than you'll create a leak, I think this is what people are talking about.

bamccaig
Member #7,536
July 2006
avatar

ImgBuffer isn't being drawn at all its just to hold the bitmap that al_load_bimtap_f loads because if I just return al_load_bitmap_f my memory usage goes up everytime(I think not using imgBuffer stops the crashing maybe). I'm just confused on what to do. If I used imgBuffer I get Random crashes if I don't i get a memory leak...

OK, I see what you're doing now and I'd recommend against it. What you should be doing is passing ownership of that ALLEGRO_BITMAP object back to the caller. What that basically means is that the caller is now responsible for destroying that bitmap when it's done with it (unless it happens to also pass the responsibility along). In any case, your "constructor" or loader function should not be attempting to remember previously loaded bitmaps and release them like that. Do you understand?

NiteHackr said:

...otherwise if you check to see if it failed to load, and it contains the address of the last bitmap still, your check won't be accurate and you'll have problems.

Technically all of the create/load functions for BITMAPs return NULL when they fail so you will reset said pointers to NULL in those cases, but if somebody happens to come along and stick 20 lines of code between destroy and load then it might get lost in the clutter. Better safe than sorry, IMO. bam_free from my C utility library accepts a pointer to a pointer. It frees the memory and sets the pointer to zero for you. :)

shadyvillian
Member #12,426
December 2010

Ok I think I get it now. I think I fixed it by integrating the function into the Image class so it works with the bitmap directly instead of just making a bitmap returning it into the image loader function(that returned the bitmap into the load function then assigned the bitmap from the parameter into the classes bitmap). I gets pretty confusing when you try to make everything object oriented and organized sometimes I guess :-/

Software Engineer by day, hacker by night.

Luiji99
Member #12,254
September 2010

I find that things get simpler when you start orienting things in an object-like way.

Programming should be fun. That's why I hate Java.
http://www.entertainingsoftware.com/

Neil Roy
Member #2,229
April 2002
avatar

bamccaig said:

It frees the memory and sets the pointer to zero for you. :)

That was something I had considered as well. Having it check to see if it is NULL, and if not, free it first. I always immediately destroy any bitmaps when I am done with them and set them to NULL so it isn't an issue.

I've done this in my own code where I had a temporary bitmap I was using to load in sprite sheets then divide them up into arrays (to avoid 3D filtering problems on tiled games). I just destroy the bitmap and set it to NULL when I am done loading etc... for each set of sprites/tiles on my latest project.

I read that the Allegro functions return NULL, but, like you, I feel safer setting it myself. Just one little line of code for peace of mind. ;)

bamccaig
Member #7,536
July 2006
avatar

A better alternative for C++ is using smart pointers i.e., boost::shared_ptr<T>, which will handle the cleanup for you automatically. This is probably something that the OP should look into as well.

Neil Roy
Member #2,229
April 2002
avatar

That sounds nice. I'm not doing much in C++ yet myself, but I'll try and keep that in mind when I do.

Karadoc ~~
Member #2,749
September 2002
avatar

Do I have to call al_destroy_bitmap every time I do this: img1 = img2

If img1 and img2 are `ALLEGRO_BITMAP*`s, (which I'm guessing they are), then it's important to understand that you are not making a copy of the bitmap image. You are only making a copy of the pointer to the image.

eg.

#SelectExpand
1ALLEGRO_BITMAP* img1 = al_load_bitmap("myimage.png"); 2if (!img1) 3{ 4 fprintf(stderr, "failed to load myimage.png\n"); 5 exit(1); 6} 7 8// img1 is loaded and ready to use. 9 10ALLEGRO_BITMAP* img2 = img1; 11 12// img2 now points to the same bitmap as img1. So either of these can now be used 13 14al_destroy_bitmap(img1); 15 16// We have now destroyed the bitmap which both img1 and img2 are pointing to. 17// So img1 _and_ img2 both now point to garbage. 18 19al_draw_bitmap(img2, 0, 0, 0); // bad! 20 21// this will probably crash, because the image pointed to by img2 has been deleted.

-----------

Go to: