input/sdl: lock map mutex after SDL call

Any SDL invocation can call the even callback on the same thread, which can call GetSDLJoystickBySDLID and eventually cause double lock on joystick_map_mutex. To avoid this, lock guard should be placed as closer as possible to the object accessing code, so that any SDL invocation is with the mutex unlocked
This commit is contained in:
Weiyi Wang 2018-10-02 11:06:42 -04:00
parent 9813d0deed
commit 22df17c303

View File

@ -159,9 +159,9 @@ std::shared_ptr<SDLJoystick> SDLState::GetSDLJoystickByGUID(const std::string& g
* it to a SDLJoystick with the same guid and that port * it to a SDLJoystick with the same guid and that port
*/ */
std::shared_ptr<SDLJoystick> SDLState::GetSDLJoystickBySDLID(SDL_JoystickID sdl_id) { std::shared_ptr<SDLJoystick> SDLState::GetSDLJoystickBySDLID(SDL_JoystickID sdl_id) {
std::lock_guard<std::mutex> lock(joystick_map_mutex);
auto sdl_joystick = SDL_JoystickFromInstanceID(sdl_id); auto sdl_joystick = SDL_JoystickFromInstanceID(sdl_id);
const std::string guid = GetGUID(sdl_joystick); const std::string guid = GetGUID(sdl_joystick);
std::lock_guard<std::mutex> lock(joystick_map_mutex);
auto map_it = joystick_map.find(guid); auto map_it = joystick_map.find(guid);
if (map_it != joystick_map.end()) { if (map_it != joystick_map.end()) {
auto vec_it = std::find_if(map_it->second.begin(), map_it->second.end(), auto vec_it = std::find_if(map_it->second.begin(), map_it->second.end(),
@ -193,13 +193,13 @@ std::shared_ptr<SDLJoystick> SDLState::GetSDLJoystickBySDLID(SDL_JoystickID sdl_
} }
void SDLState::InitJoystick(int joystick_index) { void SDLState::InitJoystick(int joystick_index) {
std::lock_guard<std::mutex> lock(joystick_map_mutex);
SDL_Joystick* sdl_joystick = SDL_JoystickOpen(joystick_index); SDL_Joystick* sdl_joystick = SDL_JoystickOpen(joystick_index);
if (!sdl_joystick) { if (!sdl_joystick) {
LOG_ERROR(Input, "failed to open joystick {}", joystick_index); LOG_ERROR(Input, "failed to open joystick {}", joystick_index);
return; return;
} }
std::string guid = GetGUID(sdl_joystick); std::string guid = GetGUID(sdl_joystick);
std::lock_guard<std::mutex> lock(joystick_map_mutex);
if (joystick_map.find(guid) == joystick_map.end()) { if (joystick_map.find(guid) == joystick_map.end()) {
auto joystick = std::make_shared<SDLJoystick>(guid, 0, sdl_joystick); auto joystick = std::make_shared<SDLJoystick>(guid, 0, sdl_joystick);
joystick_map[guid].emplace_back(std::move(joystick)); joystick_map[guid].emplace_back(std::move(joystick));
@ -218,8 +218,10 @@ void SDLState::InitJoystick(int joystick_index) {
} }
void SDLState::CloseJoystick(SDL_Joystick* sdl_joystick) { void SDLState::CloseJoystick(SDL_Joystick* sdl_joystick) {
std::lock_guard<std::mutex> lock(joystick_map_mutex);
std::string guid = GetGUID(sdl_joystick); std::string guid = GetGUID(sdl_joystick);
std::shared_ptr<SDLJoystick> joystick;
{
std::lock_guard<std::mutex> lock(joystick_map_mutex);
// This call to guid is safe since the joystick is guaranteed to be in the map // This call to guid is safe since the joystick is guaranteed to be in the map
auto& joystick_guid_list = joystick_map[guid]; auto& joystick_guid_list = joystick_map[guid];
const auto joystick_it = const auto joystick_it =
@ -227,7 +229,11 @@ void SDLState::CloseJoystick(SDL_Joystick* sdl_joystick) {
[&sdl_joystick](const std::shared_ptr<SDLJoystick>& joystick) { [&sdl_joystick](const std::shared_ptr<SDLJoystick>& joystick) {
return joystick->GetSDLJoystick() == sdl_joystick; return joystick->GetSDLJoystick() == sdl_joystick;
}); });
(*joystick_it)->SetSDLJoystick(nullptr, [](SDL_Joystick*) {}); joystick = *joystick_it;
}
// Destruct SDL_Joystick outside the lock guard because SDL can internally call event calback
// which locks the mutex again
joystick->SetSDLJoystick(nullptr, [](SDL_Joystick*) {});
} }
void SDLState::HandleGameControllerEvent(const SDL_Event& event) { void SDLState::HandleGameControllerEvent(const SDL_Event& event) {