It just took me ages to work out that my game was crashing because I hadn't called al_destroy_user_event_source before exiting. I found this kind of surprising because my rule of thumb is that destroy only needs to be called on things that allegro actually creates (such as al_create_event_queue). I figured that since allegro only initialised the event source, it didn't need to be destroyed.

Anyway I was apparently wrong about that. Neglecting to call al_destroy_user_event_source before the end of the program resulted in a potential crash. (It doesn't crash all programs.) So I was wondering, is there anything else I need to know about this? For example, if I register a user event source with an event queue, do I need to unregister it before destroying the queue? or before destroying the event_source? Does it matter which order I destroy the event sources and the event queue? I don't want any more nasty surprises...

In my tests I was making it crash without registering it in the first place.

Here's the simplest program I came up with which produced a crash:

To compile, I used "g++ -Wall crash.cpp -lallegro" (g++ is mingw in this case)
As you can see from the comments, the program needs some level of complexity for it to crash (it doesn't crash without that class being created). And when it does crash, it crashes on exit.

This is all you need to trigger the crash:

int main(void)
{
  al_init();
  ALLEGRO_EVENT_SOURCE source2;
  al_init_user_event_source(&source2);
  return 0;
}

_al_event_source_free (es=0x7fffffffe1e0) at 4.9/src/evtsrc.c:62
62	      al_unregister_event_source(*slot, es);

Seems like a bug if it's trying to unregister an event source when none were ever registered.

Edit: Actually, on second thought, it's not really a bug. It's more of a limitation imposed by the global destructors. The source2 is going out of scope before the global destructor is being called, which is likely leading to garbage data.

But I'm not sure that Allegro should register a destructor when al_init_user_event_source is called. It seems that it would be very hard for that behavior to actually ever work properly.

It doesn't matter.

I'm saying Allegro probably shouldn't even register the one it currently is registering.

Even if it's not on the stack, it will probably look like:

struct MY_OBJECT
{
  ALLEGRO_EVENT_SOURCE foo;
  int bar;
}

MY_OBJECT *obj = malloc(sizeof *obj);
al_init_user_event_source(&obj->foo);
free(obj);
return 0;

So in that case, you'd have to manually destroy the event source as well.

It seems simpler to remove the destructor and just make the programmer explicitly call it in every case.

"statically on the stack"? You can't have your cake and eat it too

I don't like manually releasing events -- that's not Allegro style.

What's the argument against the old style?

ALLEGRO_EVENT_SOURCE *source2 = al_init_user_event_source();

Well technically there is only one atexit destructor registered. But I suppose you mean the code in that destructor that goes and cleans up user event sources.

al_init_user_event_source() dynamically allocates some memory, so you'd get a memory leak if it's not destroyed.

The changes I would make to Allegro are:

• Don't automatically destroy user event sources that were created via init(). Make it the programmer's responsibility to free it up, since he most likely is responsible for the memory there.

  
  

• Document the init() function accordingly that the memory must explicitly be destroyed when you are done with it.

It's not only a memory leak. If you free the memory for a user event source without unregistering it from all event queues then you are left with dangling pointers.

I agree with Matthew's changes.