|
|
|
This thread is locked; no one can reply to it.
|
| al_destroy_display and cleaning up static memory |
|
Edgar Reynaldo
Major Reynaldo
May 2007
 |
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? 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? 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]). 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}
43/* Function: al_free
44 */
45#define al_free(p) \
46 (al_free_with_context((p), __LINE__, __FILE__, __func__))
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}
My Website! | EAGLE GUI Library Demos | My Deviant Art Gallery | Spiraloid Preview | A4 FontMaker | Skyline! (Missile Defense) Eagle and Allegro 5 binaries | Older Allegro 4 and 5 binaries | Allegro 5 compile guide |
|
Elias
Member #358
May 2000
|
Edgar Reynaldo said: 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. -- |
|
Edgar Reynaldo
Major Reynaldo
May 2007
|
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! My Website! | EAGLE GUI Library Demos | My Deviant Art Gallery | Spiraloid Preview | A4 FontMaker | Skyline! (Missile Defense) Eagle and Allegro 5 binaries | Older Allegro 4 and 5 binaries | Allegro 5 compile guide |
|
Elias
Member #358
May 2000
|
Edgar Reynaldo said: 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. -- |
|
Edgar Reynaldo
Major Reynaldo
May 2007
|
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. My Website! | EAGLE GUI Library Demos | My Deviant Art Gallery | Spiraloid Preview | A4 FontMaker | Skyline! (Missile Defense) Eagle and Allegro 5 binaries | Older Allegro 4 and 5 binaries | Allegro 5 compile guide |
|
|
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?