| Summary: | SDL_SetWindowFullscreen() vs rendering is not thread-safe | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Stas Sergeev <stsp2> |
| Component: | render | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | ||
| Version: | 2.0.7 | ||
| Hardware: | x86_64 | ||
| OS: | Linux | ||
| Attachments: | stack trace | ||
No, even the external mutex doesn't always help. I guess SDL_SetWindowFullscreen(window, SDL_WINDOW_FULLSCREEN_DESKTOP) works asynchronously with some resize events, so that rendering is unsafe for some period after the function return? It's not safe to render on a different thread than the one on which the window was created. You should always create the window and renderer on the same thread and make all rendering calls on that same thread. The only cross-platform way to use multiple threads is to create job threads that shuffle rendering commands to the main thread, or use Vulkan. > or use Vulkan.
But I am using SW renderer!
I know that opengl is not safe, and I am not
using it.
Please see the stack trace, it clearly shows
an SW renderer.
Oh interesting. I think because of the platform restriction nobody has gone to the effort of making the software renderer thread-safe in that way. I'll reopen this bug for investigation. Can you post the full stack trace for all threads? Created attachment 3183 [details]
stack trace
Yes, I can add the trace of all threads
(here it is) but it is unhelpful because
there is no thread that did full-screen
switch. I guess the full-screen switch
is kinda asynchronous (uses some X events),
so neither it is visible in the stack trace,
nor the external mutex helps much.
But please pay attention to the following
from the initial trace:
#8 SDL_FillRect4SSE (h=319, w=<optimized out>, color=0, pitch=5120,
pixels=0x7f4883670000 <error: Cannot access memory at address 0x7f4883670000>) at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/
To me this means that the rendering target
was moved, but we still have the old pointer.
A bit of an off-topic: are there any plans to support SDL_HINT_RENDER_SCALE_QUALITY for SW renderer? The code seems to be available in SDL_gfx, but the last time I looked, it was not trivially portable. Captured another very interesting stack trace:
---
#6 <signal handler called>
No locals.
#7 SDL_InvalidateMap (map=0x560f92218390)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/video/SDL_pixels.c:988
No locals.
#8 0x00007f68fd2e5b2d in SDL_FreeSurface_REAL (surface=0x560f92218330)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/video/SDL_surface.c:1300
No locals.
#9 0x00007f68fd2dfd9d in SDL_InvalidateMap (map=0x560f923a9970)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/video/SDL_pixels.c:989
No locals.
#10 0x00007f68fd2e5b2d in SDL_FreeSurface_REAL (
surface=0x7f68fdb9dcb0 <main_arena+144>)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/video/SDL_surface.c:1300
No locals.
#11 0x00007f68fd2dfd9d in SDL_InvalidateMap (
map=0x7f68fdb9dcc0 <main_arena+160>)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/video/SDL_pixels.c:989
No locals.
#12 0x00007f68fd2e5b2d in SDL_FreeSurface_REAL (
surface=0x7f68fdb9dc80 <main_arena+96>)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/video/SDL_surface.c:1300
No locals.
#13 0x00007f68fd2dfd9d in SDL_InvalidateMap (map=0x560f923a9be0)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/video/SDL_pixels.c:989
No locals.
#14 0x00007f68fd2e5b2d in SDL_FreeSurface_REAL (surface=0x560f922f9510)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/video/SDL_surface.c:1300
No locals.
#15 0x00007f68fd2e9446 in SDL_GetWindowSurface_REAL (window=0x560f91de3c30)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/video/SDL_video.c:2223
No locals.
#16 0x00007f68fd2acb1b in SW_ActivateRenderer (
renderer=renderer@entry=0x560f91f60150)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/render/software/SDL_render_sw.c:114
surface = <optimized out>
data = 0x560f91f45dd0
#17 0x00007f68fd2ad5dd in SW_RenderCopy (renderer=0x560f91f60150,
texture=0x560f922c0140, srcrect=0x7f68ea7fbe70, dstrect=0x7f68ea7fbe90)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/render/software/SDL_render_sw.c:561
surface = <optimized out>
src = <optimized out>
final_rect = {x = 0, y = 0, w = 0, h = 0}
#18 0x00007f68fd28d3a8 in SDL_RenderCopy_REAL (renderer=0x560f91f60150,
texture=<optimized out>, srcrect=<optimized out>, dstrect=0x0)
at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/render/SDL_render.c:1909
real_srcrect = {x = 0, y = 0, w = 720, h = 400}
real_dstrect = {x = 0, y = 0, w = 720, h = 400}
frect = {x = 0, y = 0, w = 1280, h = 711.111145}
#19 0x00007f68fd5b3cb7 in do_rend () at sdl.c:394
No locals.
---
The code confirms that SDL_FreeSurface() calls
SDL_InvalidateMap() which calls back to SDL_FreeSurface(),
and that recursion happens until map->dst->refcount>0.
I wonder if this is intentional.
When this recursion unrolls, it will likely produce
a multiple free in SDL_InvalidateMap(), which crashes.
(In reply to Stas Sergeev from comment #1) > No, even the external mutex doesn't always help. In fact, it does if I also lock SDL_PumpEvents(). Besides, this bug seems to have been fixed. I can't reproduce any crash now, even if I remove the mutex around SDL_PumpEvents() and SDL_SetWindowFullscreen(). Any idea what could fix it? |
If SDL_SetWindowFullscreen() is called while some thread does SDL_RenderCopy() or SDL_RenderClear(), the app will crash. The stack trace is: --- #6 <signal handler called> No locals. #7 _mm_stream_ps (__A=..., __P=0x7f4883670000) at /usr/lib/gcc/x86_64-redhat-linux/7/include/xmmintrin.h:1226 No locals. #8 SDL_FillRect4SSE (h=319, w=<optimized out>, color=0, pitch=5120, pixels=0x7f4883670000 <error: Cannot access memory at address 0x7f4883670000>) at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/ src/video/SDL_fillrect.c:128 adjust = <optimized out> i = <optimized out> c128 = <optimized out> n = 5120 p = 0x7f4883670000 <error: Cannot access memory at address 0x7f4883670000> #9 SDL_FillRect_REAL (dst=dst@entry=0x563d66a37eb0, rect=rect@entry=0x0, color=color@entry=0) at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/video/SDL_fillrect.c:313 clipped = {x = 0, y = 0, w = -243195437, h = 32584} pixels = <optimized out> color = 0 rect = 0x563d66a37ef0 dst = 0x563d66a37eb0 #10 0x00007f48f09adc4d in SW_RenderClear (renderer=<optimized out>) at /usr/src/debug/SDL2-2.0.7-2.fc27.x86_64/src/render/software/SDL_render_sw.c:404 surface = 0x563d66a37eb0 color = 0 clip_rect = {x = 0, y = 156, w = 1280, h = 712} --- Adding an external mutex helps. But I believe this is a wrong thing to do, because SDL_SetWindowFullscreen() operates on a window object and render API operates on a render object. Their internal deps should not be known to an app.