From b3523a72596e9c31c8216c6c73d807d85c4b3032 Mon Sep 17 00:00:00 2001 From: Jakob Bornecrantz Date: Sat, 30 May 2020 13:56:23 +0100 Subject: [PATCH] st/oxr: Protoct event queue with a mutex --- doc/changes/state_trackers/mr.359.4.md | 2 + src/xrt/state_trackers/oxr/oxr_event.c | 60 ++++++++++++++++------- src/xrt/state_trackers/oxr/oxr_instance.c | 15 ++++-- src/xrt/state_trackers/oxr/oxr_objects.h | 12 ++++- 4 files changed, 66 insertions(+), 23 deletions(-) create mode 100644 doc/changes/state_trackers/mr.359.4.md diff --git a/doc/changes/state_trackers/mr.359.4.md b/doc/changes/state_trackers/mr.359.4.md new file mode 100644 index 000000000..b6a066507 --- /dev/null +++ b/doc/changes/state_trackers/mr.359.4.md @@ -0,0 +1,2 @@ +OpenXR: Make the event queue thread safe, all done with a simple mutex that is +not held for long at all. diff --git a/src/xrt/state_trackers/oxr/oxr_event.c b/src/xrt/state_trackers/oxr/oxr_event.c index 759a1785d..85f6efbb8 100644 --- a/src/xrt/state_trackers/oxr/oxr_event.c +++ b/src/xrt/state_trackers/oxr/oxr_event.c @@ -7,17 +7,24 @@ * @ingroup oxr_main */ -#include -#include -#include +#include "os/os_threading.h" #include "util/u_misc.h" #include "oxr_objects.h" #include "oxr_logger.h" +#include +#include +#include +/* + * + * Struct and defines. + * + */ + struct oxr_event { struct oxr_event *next; @@ -26,13 +33,23 @@ struct oxr_event }; +/* + * + * Internal helpers. + * + */ + void lock(struct oxr_instance *inst) -{} +{ + os_mutex_lock(&inst->event.mutex); +} void unlock(struct oxr_instance *inst) -{} +{ + os_mutex_unlock(&inst->event.mutex); +} void * oxr_event_extra(struct oxr_event *event) @@ -43,16 +60,16 @@ oxr_event_extra(struct oxr_event *event) struct oxr_event * pop(struct oxr_instance *inst) { - struct oxr_event *ret = inst->next_event; + struct oxr_event *ret = inst->event.next; if (ret == NULL) { return NULL; } - inst->next_event = ret->next; + inst->event.next = ret->next; ret->next = NULL; - if (ret == inst->last_event) { - inst->last_event = NULL; + if (ret == inst->event.last) { + inst->event.last = NULL; } return ret; @@ -61,14 +78,14 @@ pop(struct oxr_instance *inst) void push(struct oxr_instance *inst, struct oxr_event *event) { - struct oxr_event *last = inst->last_event; + struct oxr_event *last = inst->event.last; if (last != NULL) { last->next = event; } - inst->last_event = event; + inst->event.last = event; - if (inst->next_event == NULL) { - inst->next_event = event; + if (inst->event.next == NULL) { + inst->event.next = event; } } @@ -105,6 +122,13 @@ oxr_event_alloc(struct oxr_logger *log, return XR_SUCCESS; } + +/* + * + * 'Exported' functions. + * + */ + XrResult oxr_event_push_XrEventDataSessionStateChanged(struct oxr_logger *log, struct oxr_session *sess, @@ -142,7 +166,7 @@ oxr_event_remove_session_events(struct oxr_logger *log, lock(inst); - struct oxr_event *e = inst->next_event; + struct oxr_event *e = inst->event.next; while (e != NULL) { struct oxr_event *cur = e; e = e->next; @@ -156,12 +180,12 @@ oxr_event_remove_session_events(struct oxr_logger *log, continue; } - if (cur == inst->next_event) { - inst->next_event = cur->next; + if (cur == inst->event.next) { + inst->event.next = cur->next; } - if (cur == inst->last_event) { - inst->last_event = NULL; + if (cur == inst->event.last) { + inst->event.last = NULL; } free(cur); } diff --git a/src/xrt/state_trackers/oxr/oxr_instance.c b/src/xrt/state_trackers/oxr/oxr_instance.c index 572ec2d45..21b749248 100644 --- a/src/xrt/state_trackers/oxr/oxr_instance.c +++ b/src/xrt/state_trackers/oxr/oxr_instance.c @@ -76,6 +76,9 @@ oxr_instance_destroy(struct oxr_logger *log, struct oxr_handle_base *hb) // Does null checking and sets to null. time_state_destroy(&inst->timekeeping); + // Mutex goes last. + os_mutex_destroy(&inst->event.mutex); + free(inst); return XR_SUCCESS; @@ -99,7 +102,7 @@ oxr_instance_create(struct oxr_logger *log, { struct oxr_instance *inst = NULL; struct xrt_device *xdevs[NUM_XDEVS] = {0}; - int xinst_ret; + int xinst_ret, m_ret; XrResult ret; OXR_ALLOCATE_HANDLE_OR_RETURN(log, inst, OXR_XR_DEBUG_INSTANCE, @@ -110,14 +113,20 @@ oxr_instance_create(struct oxr_logger *log, inst->debug_views = debug_get_bool_option_debug_views(); inst->debug_bindings = debug_get_bool_option_debug_bindings(); + m_ret = os_mutex_init(&inst->event.mutex); + if (m_ret < 0) { + ret = oxr_error(log, XR_ERROR_RUNTIME_FAILURE, + "Failed to init mutex"); + return ret; + } + /* ---- HACK ---- */ oxr_sdl2_hack_create(&inst->hack); /* ---- HACK ---- */ ret = oxr_path_init(log, inst); if (ret != XR_SUCCESS) { - free(inst); - return 0; + return ret; } // Cache certain often looked up paths. diff --git a/src/xrt/state_trackers/oxr/oxr_objects.h b/src/xrt/state_trackers/oxr/oxr_objects.h index 17e6dd941..e0953a483 100644 --- a/src/xrt/state_trackers/oxr/oxr_objects.h +++ b/src/xrt/state_trackers/oxr/oxr_objects.h @@ -14,11 +14,15 @@ #include "xrt/xrt_compositor.h" #include "xrt/xrt_vulkan_includes.h" #include "xrt/xrt_openxr_includes.h" + +#include "os/os_threading.h" + #include "util/u_hashset.h" #include "util/u_hashmap.h" #include "oxr_extension_support.h" + #ifdef __cplusplus extern "C" { #endif @@ -965,8 +969,12 @@ struct oxr_instance size_t path_num; // Event queue. - struct oxr_event *last_event; - struct oxr_event *next_event; + struct + { + struct os_mutex mutex; + struct oxr_event *last; + struct oxr_event *next; + } event; struct oxr_interaction_profile **profiles; size_t num_profiles;