Allegro.cc - Online Community

Allegro.cc Forums » Allegro Development » al_destroy_display and cleaning up static memory

This thread is locked; no one can reply to it. rss feed Print
al_destroy_display and cleaning up static memory
Edgar Reynaldo
Member #8,592
May 2007
avatar

I've been spelunking[1] in allegro internals for a few hours now trying to debug my ManyMouse program. I finally figured out I'm accessing a display that has been destroyed already by means of al_get_current_display returning a destroyed display. Shouldn't the thread local state (tls->current_display) be reset (zeroed) when a display is destroyed? Or set to another display if one is available?

src/tls.c#SelectExpand
378/* Function: al_get_current_display 379 */ 380ALLEGRO_DISPLAY *al_get_current_display(void) 381{ 382 thread_local_state *tls; 383 384 if ((tls = tls_get()) == NULL) 385 return NULL; 386 return tls->current_display; 387}

Also, al_destroy_display does nothing to zero the memory of the display in use, so it just contains leftover garbage when you destroy it. Doesn't proper practice dictate zeroing memory when you aren't going to use it anymore? Or setting it to some kind of guard value?

display.c#SelectExpand
139/* Function: al_destroy_display 140 */ 141void al_destroy_display(ALLEGRO_DISPLAY *display) 142{ 143 if (display) { 144 /* This causes warnings and potential errors on Android because 145 * it clears the context and Android needs this thread to have 146 * the context bound in its destroy function and to destroy the 147 * shader. Just skip this part on Android. 148 */ 149#ifndef ALLEGRO_ANDROID 150 ALLEGRO_BITMAP *bmp; 151 152 bmp = al_get_target_bitmap(); 153 if (bmp && _al_get_bitmap_display(bmp) == display) 154 al_set_target_bitmap(NULL); 155 156 /* This can happen if we have a current display, but the target bitmap 157 * was a memory bitmap. 158 */ 159 if (display == al_get_current_display()) 160 _al_set_current_display_only(NULL); 161#endif 162 163 al_destroy_shader(display->default_shader); 164 display->default_shader = NULL; 165 166 ASSERT(display->vt); 167 display->vt->destroy_display(display); 168 } 169}

I'm using Windows and D3D at the moment, so display->vt->destroy_display is currently set to d3d_destroy_display. However, free doesn't seem to be setting any guard pattern with MinGW 4.8.1 when free(display) is called (free is being called because the ALLEGRO_MEMORY_INTERFACE* mem is zero [because allegro never calls al_set_memory_interface anywhere, it is unused]).

memory.c#SelectExpand
49/* Function: al_free_with_context 50 */ 51void al_free_with_context(void *ptr, 52 int line, const char *file, const char *func) 53{ 54 if (mem) 55 mem->mi_free(ptr, line, file, func); 56 else 57 free(ptr); 58}

memory.h#SelectExpand
43/* Function: al_free 44 */ 45#define al_free(p) \ 46 (al_free_with_context((p), __LINE__, __FILE__, __func__))

d3d_disp.cpp#SelectExpand
893static void d3d_destroy_display(ALLEGRO_DISPLAY *display) 894{ 895 ALLEGRO_SYSTEM_WIN *system = (ALLEGRO_SYSTEM_WIN *)al_get_system_driver(); 896 ALLEGRO_DISPLAY_D3D *d3d_display = (ALLEGRO_DISPLAY_D3D *)display; 897 ALLEGRO_DISPLAY *old_disp = al_get_current_display(); 898 899 ALLEGRO_INFO("destroying display %p (current %p)\n", display, old_disp); 900 901 if (old_disp != display) 902 _al_set_current_display_only(display); 903 904 if (system->mouse_grab_display == display) 905 al_ungrab_mouse(); 906 907 _al_win_destroy_display_icons(display); 908 909 d3d_destroy_display_internals(d3d_display); 910 911 _al_vector_free(&display->display_invalidated_callbacks); 912 _al_vector_free(&display->display_validated_callbacks); 913 914 _al_vector_find_and_delete(&system->system.displays, &display); 915 916 if (system->system.displays._size <= 0) { 917 ffw_set = false; 918 already_fullscreen = false; 919 } 920 921 if (d3d_display->es_inited) { 922 _al_event_source_free(&display->es); 923 d3d_display->es_inited = false; 924 } 925 926 _al_vector_free(&display->bitmaps); 927 _al_vector_free(&((ALLEGRO_DISPLAY_WIN*) display)->msg_callbacks); 928 929 if (old_disp != display) 930 _al_set_current_display_only(old_disp); 931 932 al_free(display->vertex_cache); 933 al_free(display); 934}

Elias
Member #358
May 2000

Shouldn't the thread local state (tls->current_display) be reset (zeroed) when a display is destroyed? Or set to another display if one is available?

I think setting it to zero is what we should do.

But I already see this, so somehow you must manage to bypass that...

https://github.com/liballeg/allegro5/blob/5.1/src/win/d3d_disp.cpp#L2416

Quote:

Doesn't proper practice dictate zeroing memory when you aren't going to use it anymore?

No - the memory may be reclaimed by the next memory allocation in fact and so zeroing out would not do anything. MSVC I believe has a debug setting by which it writes a pattern into free memory and does not re-use it. valgrind does the same in Linux and it's very useful.

And you can always use al_set_memory_interface to do exactly that on your own, it's why that function exists.

--
"Either help out or stop whining" - Evert

Edgar Reynaldo
Member #8,592
May 2007
avatar

Elias said:

I think setting it to zero is what we should do.

But I already see this, so somehow you must manage to bypass that...

https://github.com/liballeg/allegro5/blob/5.1/src/win/d3d_disp.cpp#L2416

The link you gave references _al_d3d_destroy_bitmap. Perhaps you meant something else?

Elias said:

I said:

Doesn't proper practice dictate zeroing memory when you aren't going to use it anymore?

No - the memory may be reclaimed by the next memory allocation in fact and so zeroing out would not do anything. MSVC I believe has a debug setting by which it writes a pattern into free memory and does not re-use it. valgrind does the same in Linux and it's very useful.

And you can always use al_set_memory_interface to do exactly that on your own, it's why that function exists.

Oh, okay!! That's what that is for! :o I tried looking up bit patterns used by MinGW but I couldn't find anything on Google. I thought MinGW would fill memory freed by free with some special bit pattern. I see 0xffffffffbaadfood in places (uninitialized allocated memory). How would I 'not re-use' memory? Would I have to make my own memory pool and divvy that up when my malloc is called?

Elias
Member #358
May 2000

The link you gave references _al_d3d_destroy_bitmap. Perhaps you meant something else?

Oh, yes, I thought the target bitmap is invalid - didn't realize we're talking about the display itself getting destroyed. So how can the current display end up pointing to a destroyed one? Only way I can see is if two threads access it at the same time - which is just not supported.

--
"Either help out or stop whining" - Evert

Edgar Reynaldo
Member #8,592
May 2007
avatar

You may be right, but I'm not sure. I have two displays created on the main thread, and then all the mouse displays are created on the main display's window process thread that allegro creates in the window callback. Each mouse handles its own drawing on its window process thread that allegro created for the display in its own window process callback function. Mice are told to draw from the main displays callback process through PostMessage, which sends a message to the mouse's HWND, and drawing is taken care of by drawing from a DIB to the HWND of the mouse.

I really don't know what is causing the crash, and like I said I can only debug it on Vista. I'll try to get more info tonight, and see whether it is the backbuffer that is the bitmap that has been destroyed like you asked.

Go to: