Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDL_RenderFillRect crash in specific multithreaded context #986

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

SDL_RenderFillRect crash in specific multithreaded context #986

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Labels
invalid

Comments

@SDLBugzilla
Copy link
Collaborator

SDLBugzilla commented Feb 10, 2021

This bug report was migrated from our old Bugzilla tracker.

Reported in version: HG 2.1
Reported for operating system, platform: Mac OS X 10.8, x86_64

Comments on the original bug report:

On 2013-07-28 11:18:42 +0000, Taiki wrote:

My program handle two windows into two different part of the programm.
One of them is quite multithreaded and, even with an extensive use of mutexes (before any access to the render variable) crash when executing SDL_RenderFillRect.

The context in which the bug happen is the following:

The program start and launch a couple of threads
One of them create a window then create several threads to work on different tasks.
When one of those threads needs to draw something, it first call SDL_RenderFillRect (the background color was set) and then, crash, throwing a EXC_BAD_ACCESS exception. The parent thread can still draw without any problem. The renderer variable is global.
I recompiled SDL with symbols and here is the stack of the crashed thread:
glColor4f
GL_SetColor
GL_SetDrawingState
GL_RenderFillRects
SDL_RenderFillRect

No other thread try to access to the renderer variable in the same time
(two threads are waiting, one is downloading using libcurl).
Some changes to the way the thread is started (if my subroutine have to return the pthread_t variable used to start the thread) make the crash happen later, in an other thread (started after the lines which previously crashed by the same thread).

This work like a charm on Windows 8.

SDL is compiled in 32bit, because some cryptographic algorithms of my program were broken if compiled in 64bit, and SDL couldn't be linked if compiled in 64bit.
It's compiled as a static library from sources on hg.
I disabled AUDIO, JOYSTICK and HAPTIC support on both the Windows and OSX version from headers, by adding

#define SDL_AUDIO_DISABLED 1
#define SDL_JOYSTICK_DISABLED 1
#define SDL_HAPTIC_DISABLED 1

and commenting defines before, may be related? But then why doesn't it crash on Windows?

On 2013-07-28 13:18:12 +0000, Taiki wrote:

The crash occure in glColor4f.
movl %gs:120 %eax change the value of %eax to an adress referenced afterward.
The problem seems to be that in the said context, %eax is set to 0x0.
My knowledges about OpenGL are way too low to dig futhermore but I can give you as much informations you want about the context...

On 2013-07-28 14:15:28 +0000, Sam Lantinga wrote:

OpenGL has specific rules about multi-threading. When you create a context, you make it active on the thread you want to use it. You can only make it active on one thread at a time. If you don't do this, you'll crash in the way you saw.

The SDL renderer is not designed to be multi-threaded and it takes significant work to change that. Most OpenGL drivers enable a whole bunch of locking internally if you start using a context on multiple threads which slows it down, so we haven't bothered.

Most people handle this by having the main thread be the graphics thread, and using other threads for simulation, loading and other background tasks that feed data to the main thread for processing.

On 2013-07-28 14:40:55 +0000, Taiki wrote:

Is there any way to use an other rendering engine on OSX?
I'd rather like not having to deal with OpenGL internal for one platform on a software which is not a game...
(using opengl seems a little bit an overkill)

On 2013-07-28 21:29:18 +0000, Sam Lantinga wrote:

If you can add a link to a test program (or your actual program) I can see how much work it would be to get it to work the way you expect.

On 2013-07-29 03:24:15 +0000, Taiki wrote:

The context is a couple thousands lines long on a closed source software so I don't think I can post too much code here...

Basically, this was the way I designed it:

Initial thread dedicated to this context launched: initialize stuffs, allocate memory and create window. Launch processing thread (T2) then start to handle events. It may have to print things if an event change context (eg: reorder stuffs).
T2 is an intermediary thread, it'll launch a tasks in an other thread (T3), print on the window what task is being processing and wait for it to be finished.
T3 process stuffs and eventually print its changes to the context on the window.

Each access was controlled using mutexes to prevent concurrency access to the GUI.

