Can't delete pointers of ALLEGRO

Can't delete pointers of ALLEGRO_BITMAP from std::map

Ojars_L

Member #15,861

January 2015

Created asset manager in order to keep all bitmaps in the std::map. All images are properly loaded and displayed onto the screen, but there is a warning in the output window, saying:

>d:\my documents\visual studio 2015\projects\PlatformGame\asset_manager.cpp(13): warning C4150: deletion of pointer to incomplete type 'ALLEGRO_BITMAP'; no destructor called
1> d:\programmas\allegro_2015\include\allegro5\mouse_cursor.h(37): note: see declaration of 'ALLEGRO_BITMAP'
>d:\my documents\visual studio 2015\projects\PlatformGame\asset_manager.cpp(43): warning C4150: deletion of pointer to incomplete type 'ALLEGRO_BITMAP'; no destructor called
1> d:\programmas\allegro_2015\include\allegro5\mouse_cursor.h(37): note: see declaration of 'ALLEGRO_BITMAP'

and when I close the application, I get 'Debug Assertion Failed!' window.

When I commented out code in the destructor and 'remove_tex' functions, warnings and 'Debug Assertion Failed!'window disappeared.

I can't understand why <allegro5\mouse_cursor.h> is involved there and where the code is wrong?

I'm working with msvs express 2015 for windows desktop and allegro-msvc2015-x86-5.1.13

AssetManager's header file:

#SelectExpand
  1#pragma once
  2#include <map>
  3
  4//textures
  5enum class Tex {
  6  Bat,
  7  Worm,
  8  Crawler,
  9  GnomeStay,
 10  GnomeClimb,
 11  GnomeHit,
 12  GnomeWalk,
 13  GnomeFly,
 14  GnomeFall
 15
 16};
 17
 18class AssetManager
 19{
 20  std::map<Tex, ALLEGRO_BITMAP *> textures;
 21    
 22  static AssetManager instance;
 23
 24public:
 25  ~AssetManager();
 26  static AssetManager &Instance();
 27  void load_bitmap(const Tex &texName, const char *filename);
 28  static ALLEGRO_BITMAP* get_image(const Tex &texName);
 29  void remove_tex(Tex tex);
 30};

AssetManager's .cpp file

#SelectExpand
  1#include <allegro5/allegro.h>
  2#include <cassert>
  3#include "asset_manager.h"
  4
  5
  6AssetManager AssetManager::instance;
  7
  8AssetManager::~AssetManager()
  9{
 10  std::map<Tex, ALLEGRO_BITMAP *>::iterator it;
 11  for (it = textures.begin(); it != textures.end(); it++)
 12  {
 13    delete it->second;
 14    it->second = nullptr;
 15  }
 16
 17  textures.clear();
 18  
 19}
 20
 21AssetManager &AssetManager::Instance()
 22{
 23  return instance;
 24}
 25
 26void AssetManager::load_bitmap(const Tex &texName, const char *filename)
 27{
 28  ALLEGRO_BITMAP *bmp;
 29  bmp = al_load_bitmap(filename);
 30  assert(bmp);
 31  al_convert_mask_to_alpha(bmp, al_map_rgb(255, 0, 255));
 32  textures[texName] = bmp;
 33}
 34
 35ALLEGRO_BITMAP* AssetManager::get_image(const Tex &texName)
 36{
 37  return instance.textures.at(texName);
 38}
 39
 40void AssetManager::remove_tex(Tex tex)
 41{
 42  std::map<Tex, ALLEGRO_BITMAP *>::iterator it = textures.find(tex);
 43  if (it != textures.end())
 44  {
 45    delete it->second;
 46    textures.erase(it);
 47  }
 48}

bamccaig

Member #7,536

July 2006

Instead of calling operator delete, which is supposed to correspond to operator new, you need to call al_destroy_bitmap() which will internally free() all of the malloc()-family allocations. In C you use malloc and family to allocate memory on the heap and free to release it. In C++ you use operator new to allocate an object (or primitive) on the heap in the same way and invoke its constructor and you use operator delete to call the destructor and release the memory. You cannot mix them up.

--
I mean the best with what I say. It doesn't always sound that way.

Ojars_L

Member #15,861

January 2015

Thank's for so quick answer. I feel foolish... to forget so simple things...
This, of course works, bat there is new problem: when I close application, there is an error window saying: game.exe has stopped working. In the "Wiew problem details": Fault Module Name: allegro_monolith-debug-5.1.dll.

With no AssetManager app closes fine.

Where could be the problem?

bamccaig

Member #7,536

July 2006

The problem is probably using a global instance and attempting to use the destructor to clean up the objects. Allegro installs an atexit handler that shuts Allegro down, and this is likely happening before C++ gets around to calling your destructor. So Allegro is crashing because it's no longer in a functioning state. You can try a few things:

Use al_install_system instead of al_init to initialize Allegro which gives you control over when the al_uninstall_system is called (but then you should make sure it gets called, even if exceptions are thrown, etc....).
Instantiate the AssetManager as a local variable in main and pass it around. (Or as a major hack, make it a local in main, but store a pointer to it in global space...)
Move the destructor business into a method that you can call early that will destroy the internal state and mark the asset manager "disposed" so it doesn't repeat this when the destructor is called (based on .NET's IDisposable interface). Call this method from the destructor, and then call this method before main exits to make sure it happens before Allegro is deinitialized. Example:

#SelectExpand
  1void AssetManager::dispose(void)  2{  3  if(this->disposed) return; 
  4  5  std::map<Tex, ALLEGRO_BITMAP*>::iterator it;  6  7for(it = textures.begin(); it != textures.end(); it++)  8{  9al_destroy_bitmap(it->second); 10    it->second = nullptr; 11} 12 13  textures.clear(); 14  this->disposed = true; 
 15}

To be safe you could guard the rest of the methods to refuse to do anything if the object is disposed. Or you could just document that using the object after it has been disposed is undefined/bad and don't do that.

--
I mean the best with what I say. It doesn't always sound that way.

Ojars_L

Member #15,861

January 2015

Thank you for advices.
I did put code from AssetManager's destructor to main function before exit and all is OK.