-
-
Notifications
You must be signed in to change notification settings - Fork 22
Description
I posted about this on discord already, but wanted a Issue number for a PR.
I have been diving a bit into the gamepad c++ code. I have to admit that the last time I worked actively with C++ was around 2013, but I believe I still have a good idea about its memory management. Though, if I made a mistake, please point it out to me.
When I look at update_gamepads() and connect_gamepad(), as far as I can see the created read_thread is passed a reference to gamepads[joy_id].
Upon gamepad update (line 120-126), the memory slot that the old thread reads from is overwritten with a new struct data that sets alive to true again, after having been false for a few instructions.
But shouldn't there be an explicit kill/wait for the old read thread to react to alive=false, before the memory position in gamepads is reused? Although I am not 100% sure here, I would believe that the std::map either reuse the same memory for the old and the new struct or the old struct data gets deallocated as there is no garbage collection. So either the old thread get alive=true or a stale pointer, if it is slower and doesn't reach the while (gamepad->alive) check.
Adding to this, some std::map implementations could possible leave a removed value still in memory or not reuse the same slot as this is not a pure array, but it seems from my research be a bit up to each compiler and not fully specified by the spec.
Possible solutions
- Use std::thread.join() to wait for the outgoing thread to finish before updating
gamepads - Fiddle with the gamepads data structure to allow the Gamepad memory piece to stay around even after a joy_id has been discarded as disconnected so that the thread can still access it with its pointer.
I feel that 1. is possible easiest to implement, has slightly lower memory usage, but incurs a slight delay for the thread synchronization.
Any input is welcome on solution or if infact std::map still will keep the old memory piece around and this isn't actually an issue.