libsdl-org / SDL-1.2

Simple Directmedia Layer, 1.2 branch ... ***DEPRECATED***, please use https://github.com/libsdl-org/SDL for new projects!
https://libsdl.org
GNU Lesser General Public License v2.1
104 stars 85 forks source link

SDL 1.2 programs crash with certain libGL implementations #753

Closed SDLBugzilla closed 3 years ago

SDLBugzilla commented 3 years ago

This bug report was migrated from our old Bugzilla tracker.

Reported in version: 1.2.15 Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2015-01-12 12:02:20 +0000, Petr Pisar wrote:

This is a bug report regarding not calling XInitThreads(). Especially if underlying OpenGL implementation uses threads. It's copied from a Fedora bug report https://bugzilla.redhat.com/show_bug.cgi?id=1181085:

Some libGL implementations call XInitThreads() on dlopen. For example, the latest nVidia proprietary implementation does it, probably because it needs to use threads for performance. This causes applications that use SDL 1.2 to crash.

From XInitThreads(3):

The XInitThreads function initializes Xlib support for concurrent threads. This function must be the first Xlib function a multi-threaded program calls, and it must complete before any other Xlib call is made.

From https://wiki.libsdl.org/SDL_GL_LoadLibrary:

This should be done after initializing the video driver, but before creating any OpenGL windows. If no OpenGL library is loaded, the default library will be loaded upon creation of the first OpenGL window.

But SDL_VideoInit() calls XOpenDisplay() and nVidia libGL implementation calls XInitThreads() on dlopen:

  (gdb) break XInitThreads
  Function "XInitThreads" not defined.
  Make breakpoint pending on future shared library load? (y or [n]) y
  Breakpoint 1 (XInitThreads) pending.
  (gdb) run
  Starting program: /mnt/data/home/woky/src/xonotic/darkplaces/darkplaces-sdl 
  [Thread debugging using libthread_db enabled]
  Using host libthread_db library "/lib64/libthread_db.so.1".
  Game is DarkPlaces-Quake using base gamedir id1
  gamename for server filtering: DarkPlaces-Quake
  DarkPlaces-Quake Linux 18:50:26 Jan 11 2015 - debug
  Current nice level is below the soft limit - cannot use niceness
  Playing shareware version.
  Skeletal animation uses SSE code path
  DPSOFTRAST available (SSE2 instructions detected)
  Failed to init SDL joystick subsystem: 
  couldn't exec quake.rc
  couldn't exec default.cfg
  execing config.cfg
  couldn't exec autoexec.cfg
  Client using an automatically assigned port
  Client opened a socket on address 0.0.0.0:0
  Linked against SDL version 1.2.15
  Using SDL library version 1.2.15

  Breakpoint 1, XInitThreads () at locking.c:573
  573 {
  (gdb) thread apply all bt

  Thread 1 (Thread 0x7ffff7fcc700 (LWP 8884)):
  # 0  XInitThreads () at locking.c:573
  # 1  0x00007fffea173136 in ?? () from /usr/lib64/nvidia/libGL.so.1
  # 2  0x00007fffea1956cd in ?? () from /usr/lib64/nvidia/libGL.so.1
  # 3  0x00007fffea173b9d in ?? () from /usr/lib64/nvidia/libGL.so.1
  # 4  0x00007ffff7deaf0d in call_init (l=0x2923d80, argc=argc@entry=1, argv=argv@entry=0x7fffffffe038, env=env@entry=0x7fffffffe048) at dl-init.c:62
  # 5  0x00007ffff7deb05b in call_init (env=<optimized out>, argv=<optimized out>, argc=<optimized out>, l=<optimized out>) at dl-init.c:34
  # 6  _dl_init (main_map=main_map@entry=0x2923d80, argc=1, argv=0x7fffffffe038, env=0x7fffffffe048) at dl-init.c:124
  # 7  0x00007ffff7defa51 in dl_open_worker (a=a@entry=0x7fffffffcdc8) at dl-open.c:566
  # 8  0x00007ffff7deadf4 in _dl_catch_error (objname=objname@entry=0x7fffffffcdb8, errstring=errstring@entry=0x7fffffffcdc0, mallocedp=mallocedp@entry=0x7fffffffcdb7, 
          operate=operate@entry=0x7ffff7def590 <dl_open_worker>, args=args@entry=0x7fffffffcdc8) at dl-error.c:187
  # 9  0x00007ffff7deeee3 in _dl_open (file=0x7ffff701a7b6 "libGL.so.1", mode=-2147483390, caller_dlopen=0x7ffff6ff968b <X11_GL_LoadLibrary+59>, nsid=-2, argc=<optimized out>, 
          argv=<optimized out>, env=0x7fffffffe048) at dl-open.c:650
  # 10 0x00007ffff725c039 in dlopen_doit (a=a@entry=0x7fffffffcfe0) at dlopen.c:66
  # 11 0x00007ffff7deadf4 in _dl_catch_error (objname=0x292d090, errstring=0x292d098, mallocedp=0x292d088, operate=0x7ffff725bfe0 <dlopen_doit>, args=0x7fffffffcfe0) at dl-error.c:187
  # 12 0x00007ffff725c69d in _dlerror_run (operate=operate@entry=0x7ffff725bfe0 <dlopen_doit>, args=args@entry=0x7fffffffcfe0) at dlerror.c:163
  # 13 0x00007ffff725c0d1 in __dlopen (file=file@entry=0x7ffff701a7b6 "libGL.so.1", mode=mode@entry=258) at dlopen.c:87
  # 14 0x00007ffff6ff968b in X11_GL_LoadLibrary (this=0x2975760, path=0x7ffff701a7b6 "libGL.so.1") at src/video/x11/SDL_x11gl.c:506
  # 15 0x0000000000409616 in VID_InitModeGL (mode=0x7fffffffd4e0) at ../../../vid_sdl.c:2461
  # 16 0x0000000000409cdf in VID_InitMode (mode=0x7fffffffd4e0) at ../../../vid_sdl.c:2758
  # 17 0x00000000006fdbfd in VID_Mode (fullscreen=1, width=640, height=480, bpp=32, refreshrate=60, stereobuffer=0, samples=1) at ../../../vid_shared.c:1857
  # 18 0x00000000006fe4e5 in VID_Start () at ../../../vid_shared.c:2022
  # 19 0x000000000057400c in Host_StartVideo () at ../../../host.c:1088
  # 20 0x000000000047ad16 in SCR_BeginLoadingPlaque (startup=true) at ../../../cl_screen.c:761
  # 21 0x000000000057454b in Host_Init () at ../../../host.c:1324
  # 22 0x0000000000572d99 in Host_Main () at ../../../host.c:679
  # 23 0x000000000040444e in main (argc=1, argv=0x7fffffffe038) at ../../../sys_sdl.c:223

