mirror of
https://github.com/CTCaer/RetroArch.git
synced 2024-12-14 06:18:34 +00:00
Fix deadlocks when device is unplugged
== DETAILS TIL that it's bad to call synchronization code from callbacks. To avoid that, I made the following changes: - Implemented an atomic swap (see previous commit) to avoid explicit locking when working with the event list - ensure locks are only acquired in either the main thread or the I/O polling thread - use an explicit polling loop; we still use async reads, but the read doesn't immediately re-invoke itself. - remove the sleep in the polling thread. - remove unnecessary locking in the thread cleanup call--verified that the list can't be modified while it is being executed. == TESTING I tested locally, and was able to disconnect/reconnect USB devices several times without the worker thread getting deadlocked.
This commit is contained in:
gblues
parent
dca36ebaf8
commit
97e09d179f
@ -153,14 +153,16 @@ static void *ds3_init(void *handle)
|
||||
if(!instance)
|
||||
goto error;
|
||||
|
||||
memset(instance, 0, sizeof(ds3_instance_t));
|
||||
instance->handle = handle;
|
||||
|
||||
/* maybe not necessary? */
|
||||
/*
|
||||
RARCH_LOG("[ds3]: setting protocol\n");
|
||||
/*
|
||||
if(set_protocol(instance, 1))
|
||||
errors++;
|
||||
*/
|
||||
set_protocol(instance, 1);
|
||||
|
||||
RARCH_LOG("[ds3]: sending control packet\n");
|
||||
if(send_control_packet(instance) < 0)
|
||||
errors++;
|
||||
|
||||
@ -16,27 +16,63 @@
|
||||
|
||||
#include "hid_device_driver.h"
|
||||
|
||||
struct ds4_instance {
|
||||
hid_driver_t *driver;
|
||||
void *handle;
|
||||
};
|
||||
extern pad_connection_interface_t ds4_pad_connection;
|
||||
|
||||
typedef struct ds4_instance {
|
||||
void *handle;
|
||||
joypad_connection_t *pad;
|
||||
int slot;
|
||||
uint32_t buttons;
|
||||
uint16_t motors[2];
|
||||
uint8_t data[64];
|
||||
} ds4_instance_t;
|
||||
|
||||
/**
|
||||
* I'm leaving this code in here for posterity, and because maybe it can
|
||||
* be used on other platforms. But using the DS4 on the Wii U directly is
|
||||
* impossible because it doesn't generate a HID event. Which makes me think
|
||||
* it's not a HID device at all--at least, not over USB.
|
||||
*
|
||||
* I imagine it might be useful in Bluetooth mode, though.
|
||||
*/
|
||||
static void *ds4_init(void *handle)
|
||||
{
|
||||
return NULL;
|
||||
ds4_instance_t *instance;
|
||||
instance = (ds4_instance_t *)calloc(1, sizeof(ds4_instance_t));
|
||||
if(!instance)
|
||||
goto error;
|
||||
|
||||
memset(instance, 0, sizeof(ds4_instance_t));
|
||||
instance->handle = handle;
|
||||
instance->pad = hid_pad_register(instance, &ds4_pad_connection);
|
||||
if(!instance->pad)
|
||||
goto error;
|
||||
|
||||
RARCH_LOG("[ds4]: init complete.\n");
|
||||
return instance;
|
||||
|
||||
error:
|
||||
RARCH_ERR("[ds4]: init failed.\n");
|
||||
if(instance)
|
||||
free(instance);
|
||||
|
||||
return NULL;
|
||||
}
|
||||
|
||||
static void ds4_free(void *data)
|
||||
{
|
||||
struct ds4_instance *instance = (struct ds4_instance *)data;
|
||||
if(!instance)
|
||||
return;
|
||||
ds4_instance_t *instance = (ds4_instance_t *)data;
|
||||
|
||||
free(instance);
|
||||
if(instance)
|
||||
free(instance);
|
||||
}
|
||||
|
||||
static void ds4_handle_packet(void *data, uint8_t *buffer, size_t size)
|
||||
{
|
||||
ds4_instance_t *instance = (ds4_instance_t *)data;
|
||||
|
||||
if(instance && instance->pad)
|
||||
instance->pad->iface->packet_handler(instance->pad->data, buffer, size);
|
||||
}
|
||||
|
||||
static bool ds4_detect(uint16_t vendor_id, uint16_t product_id)
|
||||
@ -51,3 +87,66 @@ hid_device_t ds4_hid_device = {
|
||||
ds4_detect,
|
||||
"Sony DualShock 4"
|
||||
};
|
||||
|
||||
static void *ds4_pad_init(void *data, uint32_t slot, hid_driver_t *driver)
|
||||
{
|
||||
ds4_instance_t *instance = (ds4_instance_t *)data;
|
||||
|
||||
if(!instance)
|
||||
return NULL;
|
||||
|
||||
instance->slot = slot;
|
||||
return instance;
|
||||
}
|
||||
|
||||
static void ds4_pad_deinit(void *data)
|
||||
{
|
||||
}
|
||||
|
||||
static void ds4_get_buttons(void *data, retro_bits_t *state)
|
||||
{
|
||||
ds4_instance_t *instance = (ds4_instance_t *)data;
|
||||
if(!instance)
|
||||
return;
|
||||
|
||||
/* TODO: get buttons */
|
||||
}
|
||||
|
||||
static void ds4_packet_handler(void *data, uint8_t *packet, uint16_t size)
|
||||
{
|
||||
ds4_instance_t *instance = (ds4_instance_t *)data;
|
||||
if(!instance)
|
||||
return;
|
||||
|
||||
RARCH_LOG_BUFFER(packet, size);
|
||||
}
|
||||
|
||||
static void ds4_set_rumble(void *data, enum retro_rumble_effect effect, uint16_t strength)
|
||||
{
|
||||
}
|
||||
|
||||
static int16_t ds4_get_axis(void *data, unsigned axis)
|
||||
{
|
||||
return 0;
|
||||
}
|
||||
|
||||
static const char *ds4_get_name(void *data)
|
||||
{
|
||||
return "Sony DualShock 4";
|
||||
}
|
||||
|
||||
static bool ds4_button(void *data, uint16_t joykey)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
pad_connection_interface_t ds4_pad_connection = {
|
||||
ds4_pad_init,
|
||||
ds4_pad_deinit,
|
||||
ds4_packet_handler,
|
||||
ds4_set_rumble,
|
||||
ds4_get_buttons,
|
||||
ds4_get_axis,
|
||||
ds4_get_name,
|
||||
ds4_button
|
||||
};
|
||||
|
||||
@ -21,7 +21,7 @@ hid_driver_instance_t hid_instance = {0};
|
||||
hid_device_t *hid_device_list[] = {
|
||||
&wiiu_gca_hid_device,
|
||||
&ds3_hid_device,
|
||||
&ds4_hid_device,
|
||||
/* &ds4_hid_device, */
|
||||
NULL /* must be last entry in list */
|
||||
};
|
||||
|
||||
|
||||
@ -15,6 +15,7 @@
|
||||
*/
|
||||
|
||||
#include "wiiu_hid.h"
|
||||
#include <wiiu/os/atomic.h>
|
||||
|
||||
static wiiu_event_list events;
|
||||
static wiiu_adapter_list adapters;
|
||||
@ -377,21 +378,17 @@ static void synchronized_process_adapters(wiiu_hid_t *hid)
|
||||
|
||||
static void synchronized_add_event(wiiu_attach_event *event)
|
||||
{
|
||||
OSFastMutex_Lock(&(events.lock));
|
||||
event->next = events.list;
|
||||
events.list = event;
|
||||
OSFastMutex_Unlock(&(events.lock));
|
||||
wiiu_attach_event *head = (wiiu_attach_event *)SwapAtomic32((uint32_t *)&events.list, 0);
|
||||
|
||||
event->next = head;
|
||||
head = event;
|
||||
|
||||
SwapAtomic32((uint32_t *)&events.list, (uint32_t)head);
|
||||
}
|
||||
|
||||
static wiiu_attach_event *synchronized_get_events_list(void)
|
||||
{
|
||||
wiiu_attach_event *list;
|
||||
OSFastMutex_Lock(&(events.lock));
|
||||
list = events.list;
|
||||
events.list = NULL;
|
||||
OSFastMutex_Unlock(&(events.lock));
|
||||
|
||||
return list;
|
||||
return (wiiu_attach_event *)SwapAtomic32((uint32_t *)&events.list, 0);
|
||||
}
|
||||
|
||||
static wiiu_adapter_t *synchronized_lookup_adapter(uint32_t handle)
|
||||
@ -422,24 +419,24 @@ static int32_t wiiu_attach_callback(HIDClient *client,
|
||||
{
|
||||
wiiu_attach_event *event = NULL;
|
||||
|
||||
switch(attach)
|
||||
{
|
||||
case HID_DEVICE_ATTACH:
|
||||
log_device(device);
|
||||
case HID_DEVICE_DETACH:
|
||||
if (device)
|
||||
event = new_attach_event(device);
|
||||
|
||||
if(!event)
|
||||
goto error;
|
||||
|
||||
event->type = attach;
|
||||
synchronized_add_event(event);
|
||||
return DEVICE_USED;
|
||||
default:
|
||||
break;
|
||||
if(attach) {
|
||||
RARCH_LOG("[hid]: Device attach event generated.\n");
|
||||
log_device(device);
|
||||
} else {
|
||||
RARCH_LOG("[hid]: Device detach event generated.\n");
|
||||
}
|
||||
|
||||
if (device)
|
||||
event = new_attach_event(device);
|
||||
|
||||
if(!event)
|
||||
goto error;
|
||||
|
||||
event->type = attach;
|
||||
synchronized_add_event(event);
|
||||
|
||||
return DEVICE_USED;
|
||||
|
||||
error:
|
||||
delete_attach_event(event);
|
||||
return DEVICE_UNUSED;
|
||||
@ -453,7 +450,7 @@ static void wiiu_hid_detach(wiiu_hid_t *hid, wiiu_attach_event *event)
|
||||
* the read loop method will update this to ADAPTER_STATE_GC
|
||||
* so the adapter poll method can clean it up. */
|
||||
if(adapter)
|
||||
adapter->state = ADAPTER_STATE_DONE;
|
||||
adapter->connected = false;
|
||||
}
|
||||
|
||||
|
||||
@ -479,15 +476,6 @@ error:
|
||||
delete_adapter(adapter);
|
||||
}
|
||||
|
||||
void wiiu_start_read_loop(wiiu_adapter_t *adapter)
|
||||
{
|
||||
HIDRead(adapter->handle,
|
||||
adapter->rx_buffer,
|
||||
adapter->rx_size,
|
||||
wiiu_hid_read_loop_callback,
|
||||
adapter);
|
||||
}
|
||||
|
||||
static void wiiu_hid_read_loop_callback(uint32_t handle, int32_t error,
|
||||
uint8_t *buffer, uint32_t buffer_size, void *userdata)
|
||||
{
|
||||
@ -498,17 +486,49 @@ static void wiiu_hid_read_loop_callback(uint32_t handle, int32_t error,
|
||||
return;
|
||||
}
|
||||
|
||||
if(adapter->hid->polling_thread_quit)
|
||||
if(error < 0)
|
||||
{
|
||||
RARCH_LOG("Shutting down read loop for device: %s\n",
|
||||
adapter->driver->name);
|
||||
adapter->state = ADAPTER_STATE_DONE;
|
||||
int16_t hid_error_code = error & 0xffff;
|
||||
switch(hid_error_code)
|
||||
{
|
||||
case -100:
|
||||
RARCH_ERR("[hid]: Invalid RM command (%s)\n", adapter->driver->name);
|
||||
break;
|
||||
case -102:
|
||||
RARCH_ERR("[hid]: Invalid IOCTL command (%s)\n", adapter->driver->name);
|
||||
break;
|
||||
case -103:
|
||||
RARCH_ERR("[hid]: bad vector count (%s)\n", adapter->driver->name);
|
||||
break;
|
||||
case -104:
|
||||
RARCH_ERR("[hid]: invalid memory bank (%s)\n", adapter->driver->name);
|
||||
break;
|
||||
case -105:
|
||||
RARCH_ERR("[hid]: invalid memory alignment (%s)\n", adapter->driver->name);
|
||||
break;
|
||||
case -106:
|
||||
RARCH_ERR("[hid]: invalid data size (%s)\n", adapter->driver->name);
|
||||
break;
|
||||
case -107:
|
||||
RARCH_ERR("[hid]: request cancelled (%s)\n", adapter->driver->name);
|
||||
break;
|
||||
case -108:
|
||||
RARCH_ERR("[hid]: request timed out (%s)\n", adapter->driver->name);
|
||||
break;
|
||||
case -109:
|
||||
RARCH_ERR("[hid]: request aborted (%s)\n", adapter->driver->name);
|
||||
break;
|
||||
case -110:
|
||||
RARCH_ERR("[hid]: client priority error (%s)\n", adapter->driver->name);
|
||||
break;
|
||||
case -111:
|
||||
RARCH_ERR("[hid]: invalid device handle (%s)\n", adapter->driver->name);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if(adapter->state == ADAPTER_STATE_READY ||
|
||||
adapter->state == ADAPTER_STATE_READING) {
|
||||
|
||||
adapter->state = ADAPTER_STATE_READING;
|
||||
if(adapter->state == ADAPTER_STATE_READING) {
|
||||
adapter->state = ADAPTER_STATE_READY;
|
||||
/* "error" usually is something benign like "device not ready", at
|
||||
* least from my own experiments. Just ignore the error and retry
|
||||
* the read. */
|
||||
@ -517,16 +537,6 @@ static void wiiu_hid_read_loop_callback(uint32_t handle, int32_t error,
|
||||
buffer, buffer_size);
|
||||
}
|
||||
}
|
||||
|
||||
/* this can also get set if something goes wrong in initialization */
|
||||
if(adapter->state == ADAPTER_STATE_DONE)
|
||||
{
|
||||
adapter->state = ADAPTER_STATE_GC;
|
||||
return;
|
||||
}
|
||||
|
||||
HIDRead(adapter->handle, adapter->rx_buffer, adapter->rx_size,
|
||||
wiiu_hid_read_loop_callback, adapter);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -540,17 +550,18 @@ static void wiiu_hid_polling_thread_cleanup(OSThread *thread, void *stack)
|
||||
|
||||
RARCH_LOG("Waiting for in-flight reads to finish.\n");
|
||||
|
||||
/* We don't need to protect the adapter list here because nothing else
|
||||
will access it during this method (the HID system is shut down, and
|
||||
the only other access is the polling thread that just stopped */
|
||||
do
|
||||
{
|
||||
OSFastMutex_Lock(&(adapters.lock));
|
||||
incomplete = 0;
|
||||
for(adapter = adapters.list; adapter != NULL; adapter = adapter->next)
|
||||
{
|
||||
if(adapter->state == ADAPTER_STATE_READING)
|
||||
incomplete++;
|
||||
}
|
||||
/* We are clear for shutdown. Clean up the list
|
||||
* while we are holding the lock. */
|
||||
|
||||
if(incomplete == 0)
|
||||
{
|
||||
RARCH_LOG("All in-flight reads complete.\n");
|
||||
@ -562,7 +573,6 @@ static void wiiu_hid_polling_thread_cleanup(OSThread *thread, void *stack)
|
||||
delete_adapter(adapter);
|
||||
}
|
||||
}
|
||||
OSFastMutex_Unlock(&(adapters.lock));
|
||||
|
||||
if(incomplete)
|
||||
usleep(5000);
|
||||
@ -572,8 +582,7 @@ static void wiiu_hid_polling_thread_cleanup(OSThread *thread, void *stack)
|
||||
RARCH_WARN("[hid]: timed out waiting for in-flight read to finish.\n");
|
||||
incomplete = 0;
|
||||
}
|
||||
}while(incomplete);
|
||||
|
||||
} while(incomplete);
|
||||
}
|
||||
|
||||
static void wiiu_handle_attach_events(wiiu_hid_t *hid, wiiu_attach_event *list)
|
||||
@ -594,7 +603,7 @@ static void wiiu_handle_attach_events(wiiu_hid_t *hid, wiiu_attach_event *list)
|
||||
}
|
||||
}
|
||||
|
||||
static void wiiu_handle_ready_adapters(wiiu_hid_t *hid)
|
||||
static void wiiu_poll_adapters(wiiu_hid_t *hid)
|
||||
{
|
||||
wiiu_adapter_t *it;
|
||||
OSFastMutex_Lock(&(adapters.lock));
|
||||
@ -602,11 +611,25 @@ static void wiiu_handle_ready_adapters(wiiu_hid_t *hid)
|
||||
for(it = adapters.list; it != NULL; it = it->next)
|
||||
{
|
||||
if(it->state == ADAPTER_STATE_READY)
|
||||
wiiu_start_read_loop(it);
|
||||
{
|
||||
if(it->connected)
|
||||
{
|
||||
it->state = ADAPTER_STATE_READING;
|
||||
HIDRead(it->handle,
|
||||
it->rx_buffer,
|
||||
it->rx_size,
|
||||
wiiu_hid_read_loop_callback,
|
||||
it);
|
||||
} else {
|
||||
it->state = ADAPTER_STATE_DONE;
|
||||
}
|
||||
}
|
||||
|
||||
if(it->state == ADAPTER_STATE_DONE)
|
||||
it->state = ADAPTER_STATE_GC;
|
||||
}
|
||||
|
||||
OSFastMutex_Unlock(&(adapters.lock));
|
||||
|
||||
}
|
||||
|
||||
static int wiiu_hid_polling_thread(int argc, const char **argv)
|
||||
@ -618,8 +641,7 @@ static int wiiu_hid_polling_thread(int argc, const char **argv)
|
||||
while(!hid->polling_thread_quit)
|
||||
{
|
||||
wiiu_handle_attach_events(hid, synchronized_get_events_list());
|
||||
wiiu_handle_ready_adapters(hid);
|
||||
usleep(10000);
|
||||
wiiu_poll_adapters(hid);
|
||||
}
|
||||
|
||||
RARCH_LOG("[hid]: polling thread is stopping\n");
|
||||
@ -674,6 +696,17 @@ static void delete_hidclient(HIDClient *client)
|
||||
free(client);
|
||||
}
|
||||
|
||||
static void init_cachealigned_buffer(int32_t min_size, uint8_t **out_buf_ptr, int32_t *actual_size)
|
||||
{
|
||||
int cacheblocks = (min_size < 32) ? 1 : min_size / 32;
|
||||
if(min_size > 32 && min_size % 32 != 0)
|
||||
cacheblocks++;
|
||||
|
||||
*actual_size = 32 * cacheblocks;
|
||||
|
||||
*out_buf_ptr = alloc_zeroed(32, *actual_size);
|
||||
}
|
||||
|
||||
static wiiu_adapter_t *new_adapter(wiiu_attach_event *event)
|
||||
{
|
||||
wiiu_adapter_t *adapter = alloc_zeroed(32, sizeof(wiiu_adapter_t));
|
||||
@ -683,10 +716,9 @@ static wiiu_adapter_t *new_adapter(wiiu_attach_event *event)
|
||||
|
||||
adapter->handle = event->handle;
|
||||
adapter->interface_index = event->interface_index;
|
||||
adapter->rx_size = event->max_packet_size_rx;
|
||||
adapter->rx_buffer = alloc_zeroed(32, adapter->rx_size);
|
||||
adapter->tx_size = event->max_packet_size_tx;
|
||||
adapter->tx_buffer = alloc_zeroed(32, adapter->tx_size);
|
||||
init_cachealigned_buffer(event->max_packet_size_rx, &adapter->rx_buffer, &adapter->rx_size);
|
||||
init_cachealigned_buffer(event->max_packet_size_tx, &adapter->tx_buffer, &adapter->tx_size);
|
||||
adapter->connected = true;
|
||||
|
||||
return adapter;
|
||||
}
|
||||
|
||||
@ -62,6 +62,7 @@ struct wiiu_adapter {
|
||||
int32_t tx_size;
|
||||
uint32_t handle;
|
||||
uint8_t interface_index;
|
||||
bool connected;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
Loading…
Reference in New Issue
Block a user