Rev. 6497 leads to (rare) random deadlocks on destroying the SDL_Renderer

Dear all,

on destroying an OpenGL-enabled SDL_Renderer, it seems that random
deadlocks might occur, if one is freeing SDL_Texture objects.

I cannot clearly reproduce that issue and within 100 runs, it might
occur once or twice. The issue does not occur, when freeing a SDL_Texture
happens as soon as possible.

On bulk-freeing man SDL_Texture objects and destroying the SDL_Renderer
right after that, the issue does not occur. This occurs for me since
rev. 6497 and I can not narrow it down except for some possibly invalid
texture pointer being passed to CHECK_TEXTURE_MAGIC(), which seems to
enter a dead lock on setting the SDL error:

Find below three backtraces from three successfully failing runs:

SDL_GetErrBuf () at /home/marcus/devel/projects/SDL/src/thread/SDL_thread.c:176
176 return (errbuf);
(gdb) bt
#0 SDL_GetErrBuf ()
at /home/marcus/devel/projects/SDL/src/thread/SDL_thread.c:176
#1 0x000000080427f093 in SDL_SetError (fmt=0x80436baa3 “Invalid texture”)
at /home/marcus/devel/projects/SDL/src/SDL_error.c:62
#2 0x00000008042c4b47 in SDL_DestroyTexture (texture=0x808fbb400)
at /home/marcus/devel/projects/SDL/src/render/SDL_render.c:1344
#3 0x00000008042c4c8b in SDL_DestroyRenderer (renderer=0x808d8e100)
at /home/marcus/devel/projects/SDL/src/render/SDL_render.c:1380

0x000000080427f7be in SDL_GetErrorMsg (errstr=0x80458bf00 “”, maxlen=1023)
at /home/marcus/devel/projects/SDL/src/SDL_error.c:139
139 while (*fmt && (maxlen > 0)) {
(gdb) bt
#0 0x000000080427f7be in SDL_GetErrorMsg (errstr=0x80458bf00 “”, maxlen=1023)
at /home/marcus/devel/projects/SDL/src/SDL_error.c:139
#1 0x000000080427f7f5 in SDL_GetError ()
at /home/marcus/devel/projects/SDL/src/SDL_error.c:206
#2 0x000000080427f512 in SDL_SetError (fmt=0x80436bab2 “”)
at /home/marcus/devel/projects/SDL/src/SDL_error.c:114
#3 0x00000008042c4b47 in SDL_DestroyTexture (texture=0x808fbb400)
at /home/marcus/devel/projects/SDL/src/render/SDL_render.c:1344
#4 0x00000008042c4c8b in SDL_DestroyRenderer (renderer=0x808d8e100)
at /home/marcus/devel/projects/SDL/src/render/SDL_render.c:1380

0x000000080427f7b9 in SDL_GetErrorMsg (errstr=0x80458bf00 “Invalid texture”,
maxlen=1021) at /home/marcus/devel/projects/SDL/src/SDL_error.c:139
139 while (*fmt && (maxlen > 0)) {
(gdb) bt
#0 0x000000080427f7b9 in SDL_GetErrorMsg (
errstr=0x80458bf00 “Invalid texture”, maxlen=1021)
at /home/marcus/devel/projects/SDL/src/SDL_error.c:139
#1 0x000000080427f7f5 in SDL_GetError ()
at /home/marcus/devel/projects/SDL/src/SDL_error.c:206
#2 0x000000080427f512 in SDL_SetError (fmt=0x80436bab2 “”)
at /home/marcus/devel/projects/SDL/src/SDL_error.c:114
#3 0x00000008042c4b47 in SDL_DestroyTexture (texture=0x808fbb400)
at /home/marcus/devel/projects/SDL/src/render/SDL_render.c:1344
#4 0x00000008042c4c8b in SDL_DestroyRenderer (renderer=0x808d8e100)
at /home/marcus/devel/projects/SDL/src/render/SDL_render.c:1380

Attached is a program, with which I can reproduce that issue from time
to time.

Cheers
Marcus
-------------- next part --------------
A non-text attachment was scrubbed…
Name: main.c
Type: text/x-csrc
Size: 1019 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20121001/08c25119/attachment.c
-------------- next part --------------
A non-text attachment was scrubbed…
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20121001/08c25119/attachment.pgp

Quoth Marcus von Appen , on 2012-10-01 21:27:17 +0200:

Attached is a program, with which I can reproduce that issue from time
to time.
[…]
int cnt = 0;
for (int w = 1; w <= maxw; w++)
{
for (int h = 1; h <= maxh; h++)
{
textures[cnt] = SDL_CreateTexture(renderer,
SDL_PIXELFORMAT_RGBA8888,
SDL_TEXTUREACCESS_STATIC,
w, h);
[…]
}
}

Erm, isn’t this storing all the pointers into textures[0] if you don’t
increment cnt anywhere, then trying to free the last texture plus a
bunch of garbage? Can you still reproduce the problem with (h++,
cnt++) in the inner loop?

—> Drake Wilson

On, Mon Oct 01, 2012, Drake Wilson wrote:

Quoth Marcus von Appen <@Marcus_von_Appen>, on 2012-10-01 21:27:17 +0200:

Attached is a program, with which I can reproduce that issue from time
to time.
[…]
int cnt = 0;
for (int w = 1; w <= maxw; w++)
{
for (int h = 1; h <= maxh; h++)
{
textures[cnt] = SDL_CreateTexture(renderer,
SDL_PIXELFORMAT_RGBA8888,
SDL_TEXTUREACCESS_STATIC,
w, h);
[…]
}
}

Erm, isn’t this storing all the pointers into textures[0] if you don’t
increment cnt anywhere, then trying to free the last texture plus a
bunch of garbage? Can you still reproduce the problem with (h++,
cnt++) in the inner loop?

Meh, that happens on hacking together an example without checking it
over :-/. Of course there should be a cnt++ somewhere after
textures[cnt] = …

The problem however still exists, even with the cnt++ ;-).