So if a program is written according to the SDL docs and runs with nVidia libGL implementation, the XInitThreads() is called too late from libGL and bad things happen[1]. This problem doesn't occur with SDL 2.0.3 because it calls XInitThreads() during video initialization.

I experience this problem for about 3-6 months now so it's probably caused by change in nVidia proprietary libGL implementation. However, it breaks some packages already present in Fedora which use SDL 1.2. Xonotic is affected for example. The crash happens during closing of the display where libX11 tries to release mutex which is NULL (or some random memory, see [1]).

SDL 1.2 should be patched to explicitly call XInitThreads in SDL_VideoInit() to avoid such crashes under arbitrary libGL implementations.

I wouldn't be able to figure this out without the insight of Xonotic core team wizards, especially divVerent. Thank you.

[1] http://forums.fedoraforum.org/showthread.php?t=302471

Version-Release number of selected component (if applicable): SDL-1.2.15-17.fc21.x86_64

Steps to Reproduce:

  1. sudo dnf install xonotic
  2. xonotic-sdl
  3. # from game UI, either exit the game or change the resolution

Actual results: Game segfaults.

Expected results: Game doesn't segfault.

On 2015-02-01 10:29:56 +0000, wrote:

Whoops. Found the problem. Some time ago, out of curiosity, I put this into my profile.

export __GL_THREADED_OPTIMIZATIONS=1

Removing it or setting it to 0 fixes the problem for me. Sorry for the noise.

On 2015-08-25 09:38:21 +0000, Ryan C. Gordon wrote:

Hello, and sorry if you're getting several copies of this message by email, since we are closing many bugs at once here.

