Bug #757

With Gamepad plugged in, ET:L does not start - black screen, not responding - SDL BUG #2833

Added by Kissaki over 4 years ago. Updated over 2 years ago.

Status:Fixed% Done:

100%

Priority:NormalSpent time:-
Assignee:-
Category:Client
Target version:2.76
OS:Windows Arch:

Description

With ET:L 2.71a, having my Razer Onza (XBox-style gamepad controller) plugged in ET:L does not start at all.

SDL2.dll seems to continually throw access violation exceptions.

In order to get a stacktrace I built 8ed276 myself with nmake VS2013, built SDL2 from the sources in the lib folder, and replaced the contents of SDL2-windows with the self-compiled SDL2 include files and library files.
The stack trace is attached as a screenshot.

2015-01-05 16_38_47-etl (Debugging) - Microsoft Visual Studio.png (16.2 KB) Kissaki, 05.01.2015 16:46

etl-gamepad-stacktrace-1.png (19.7 KB) Kissaki, 05.01.2015 17:58

etl-gamepad-stacktrace-2.png (18.4 KB) Kissaki, 05.01.2015 17:58

etl-gamepad-stacktrace-3.png (14.8 KB) Kissaki, 05.01.2015 17:58

519
520
521
522

Related issues

Related to ET: Legacy Development - Bug #326: Fix/update/inspect the joystick code New 02.08.2013
Related to ET: Legacy Development - Task #891: Update SDL2 to version 2.04 Fixed 24.11.2015
Duplicated by ET: Legacy Development - Bug #243: Mac OSX Joystick Support Invalid 13.03.2013

Associated revisions

Revision 2e8b4140
Added by Spyhawk over 4 years ago

sdl: disable joystick code, refs #757

Revision cecfa7a6
Added by Spyhawk over 4 years ago

sdl: commented out disabled code, refs #757

Revision 4cd54dc7
Added by IR4T4 over 3 years ago

sdl: joystick code enabled again - we no longer init the SDL joystick
subsystem when in_joystick cvar is set to 0 (which is default) refs #243
#326 #757

Revision d66999ed
Added by Spyhawk over 3 years ago

sdl: completely reenabled joystick code, refs #243 #326 #757

Revision b3273168
Added by IR4T4 over 3 years ago

sdl: don’t process the joystick when unfocussed etc (treat like mouse),
better descriptions for 'input devices’ , shut down joystick in
IN_Restart() refs #243 #326 #757

History

#1 Updated by IR4T4 over 4 years ago

  • Target version set to 2.78

#2 Updated by Kissaki over 4 years ago

I debugged a bit.

1.)
etl.exe IN_InitJoystick
calls SDL2.dll SDL_Init
calls SDL2.dll SDL_SYS_JoystickInit
calls SDL2.dll SDL_SYS_JoystickDetect
calls IDirectInput8_EnumDevices with a non-NULL dinput

2.)
Through etl.exe IN_InitJoystick
calls SDL2.dll SDL_JoystickQuit
calls SDL2.dll SDL_SYS_JoystickQuit
https://github.com/etlegacy/etlegacy/blob/fb46b5b97201749f792461c63c35484ff1981c60/src/sdl/sdl_input.c#L710-L715

SDL_SYS_JoystickQuit unsets dinput via `IDirectInput8_Release(dinput);`

3.)
Then, during event polling in the main event loop,
etl.exe IN_ProcessEvents 1030
calls SDL2.dll SDL_SYS_JoystickDetect again,
calls IDirectInput8_EnumDevices with with dinput = NULL (after it was released above

Thus, I suspect `IDirectInput8_EnumDevices` dereferences dinput which is NULL, which results in an access violation.
My guess now is that my gamepad is recognized my SDL2, an added event is added to the event queue, but the ETL code releases the joystick before this event is handled.

So I guess, either the early release is not allowed by the SDL, or the SDL is not robust enough.

#3 Updated by Kissaki over 4 years ago

Attached stacktraces as screenshots.

#4 Updated by Saukko over 4 years ago

I have a couple of gamepads at home. I had my Logitech Extreme Wingman plugged in for ages without any problems occuring. It is a pretty old gamepad but I have also another one to test with. I will tell tomorrow about the results when I get back home.

-*S

#5 Updated by Kissaki over 4 years ago

  • Assignee set to Kissaki

#6 Updated by Spyhawk over 4 years ago

From IRC:

18:01 < Kissaki> currently forumlating a robustness question on their irc :)
18:04 < Ensiform> what SDL2
18:11 < Ensiform> where is LegacySDL_Init
18:12 < Ensiform> according to docs the added NOPARACHUTE is irrelevant :d
18:14 < Ensiform> SDL_Quit is also ignored on windows
18:14 < Kissaki> it's an etl implementation error. SDL guys say only one SDL_Init should be called, and subsystems be initialized 
                 by SDL_InitSubsystem
