We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 5264 - Controller disconnect causes crash/exception thrown
Summary: Controller disconnect causes crash/exception thrown
Status: REOPENED
Alias: None
Product: SDL
Classification: Unclassified
Component: joystick (show other bugs)
Version: 2.0.12
Hardware: x86_64 Windows 10
: P2 normal
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL: https://github.com/visualboyadvance-m...
Keywords:
Depends on:
Blocks:
 
Reported: 2020-08-18 18:26 UTC by briansrls
Modified: 2020-10-18 10:47 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description briansrls 2020-08-18 18:26:53 UTC
The issue occurs on a disconnect of an Xbox One controller, either via bluetooth disconnect (idle shutdown, pulling batteries) or disconnecting a controller plugged in with usb.

Exception thrown at 0x00007FFA0405BF80 (SDL2d.dll) in visualboyadvance-m.exe: 0xC0000005: Access violation reading location 0x0000000000000048.

SDL2d.dll!SDL_JoystickRumble_REAL(_SDL_Joystick * joystick, unsigned short low_frequency_rumble, unsigned short high_frequency_rumble, unsigned int duration_ms) Line 776
	at C:\Users\brian\source\repos\vcpkg\buildtrees\sdl2\src\SDL2-2-76158f9b46.clean\src\joystick\SDL_joystick.c(776)
SDL2d.dll!SDL_JoystickClose_REAL(_SDL_Joystick * joystick) Line 826
	at C:\Users\brian\source\repos\vcpkg\buildtrees\sdl2\src\SDL2-2-76158f9b46.clean\src\joystick\SDL_joystick.c(826)
SDL2d.dll!SDL_JoystickClose(_SDL_Joystick * a) Line 246
	at C:\Users\brian\source\repos\vcpkg\buildtrees\sdl2\src\SDL2-2-76158f9b46.clean\src\dynapi\SDL_dynapi_procs.h(246)

Issue is easily reproducible on Ubuntu and Windows.

https://github.com/visualboyadvance-m/visualboyadvance-m/issues/718

Seems like joystick state is bad during/after a disconnect.
Comment 1 Mathieu Eyraud 2020-08-19 09:15:09 UTC
This is not a bug in SDL but in your app:

You use the 'joystick index' to add device in wxSDLJoy::joystate but use the 'instance id' when you remove it.

If you are lucky these ids match. Otherwise you read garbage from wxSDLJoy::joystate, that why you see your controller has a lot of button.