We have decided to mark all SDL 1.2-related bugs as RESOLVED ENDOFLIFE, as we don't intend to work on SDL 1.2 any further, but didn't want to mark a large quantity of bugs as RESOLVED WONTFIX, to clearly show what was left unattended to and make it easily searchable.

Our current focus is on SDL 2.0.

If you are still having problems with an ENDOFLIFE bug, your absolute best option is to move your program to SDL2, as it will likely fix the problem by default, and give you access to modern platforms and tons of super-cool new features.

Failing that, we will accept small patches to fix these issues, and put them in revision control, although we do not intend to do any further official 1.2 releases.

Failing that, please feel free to contact me directly by email (icculus@icculus.org) and we'll try to find some way to help you out of your situation.

Thank you, --ryan.

sezero commented 3 years ago

SDL2 handled this by calling XInitThreads() as soon as SDL_X11_LoadSymbols succeeds: https://github.com/SDL-mirror/SDL/commit/635ebd967b7a8141e1022ed5c6b462df906f5c3a

sezero commented 2 years ago

@icculus, @slouken: I'd like to apply following XInitThreads patch here: Without it, I can't run tucnak under valgring for debugging. (i.e. for https://github.com/libsdl-org/sdl12-compat/issues/182)

video/x11/SDL_x11video.c: add XInitThreads to X11_CreateDevice.

Fixes: https://bugzilla.libsdl.org/show_bug.cgi?id=2843
       https://github.com/libsdl-org/SDL-1.2/issues/753
Also see SDL2 commit:
https://github.com/libsdl-org/SDL/commit/635ebd967b7a8141e1022ed5c6b462df906f5c3a

diff --git a/src/video/x11/SDL_x11sym.h b/src/video/x11/SDL_x11sym.h
index bd83f7f..bc54b96 100644
--- a/src/video/x11/SDL_x11sym.h
+++ b/src/video/x11/SDL_x11sym.h
@@ -76,6 +76,7 @@ SDL_X11_SYM(int,XMoveResizeWindow,(Display* a,Window b,int c,int d,unsigned int
 SDL_X11_SYM(int,XMoveWindow,(Display* a,Window b,int c,int d),(a,b,c,d),return)
 SDL_X11_SYM(int,XNextEvent,(Display* a,XEvent* b),(a,b),return)
 SDL_X11_SYM(Display*,XOpenDisplay,(_Xconst char* a),(a),return)
+SDL_X11_SYM(Status,XInitThreads,(void),(),return)
 SDL_X11_SYM(int,XPeekEvent,(Display* a,XEvent* b),(a,b),return)
 SDL_X11_SYM(int,XPending,(Display* a),(a),return)
 SDL_X11_SYM(int,XPutImage,(Display* a,Drawable b,GC c,XImage* d,int e,int f,int g,int h,unsigned int i,unsigned int j),(a,b,c,d,e,f,g,h,i,j),return)
diff --git a/src/video/x11/SDL_x11video.c b/src/video/x11/SDL_x11video.c
index bbf0d5d..b4f9f1b 100644
--- a/src/video/x11/SDL_x11video.c
+++ b/src/video/x11/SDL_x11video.c
@@ -103,6 +103,7 @@ static SDL_VideoDevice *X11_CreateDevice(int devindex)
    SDL_VideoDevice *device = NULL;

    if ( SDL_X11_LoadSymbols() ) {
+       XInitThreads();
        /* Initialize all variables that we clean on shutdown */
        device = (SDL_VideoDevice *)SDL_malloc(sizeof(SDL_VideoDevice));
        if ( device ) {
sezero commented 2 years ago

FWIW, here is the failure, which doesn't happen with the patch applied:

==5380== Memcheck, a memory error detector
==5380== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==5380== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==5380== Command: ./tucnak
==5380== 
[xcb] Unknown sequence number while processing queue
[xcb] Most likely this is a multi-threaded client and XInitThreads has not been called
[xcb] Aborting, sorry about that.
tucnak: xcb_io.c:259: poll_for_event: Assertion `!xcb_xlib_threads_sequence_lost' failed.
==5380== 
icculus commented 2 years ago

Go ahead and do it.

sezero commented 2 years ago

Go ahead and do it.

https://github.com/libsdl-org/SDL-1.2/commit/4c3097466bdf254a262ab7ffb72633992a892801