freeing memory
shadyvillian

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.

Thomas Fjellstrom

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.

shadyvillian

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

Edgar Reynaldo

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

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

Arthur Kalliokoski

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

shadyvillian

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.

Arthur Kalliokoski

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.

Edgar Reynaldo

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

Post the code.

shadyvillian

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;

bamccaig

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

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.

Schyfis

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.

shadyvillian

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...

Neil Roy

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

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

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 :-/

Luiji99

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

Neil Roy
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

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

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 ~~

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.

Thread #611122. Printed from Allegro.cc