From 'SDL_GameControllerOpen' documentation:
"The index passed as an argument refers to the N'th game controller on the system. This index is not the value which will identify this controller in future controller events. The joystick's instance id (SDL_JoystickID) will be used there instead."
Comment 2 briansrls 2020-08-24 01:31:46 UTC
(In reply to meyraud705 from comment #1)
> This is not a bug in SDL but in your app:
> 
> You use the 'joystick index' to add device in wxSDLJoy::joystate but use the
> 'instance id' when you remove it.
> 
> If you are lucky these ids match. Otherwise you read garbage from
> wxSDLJoy::joystate, that why you see your controller has a lot of button.
> 
> From 'SDL_GameControllerOpen' documentation:
> "The index passed as an argument refers to the N'th game controller on the
> system. This index is not the value which will identify this controller in
> future controller events. The joystick's instance id (SDL_JoystickID) will
> be used there instead."

Hello meyraud705,

Thanks for the fast and clear response, I've gone ahead and started to make some changes to our SDL interface to better distinguish between instance ID and joystick index.

After making these changes, I still noticed some crashing during disconnecting.  Let me try to show a small flow with some output:

JOYDEVICEADDED Joy index = 0  //SDL_JOYDEVICEADDED event to add joystick index 0
CONTROLLERDEVICEADDED Joy index = 0 //SDL_CONTROLLER_DEVICE_ADDED
GameControllerOpen for idx 0 //Call to SDL_GameControllerOpen(0)
ConnectController 00000192BCE695B0  //ptr to the SDL::GameController returned                                                                                                  
6:19:22 PM: Debug: GOT SDL_CONTROLLERBUTTON: joy:0 but:10 val:1 prev_val:0  //Hitting some random buttons, get some controller events                                             
6:19:22 PM: Debug: GOT SDL_CONTROLLERBUTTON: joy:0 but:10 val:0 prev_val:1                                              
CONTROLLERDEVICEREMOVED Joy IID = 0 //SDL_CONTROLLERDEVICEREMOVED event (instance ID zero, need to map this instance ID to an index for disconnect)            
Disconnected game controller 1 //Disconnecting index 0, the printout shows 1 but internally we add 1 for user friendliness
JoystickClose 00000192BCE695B0  //Now, we map the instance ID back to the SDL::GameController we got earlier and get the correct pointer.
//Unhandled exception read access violation occurs at this point, somewhere in the JoystickClose

So, what was interesting to me in this flow is that during disconnect, the index we added as a GameController (index 0) is now not being recognized as a game controller via SDL_IsGameController(joy).  Maybe that makes sense in some way?

Here's our disconnect logic, I've added some comments.  You can correlate the printouts to the output I just pasted above.

    if (auto& dev = joystate[joy].dev) {
        if (SDL_IsGameController(joy)) { //Checking if index 0 is a gamecontroller, it is not
            if (SDL_GameControllerGetAttached(dev)) {
                std::cout << "GameControllerClose " << (SDL_GameController*)dev << std::endl;
                SDL_GameControllerClose(dev);
            }
        }
        else {
            if (SDL_JoystickGetAttached(dev)) { //Checking if this pointer is an attached joystick, it is
                std::cout << "JoystickClose " << (SDL_Joystick*)dev << std::endl;
                SDL_JoystickClose(dev); //Close the joystick, and throw exception.
            }
        }

        joystate.erase(joy);
    }

Reopening this for now, thanks.
Comment 3 briansrls 2020-08-24 01:35:26 UTC
Sorry the formatting isn't great, it looked better in the preview.  You can copy paste to a text editor to get the actual formatting back.
Comment 4 Mathieu Eyraud 2020-08-24 09:59:45 UTC
Disconnected joystick are not counted by SDL_NumJoysticks so they do not have an index. That is why your call to SDL_IsGameController return false after disconnecting your joystick. You need to store whether it's a game controller or not yourself.

Moreover, index are not constant during the lifetime of the joystick: if you have 2 joysticks connected and remove the one at index 0, then the second joystick changes its index from 1 to 0. This way index are always between 0 and SDL_NumJoysticks.

Only use index when opening joystick and use instance id or a pointer to SDL_GameController/SDL_Joystick in all other cases.
Comment 5 briansrls 2020-08-25 07:29:35 UTC
Thanks again, I have one more question.  In my SDL header file, I noticed a function exists for SDL_joystick.h - 

/**
 *  Get the instance ID of an opened joystick or -1 if the joystick is invalid.
 */
extern DECLSPEC SDL_JoystickID SDLCALL SDL_JoystickInstanceID(SDL_Joystick * joystick);

However, I couldn't find the corresponding GameController function. I do see a SDL_GameControllerGetJoystick function, which I could use to then get the Instance ID, but it seems strange that there wouldn't simply be a SDL_GameControllerInstanceID function.

The use case is when first opening a GameController, I'd like to save the details into a map indexed by instance_id (as it's a unique key value).  GameControllerOpen returns the pointer to the controller, but I don't see any obvious way of getting an instance ID until we get the first events.  Any ideas?
Comment 6 Mathieu Eyraud 2020-08-25 08:58:45 UTC
GameController is only a layer on top of a SDL_Joystick. As you guessed, you need to get the underlying joystick with SDL_GameControllerGetJoystick, and then get the joystick instance id.
Comment 7 briansrls 2020-08-27 20:24:34 UTC
Hi Meyraud,

Regarding disconnecting/reconnecting controllers.  Is there anyway to track a particular controller through disconnects/reconnects?  For example, we have player_index, does this stay with a particular controller even if it has completely disconnected?  Or, does SDL completely "forget" about controllers when they are closed/disconnected?

Let's say instance ID 0 connects, we assign some controls using the instance ID, and the controller disconnects/reconnects.  Now, the controller will be instance_id 1, and the binding we associated with instance ID 0 will never be valid anymore.  I thought to solve this with player index, but had the above question.
Comment 8 Mathieu Eyraud 2020-08-28 19:27:50 UTC
Yes, using player_index for binding is the way to go. When connecting a controller it will be assigned the first free player index.

Note that if you disconnect and reconnect only 1 controller it will get same player_index. But if you disconnect 2 controllers and reconnect them in reverse order they may swap their player_index.

Obviously, each time you restart your app, if you have several controllers, they may be assigned a different player_index.

If you need to have the binding associated with a specific device in a persistent way, you should use SDL_JoystickGUID. For example if your game is a driving sim and the steering and pedals are two devices, the throttle binding need to be always associated with an axis on the pedal.
Comment 9 briansrls 2020-08-28 22:14:33 UTC
That's good to hear, I'll consider using GUID, my feeling is that we don't need something that complex for now.

I was able to rewrite our code to better differentiate instance_id and index, and am now using player_index to track controllers and it seems to be working well.  

I have a few questions while wrapping up my changes:

1. SDL_CONTROLLERDEVICEREMAPPED - 
My header file says "/**< The controller mapping was updated */", what scenario is this sent in?


2. If I forcefully disconnect a joystick (unplug), and receive the JOYDEVICEREMOVED/CONTROLLERDEVICEREMOVED event from SDL, I'm assuming I don't have to explicitly close (GameControllerClose) the device in my program?  For now, I am just removing traces of that device in my program?

Thanks
Comment 10 Mathieu Eyraud 2020-08-30 15:12:48 UTC
1. I think it is only called if you update a mapping by calling SDL_GameControllerAddMapping or set SDL_HINT_GAMECONTROLLERCONFIG.

2. Joystick and controller are ref counted. You need to call close for each call to open to actually free their memory.