From 4ae11053ce835ad21c54e55969f7fd55fab69649 Mon Sep 17 00:00:00 2001 From: Flyinghead Date: Thu, 21 Feb 2019 14:49:27 +0100 Subject: [PATCH] use smart pointers to avoid crash when a gamepad is disconnected --- core/input/gamepad_device.cpp | 6 +-- core/input/gamepad_device.h | 41 +++++++++++-------- core/linux-dist/evdev.cpp | 9 ++-- core/linux-dist/evdev_gamepad.h | 26 +++++++----- core/rend/gui.cpp | 12 ++++-- core/sdl/sdl.cpp | 14 ++++--- core/sdl/sdl_gamepad.h | 22 ++++++---- .../reicast/src/main/jni/src/Android.cpp | 13 +++--- .../src/main/jni/src/android_gamepad.h | 23 +++++++---- 9 files changed, 100 insertions(+), 66 deletions(-) diff --git a/core/input/gamepad_device.cpp b/core/input/gamepad_device.cpp index fbdbd5500..9b4b58620 100644 --- a/core/input/gamepad_device.cpp +++ b/core/input/gamepad_device.cpp @@ -27,7 +27,7 @@ extern u16 kcode[4]; extern u8 rt[4], lt[4]; extern s8 joyx[4], joyy[4]; -std::vector GamepadDevice::_gamepads; +std::vector> GamepadDevice::_gamepads; std::mutex GamepadDevice::_gamepads_mutex; bool GamepadDevice::gamepad_btn_input(u32 code, bool pressed) @@ -183,10 +183,10 @@ int GamepadDevice::GetGamepadCount() return count; } -GamepadDevice *GamepadDevice::GetGamepad(int index) +std::shared_ptr GamepadDevice::GetGamepad(int index) { _gamepads_mutex.lock(); - GamepadDevice *dev; + std::shared_ptr dev; if (index >= 0 && index < _gamepads.size()) dev = _gamepads[index]; else diff --git a/core/input/gamepad_device.h b/core/input/gamepad_device.h index 050562fe4..8aa1b776e 100644 --- a/core/input/gamepad_device.h +++ b/core/input/gamepad_device.h @@ -18,6 +18,7 @@ */ #pragma once +#include #include #include "types.h" #include "mapping.h" @@ -32,17 +33,8 @@ public: void set_maple_port(int port) { _maple_port = port; } virtual bool gamepad_btn_input(u32 code, bool pressed); bool gamepad_axis_input(u32 code, int value); - virtual ~GamepadDevice() { - save_mapping(); - _gamepads_mutex.lock(); - for (auto it = _gamepads.begin(); it != _gamepads.end(); it++) - if (*it == this) - { - _gamepads.erase(it); - break; - } - _gamepads_mutex.unlock(); - } + virtual ~GamepadDevice() {} + void detect_btn_input(input_detected_cb button_pressed) { _input_detected = button_pressed; @@ -61,16 +53,33 @@ public: void save_mapping(); bool remappable() { return _remappable; } + static void Register(std::shared_ptr gamepad) + { + _gamepads_mutex.lock(); + _gamepads.push_back(gamepad); + _gamepads_mutex.unlock(); + } + + static void Unregister(std::shared_ptr gamepad) + { + gamepad->save_mapping(); + _gamepads_mutex.lock(); + for (auto it = _gamepads.begin(); it != _gamepads.end(); it++) + if (*it == gamepad) + { + _gamepads.erase(it); + break; + } + _gamepads_mutex.unlock(); + } + static int GetGamepadCount(); - static GamepadDevice *GetGamepad(int index); + static std::shared_ptr GetGamepad(int index); protected: GamepadDevice(int maple_port, const char *api_name, bool remappable = true) : _api_name(api_name), _maple_port(maple_port), input_mapper(NULL), _input_detected(NULL), _remappable(remappable) { - _gamepads_mutex.lock(); - _gamepads.push_back(this); - _gamepads_mutex.unlock(); } bool find_mapping(const char *custom_mapping = NULL); @@ -92,6 +101,6 @@ private: input_detected_cb _input_detected; bool _remappable; - static std::vector _gamepads; + static std::vector> _gamepads; static std::mutex _gamepads_mutex; }; diff --git a/core/linux-dist/evdev.cpp b/core/linux-dist/evdev.cpp index 4712e8034..5740cca23 100644 --- a/core/linux-dist/evdev.cpp +++ b/core/linux-dist/evdev.cpp @@ -18,19 +18,20 @@ static void input_evdev_add_device(const char *devnode) int fd = open(devnode, O_RDWR); if (fd >= 0) { - new EvdevGamepadDevice(maple_port, devnode, fd); + std::shared_ptr gamepad = std::make_shared(maple_port, devnode, fd); if (maple_port < 3) maple_port++; + EvdevGamepadDevice::AddDevice(gamepad); } } static void input_evdev_remove_device(const char *devnode) { - EvdevGamepadDevice *gamepad = EvdevGamepadDevice::GetControllerForDevnode(devnode); + std::shared_ptr gamepad = EvdevGamepadDevice::GetControllerForDevnode(devnode); if (gamepad != NULL) { maple_port = gamepad->maple_port(); // Reuse the maple port for the next device connected - delete gamepad; + EvdevGamepadDevice::RemoveDevice(gamepad); } } @@ -212,7 +213,7 @@ bool input_evdev_handle(u32 port) void input_evdev_rumble(u32 port, u16 pow_strong, u16 pow_weak) { - EvdevGamepadDevice *dev = EvdevGamepadDevice::GetControllerForPort(port); + std::shared_ptr dev = EvdevGamepadDevice::GetControllerForPort(port); if (dev != NULL) dev->Rumble(pow_strong, pow_weak); } diff --git a/core/linux-dist/evdev_gamepad.h b/core/linux-dist/evdev_gamepad.h index 93f5a985a..06bdf66ae 100644 --- a/core/linux-dist/evdev_gamepad.h +++ b/core/linux-dist/evdev_gamepad.h @@ -54,16 +54,11 @@ public: } else printf("using custom mapping '%s'\n", input_mapper->name.c_str()); - auto it = evdev_gamepads.find(_devnode); - if (it != evdev_gamepads.end()) - delete it->second; - evdev_gamepads[_devnode] = this; } virtual ~EvdevGamepadDevice() override { printf("evdev: Device '%s' on port %d disconnected\n", _name.c_str(), maple_port()); close(_fd); - evdev_gamepads.erase(_devnode); } // FIXME add to base class @@ -99,7 +94,7 @@ public: } } - static EvdevGamepadDevice *GetControllerForPort(int port) + static std::shared_ptr GetControllerForPort(int port) { for (auto pair : evdev_gamepads) if (pair.second->maple_port() == port) @@ -107,7 +102,7 @@ public: return NULL; } - static EvdevGamepadDevice *GetControllerForDevnode(const char *devnode) + static std::shared_ptr GetControllerForDevnode(const char *devnode) { auto it = evdev_gamepads.find(devnode); if (it == evdev_gamepads.end()) @@ -124,7 +119,18 @@ public: static void CloseDevices() { while (!evdev_gamepads.empty()) - delete evdev_gamepads.begin()->second; + RemoveDevice(evdev_gamepads.begin()->second); + } + + static void AddDevice(std::shared_ptr gamepad) + { + evdev_gamepads[gamepad->_devnode] = gamepad; + GamepadDevice::Register(gamepad); + } + static void RemoveDevice(std::shared_ptr gamepad) + { + evdev_gamepads.erase(gamepad->_devnode); + GamepadDevice::Unregister(gamepad); } protected: @@ -167,7 +173,7 @@ private: int _fd; std::string _devnode; int _rumble_effect_id; - static std::map evdev_gamepads; + static std::map> evdev_gamepads; }; -std::map EvdevGamepadDevice::evdev_gamepads; +std::map> EvdevGamepadDevice::evdev_gamepads; diff --git a/core/rend/gui.cpp b/core/rend/gui.cpp index be3ad901d..c3daab2c7 100644 --- a/core/rend/gui.cpp +++ b/core/rend/gui.cpp @@ -397,7 +397,7 @@ static MapleDeviceType maple_expansion_device_type_from_index(int idx) } } -static GamepadDevice *mapped_device; +static std::shared_ptr mapped_device; static u32 mapped_code; static double map_start_time; @@ -429,16 +429,20 @@ static void detect_input_popup(int index, bool analog) } else mapped_device->get_input_mapping()->set_button(button_keys[index], mapped_code); + mapped_device = NULL; ImGui::CloseCurrentPopup(); } else if (now - map_start_time >= 5) + { + mapped_device = NULL; ImGui::CloseCurrentPopup(); + } ImGui::EndPopup(); } ImGui::PopStyleVar(2); } -static void controller_mapping_popup(GamepadDevice *gamepad) +static void controller_mapping_popup(std::shared_ptr gamepad) { ImGui::SetNextWindowPos(ImVec2(0, 0)); ImGui::SetNextWindowSize(ImVec2(screen_width, screen_height)); @@ -715,8 +719,8 @@ static void gui_display_settings() ImGui::NextColumn(); for (int i = 0; i < GamepadDevice::GetGamepadCount(); i++) { - GamepadDevice *gamepad = GamepadDevice::GetGamepad(i); - if (gamepad == NULL) + std::shared_ptr gamepad = GamepadDevice::GetGamepad(i); + if (!gamepad) continue; ImGui::Text("%s", gamepad->api_name().c_str()); ImGui::NextColumn(); diff --git a/core/sdl/sdl.cpp b/core/sdl/sdl.cpp index 14882f6e4..39b56a043 100644 --- a/core/sdl/sdl.cpp +++ b/core/sdl/sdl.cpp @@ -52,14 +52,15 @@ static void sdl_open_joystick(int index) printf("SDL: Cannot open joystick %d\n", index + 1); return; } - new SDLGamepadDevice(index < MAPLE_PORTS ? index : -1, pJoystick); + std::shared_ptr gamepad = std::make_shared(index < MAPLE_PORTS ? index : -1, pJoystick); + SDLGamepadDevice::AddSDLGamepad(gamepad); } static void sdl_close_joystick(SDL_JoystickID instance) { - SDLGamepadDevice *device = SDLGamepadDevice::GetSDLGamepad(instance); - if (device != NULL) - delete device; + std::shared_ptr gamepad = SDLGamepadDevice::GetSDLGamepad(instance); + if (gamepad != NULL) + gamepad->Close(); } void input_sdl_init() @@ -91,6 +92,7 @@ static void set_mouse_position(int x, int y) } } +// FIXME this shouldn't be done by port. Need something like: handle_events() then get_port(0), get_port(2), ... void input_sdl_handle(u32 port) { #define SET_FLAG(field, mask, expr) field =((expr) ? (field & ~mask) : (field | mask)) @@ -117,14 +119,14 @@ void input_sdl_handle(u32 port) case SDL_JOYBUTTONDOWN: case SDL_JOYBUTTONUP: { - SDLGamepadDevice *device = SDLGamepadDevice::GetSDLGamepad((SDL_JoystickID)event.jbutton.which); + std::shared_ptr device = SDLGamepadDevice::GetSDLGamepad((SDL_JoystickID)event.jbutton.which); if (device != NULL) device->gamepad_btn_input(event.jbutton.button, event.type == SDL_JOYBUTTONDOWN); } break; case SDL_JOYAXISMOTION: { - SDLGamepadDevice *device = SDLGamepadDevice::GetSDLGamepad((SDL_JoystickID)event.jaxis.which); + std::shared_ptr device = SDLGamepadDevice::GetSDLGamepad((SDL_JoystickID)event.jaxis.which); if (device != NULL) device->gamepad_axis_input(event.jaxis.axis, event.jaxis.value); } diff --git a/core/sdl/sdl_gamepad.h b/core/sdl/sdl_gamepad.h index 89c618733..70ff3345f 100644 --- a/core/sdl/sdl_gamepad.h +++ b/core/sdl/sdl_gamepad.h @@ -65,20 +65,24 @@ public: } else printf("using custom mapping '%s'\n", input_mapper->name.c_str()); - auto it = sdl_gamepads.find(sdl_joystick_instance); - if (it != sdl_gamepads.end()) - delete it->second; - sdl_gamepads[sdl_joystick_instance] = this; } - virtual ~SDLGamepadDevice() override + + SDL_JoystickID sdl_instance() { return sdl_joystick_instance; } + + void Close() { printf("SDL: Joystick '%s' on port %d disconnected\n", _name.c_str(), maple_port()); SDL_JoystickClose(sdl_joystick); + GamepadDevice::Unregister(gamepad); sdl_gamepads.erase(sdl_joystick_instance); } - SDL_JoystickID sdl_instance() { return sdl_joystick_instance; } - static SDLGamepadDevice *GetSDLGamepad(SDL_JoystickID id) + static void AddSDLGamepad(std::shared_ptr gamepad) + { + sdl_gamepads[gamepad->sdl_joystick_instance] = gamepad; + GamepadDevice::Register(gamepad); + } + static std::shared_ptr GetSDLGamepad(SDL_JoystickID id) { auto it = sdl_gamepads.find(id); if (it != sdl_gamepads.end()) @@ -97,10 +101,10 @@ protected: private: SDL_Joystick* sdl_joystick; SDL_JoystickID sdl_joystick_instance; - static std::map sdl_gamepads; + static std::map> sdl_gamepads; }; -std::map SDLGamepadDevice::sdl_gamepads; +std::map> SDLGamepadDevice::sdl_gamepads; class KbInputMapping : public InputMapping { diff --git a/shell/android-studio/reicast/src/main/jni/src/Android.cpp b/shell/android-studio/reicast/src/main/jni/src/Android.cpp index 98a9127a7..d0be66232 100644 --- a/shell/android-studio/reicast/src/main/jni/src/Android.cpp +++ b/shell/android-studio/reicast/src/main/jni/src/Android.cpp @@ -717,27 +717,28 @@ void os_DebugBreak() JNIEXPORT void JNICALL Java_com_reicast_emulator_periph_InputDeviceManager_joystickAdded(JNIEnv *env, jobject obj, jint id, jstring name, jint maple_port) { const char* joyname = env->GetStringUTFChars(name,0); - new AndroidGamepadDevice(maple_port, id, joyname); + std::shared_ptr gamepad = std::make_shared(maple_port, id, joyname); + AndroidGamepadDevice::AddAndroidGamepad(gamepad); env->ReleaseStringUTFChars(name, joyname); } JNIEXPORT void JNICALL Java_com_reicast_emulator_periph_InputDeviceManager_joystickRemoved(JNIEnv *env, jobject obj, jint id) { - AndroidGamepadDevice *device = AndroidGamepadDevice::GetAndroidGamepad(id); + std::shared_ptr device = AndroidGamepadDevice::GetAndroidGamepad(id); if (device != NULL) - delete device; + AndroidGamepadDevice::RemoveAndroidGamepad(device); } JNIEXPORT void JNICALL Java_com_reicast_emulator_periph_InputDeviceManager_virtualGamepadEvent(JNIEnv *env, jobject obj, jint kcode, jint joyx, jint joyy, jint lt, jint rt) { - AndroidGamepadDevice *device = AndroidGamepadDevice::GetAndroidGamepad(AndroidGamepadDevice::VIRTUAL_GAMEPAD_ID); + std::shared_ptr device = AndroidGamepadDevice::GetAndroidGamepad(AndroidGamepadDevice::VIRTUAL_GAMEPAD_ID); if (device != NULL) device->virtual_gamepad_event(kcode, joyx, joyy, lt, rt); } JNIEXPORT jboolean JNICALL Java_com_reicast_emulator_periph_InputDeviceManager_joystickButtonEvent(JNIEnv *env, jobject obj, jint id, jint key, jboolean pressed) { - AndroidGamepadDevice *device = AndroidGamepadDevice::GetAndroidGamepad(id); + std::shared_ptr device = AndroidGamepadDevice::GetAndroidGamepad(id); if (device != NULL) return device->gamepad_btn_input(key, pressed); else @@ -746,7 +747,7 @@ JNIEXPORT jboolean JNICALL Java_com_reicast_emulator_periph_InputDeviceManager_j } JNIEXPORT jboolean JNICALL Java_com_reicast_emulator_periph_InputDeviceManager_joystickAxisEvent(JNIEnv *env, jobject obj, jint id, jint key, jint value) { - AndroidGamepadDevice *device = AndroidGamepadDevice::GetAndroidGamepad(id); + std::shared_ptr device = AndroidGamepadDevice::GetAndroidGamepad(id); if (device != NULL) return device->gamepad_axis_input(key, value); else diff --git a/shell/android-studio/reicast/src/main/jni/src/android_gamepad.h b/shell/android-studio/reicast/src/main/jni/src/android_gamepad.h index 1be54e0b4..8d28152f7 100644 --- a/shell/android-studio/reicast/src/main/jni/src/android_gamepad.h +++ b/shell/android-studio/reicast/src/main/jni/src/android_gamepad.h @@ -92,18 +92,13 @@ public: } else printf("using custom mapping '%s'\n", input_mapper->name.c_str()); - auto it = android_gamepads.find(id); - if (it != android_gamepads.end()) - delete it->second; - android_gamepads[id] = this; } virtual ~AndroidGamepadDevice() override { printf("Android: Joystick '%s' on port %d disconnected\n", _name.c_str(), maple_port()); - android_gamepads.erase(android_id); } - static AndroidGamepadDevice *GetAndroidGamepad(int id) + static std::shared_ptr GetAndroidGamepad(int id) { auto it = android_gamepads.find(id); if (it != android_gamepads.end()) @@ -112,6 +107,18 @@ public: return NULL; } + static void AddAndroidGamepad(std::shared_ptr gamepad) + { + android_gamepads[gamepad->android_id] = gamepad; + GamepadDevice::Register(gamepad); + }; + + static void RemoveAndroidGamepad(std::shared_ptr gamepad) + { + android_gamepads.erase(gamepad->android_id); + GamepadDevice::Unregister(gamepad); + }; + void virtual_gamepad_event(int kcode, int joyx, int joyy, int lt, int rt) { // No virtual gamepad when the GUI is open: touch events only @@ -150,11 +157,11 @@ protected: private: int android_id; - static std::map android_gamepads; + static std::map> android_gamepads; u16 previous_kcode = 0xffff; }; -std::map AndroidGamepadDevice::android_gamepads; +std::map> AndroidGamepadDevice::android_gamepads; class MouseInputMapping : public InputMapping {