Cheers
Marcus
-------------- next part --------------
A non-text attachment was scrubbed…
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20121001/fb11f5c5/attachment.pgp

Quoth Marcus von Appen , on 2012-10-01 21:55:48 +0200:

Meh, that happens on hacking together an example without checking it
over :-/. Of course there should be a cnt++ somewhere after
textures[cnt] = …

The problem however still exists, even with the cnt++ ;-).

I’m making an untested guess here, but at a glance it looks like
r6497 can corrupt a doubly-linked list in the case of alternate-format
backing textures. Does this do any good?

— a/src/render/SDL_render.c Mon Oct 01 00:56:58 2012 -0700
+++ b/src/render/SDL_render.c Mon Oct 01 16:40:57 2012 -0500
@@ -395,7 +395,13 @@

     /* Swap textures to have texture before texture->native in the list */
     texture->native->next = texture->next;
  •    if (texture->native->next) {
    
  •        texture->native->next->prev = texture->native;
    
  •    }
       texture->prev = texture->native->prev;
    
  •    if (texture->prev) {
    
  •        texture->prev->next = texture;
    
  •    }
       texture->native->prev = texture;
       texture->next = texture->native;
       renderer->textures = texture;
    

Cheers
Marcus

—> Drake Wilson

On, Mon Oct 01, 2012, Drake Wilson wrote:

Quoth Marcus von Appen <@Marcus_von_Appen>, on 2012-10-01 21:55:48 +0200:

Meh, that happens on hacking together an example without checking it
over :-/. Of course there should be a cnt++ somewhere after
textures[cnt] = …

The problem however still exists, even with the cnt++ ;-).

I’m making an untested guess here, but at a glance it looks like
r6497 can corrupt a doubly-linked list in the case of alternate-format
backing textures. Does this do any good?

[… list patch …]

Yes, after stress-testing and inspecting the list chains without the
patch, it seems that sometimes, if the textures are not freed in the
correct order (might hit quite often in threaded environments with a GC
attached, which destructs a container encapsulating the SDL_Texture),
the list contains an invalid pointer.

With your patch applied, I could not reproduce that issue anymore.

Cheers
Marcus
-------------- next part --------------
A non-text attachment was scrubbed…
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
URL: http://lists.libsdl.org/pipermail/sdl-libsdl.org/attachments/20121002/4eb06b04/attachment.pgp

Patch applied, thanks!On Mon, Oct 1, 2012 at 2:41 PM, Drake Wilson wrote:

Quoth Marcus von Appen , on 2012-10-01 21:55:48 +0200:

Meh, that happens on hacking together an example without checking it
over :-/. Of course there should be a cnt++ somewhere after
textures[cnt] = …

The problem however still exists, even with the cnt++ ;-).

I’m making an untested guess here, but at a glance it looks like
r6497 can corrupt a doubly-linked list in the case of alternate-format
backing textures. Does this do any good?

— a/src/render/SDL_render.c Mon Oct 01 00:56:58 2012 -0700
+++ b/src/render/SDL_render.c Mon Oct 01 16:40:57 2012 -0500
@@ -395,7 +395,13 @@

     /* Swap textures to have texture before texture->native in the

list */
texture->native->next = texture->next;

  •    if (texture->native->next) {
    
  •        texture->native->next->prev = texture->native;
    
  •    }
       texture->prev = texture->native->prev;
    
  •    if (texture->prev) {
    
  •        texture->prev->next = texture;
    
  •    }
       texture->native->prev = texture;
       texture->next = texture->native;
       renderer->textures = texture;
    

Cheers
Marcus

—> Drake Wilson


SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org