Worked perfectly on Windows (probably because DirectX was used instead of OpenGL), crashed miserably on OSX was T2 first tried to print something (and, sometimes T2 didn't crashed but then T3 did).

I'm fixing it using a UI-dedicated thread but I still have to fix a couple of problem related to queuing request.
I replaced every call to SDL primitives by functions which set a flag then trigger the UI thread to process its request.
Not being able to have something which just work is a bit a pain in the ass but well, I can understand it's OpenGL's fault...

On 2020-10-10 12:07:40 +0000, Stas Sergeev wrote:

Hi, will this problem be solved with a
Vulkan renderer? AFAIK only openGL have
such problem. I use software renderer in
SDL because of that, but it doesn't have
the scaling filters, so the quality is
very bad.

On 2020-10-12 16:24:43 +0000, Sam Lantinga wrote:

(In reply to Stas Sergeev from comment # 6)

Hi, will this problem be solved with a
Vulkan renderer? AFAIK only openGL have
such problem. I use software renderer in
SDL because of that, but it doesn't have
the scaling filters, so the quality is
very bad.

No, Vulkan has no multi-threading protection. You typically want to create your window and renderer on the main thread and use that to process events and make graphics calls, and then any other threads would send messages to the main thread to do any actual drawing.

On 2020-10-12 16:59:03 +0000, Stas Sergeev wrote:

I am not a Vulkan expert by any means, but at least this:
https://developer.nvidia.com/sites/default/files/akamai/gameworks/blog/munich/mschott_vulkan_multi_threading.pdf

seems to be saying that Vulkan is thread-friendly.
Is it just my misunderstanding?

On 2020-10-12 19:25:35 +0000, Sam Lantinga wrote:

Yes, to clarify, it's not like Direct3D where you just pass a multi-threaded flag to the initialization and you can start making calls on any thread. You have to specifically design your application and rendering systems with threading in mind and use the correct synchronization mechanisms to make that work. All of that code would be outside of SDL though, so you wouldn't mix using the SDL renderer if you are writing your own low level Vulkan rendering code.

On 2020-10-12 19:37:46 +0000, Stas Sergeev wrote:

(In reply to Sam Lantinga from comment # 9)

Yes, to clarify, it's not like Direct3D where you just pass a multi-threaded
flag to the initialization and you can start making calls on any thread. You
have to specifically design your application and rendering systems with
threading in mind and use the correct synchronization mechanisms to make
that work. All of that code would be outside of SDL though,

Yes, I know currently SDL only allows
you to deal with Vulkan on your own.
But is this forever?
I was kind of expecting the Vulkan render backend
in SDL one day, with all the locking/threading
API exported via some SDL wrappers. Especially since
Vulkan advertises their multi-threaded support that
much, and SDL have only software renderer thread-safe.

I won't suppose you can give me a lecture why
this isn't possible, but at least some brief
clarification or an URL where this was discussed,
would help.

@SDLBugzilla SDLBugzilla added bug invalid labels Feb 10, 2021
stefand added a commit to stefand/dosemu2 that referenced this issue Jun 17, 2021
This band-aid makes sdl.hwrend work with Nvidia.

It's just a dirty fix though. Dosemu2 is using SDL in a way it should not
be used - renderers are not thread safe:
libsdl-org/SDL#986

A better approach would be to create the SDL objects from the render
thread and only use them from there. That requires some restructuring
of the entire file; I tried 3 times and ran into dead ends. Maybe it needs
a restructuring of the larger video output system.

It also appears that dosemu is doing pixel format conversions (e.g.
palettized to 32 bit RGB) before calling into sdl.c. Ideally, we'd create
an SDL texture that matches the VGA mode (e.g. GL_RED8 for 8 bit palettes),
and do the conversion in a shader with a second texture for color lookups.

Beyond that, we should enable vsync and use it as a rate limit in the
renderer thread. Hypothetically we could then feed the return timings
from SDL_RenderPresent() to the application via VGA interrupts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid
Projects
None yet
Development

No branches or pull requests

1 participant