18:15 < Ensiform> also an ioq3 error

SDL NOPARACHUTE is now build in, and SDL_InitSubSystem should be used instead of multiple SDL_Init calls.

#7 Updated by Kissaki over 4 years ago

Spyhawk wrote:

18:14 < Kissaki> it’s an etl implementation error. SDL guys say only one SDL_Init should be called, and subsystems be initialized by SDL_InitSubsystem

That’s obsolete.
Initial misinformation by one of the people in their IRC. Was resolved through discussion.

I created a minimal program for reproducibility of the issue and will submit it to the SDL bug tracker.
Before that, will try with their current HEAD version.

As for the not directly associated LegacySDL_Init, I did local commits cleaning that up as well as the incomplete checks of the return code and will try to send that as a PR.

#8 Updated by Kissaki over 4 years ago

To be more precise, SDL_InitSubsystem should be used, but is not really a requirement (SDL_Init calls that directly anyway).
For clean code, it should - according to the #SDL chatters - be used though.

#9 Updated by Kissaki over 4 years ago

I created an SDL bug 2833()
with a minimal program that leads to the access violation.

#10 Updated by Kissaki over 4 years ago

  • Assignee deleted (Kissaki)

#11 Updated by Kissaki over 4 years ago

I also created the mentioned pull request for the changes that don’t belong to this issue (just related, and reasonable changes).
https://github.com/etlegacy/etlegacy/pull/110

#12 Updated by Spyhawk over 4 years ago

  • Related to Bug #326: Fix/update/inspect the joystick code added

#13 Updated by Spyhawk over 4 years ago

Since we’ve had at least 5 reports of crash-at-startup on Windows in the past few days, I disabled the Joystick code for now.
According to SDL #2833 and the attached test program, it seems to really be a SDL2 bug - nothing we can do here.

But maybe it’d be worth adding some macro to enable it for Pandora users only?

#14 Updated by Spyhawk over 4 years ago

  • Related to deleted (Bug #326: Fix/update/inspect the joystick code)

#15 Updated by Spyhawk over 4 years ago

  • Duplicated by Bug #326: Fix/update/inspect the joystick code added

#16 Updated by Spyhawk over 4 years ago

  • Duplicated by deleted (Bug #326: Fix/update/inspect the joystick code)

#17 Updated by Spyhawk over 4 years ago

  • Duplicated by Bug #243: Mac OSX Joystick Support added

#18 Updated by Spyhawk over 4 years ago

  • Related to Bug #326: Fix/update/inspect the joystick code added

#19 Updated by Spyhawk over 4 years ago

  • Target version changed from 2.78 to ALL

#20 Updated by Spyhawk over 3 years ago

  • Related to Task #891: Update SDL2 to version 2.04 added

#21 Updated by IR4T4 over 3 years ago

  • Target version changed from ALL to 2.75
From the SDL2 2.04 changelog: Mac OS X:
  • Improved joystick hot-plug detection

I don’t know if SDL 2.04 fixes the above case Kissaki discovered or the issues with OSX. However our joystick code is inactive (see IN_InitJoystick()) and we should enable it again.
IN_InitJoystick() needs some rework: Regardless how in_joystick cvar is set the code tries to init system joysticks/gamepads. We should check in_joystick at start of IN_InitJoystick(), so there is no system init process when in_joystick is set to 0.

#22 Updated by IR4T4 over 3 years ago

  • Subject changed from With Gamepad plugged in, ET:L does not start - black screen, not responding to With Gamepad plugged in, ET:L does not start - black screen, not responding - SDL BUG #2833
  • Target version changed from 2.75 to ALL
  • % Done changed from 0 to 20

Joystick code has been enabled again. In case of crash restart ETL:

etl[exe] +set in_joystick 0

Waiting for the SDL fix - moved to milestone ALL.

#23 Updated by Saukko over 2 years ago

Could it be that this issue is fixed? At least according to the link below.
https://hg.libsdl.org/SDL/rev/aa816d06ed78

-*S

#24 Updated by Spyhawk over 2 years ago

  • % Done changed from 20 to 90

It will be fixed once a new SDL version is released, and once we do a new release using it.

#25 Updated by Spyhawk over 2 years ago

  • Target version changed from ALL to 2.76

This is fixed in SDL 2.0.5 which has been released a few weeks ago. We should upgrade our SDL for the next version.
Also remove related comment in sdl_input.c.

#26 Updated by Spyhawk over 2 years ago

  • Status changed from New to Fixed
  • % Done changed from 90 to 100

Also available in: Atom PDF