The open source OpenXR runtime
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

st/oxr: Add synchronization primitive that handles session-running-status and blocking of wait frame

rpavlik: Fixes, rename
rpavlik: Add is_session_running

Co-authored-by: Rylie Pavlik <rylie.pavlik@collabora.com>
Part-of: <https://gitlab.freedesktop.org/monado/monado/-/merge_requests/2344>

authored by

Yulou Liu
Rylie Pavlik
and committed by
Marge Bot
3b232b74 ce3a0585

+298 -22
+5
doc/changes/state_trackers/mr.1934.md
··· 1 + --- 2 + - mr.2344 3 + --- 4 + - Fix possible deadlock from unanswered `xrWaitFrame`. 5 + - Fix conformance issues related to session state and session running/non-running.
+2
src/xrt/state_trackers/oxr/CMakeLists.txt
··· 32 32 oxr_dpad.c 33 33 oxr_event.c 34 34 oxr_extension_support.h 35 + oxr_frame_sync.c 36 + oxr_frame_sync.h 35 37 oxr_handle_base.c 36 38 oxr_input.c 37 39 oxr_input_transform.c
+3 -1
src/xrt/state_trackers/oxr/oxr_api_session.c
··· 17 17 #include "util/u_debug.h" 18 18 #include "util/u_trace_marker.h" 19 19 20 + #include "oxr_frame_sync.h" 20 21 #include "oxr_objects.h" 21 22 #include "oxr_logger.h" 22 23 #include "oxr_two_call.h" ··· 99 100 OXR_VERIFY_VIEW_CONFIG_TYPE(&log, sess->sys->inst, beginInfo->primaryViewConfigurationType); 100 101 } 101 102 102 - if (sess->has_begun) { 103 + // Going to effectively double check this, but this gives us an early out. 104 + if (oxr_frame_sync_is_session_running(&sess->frame_sync)) { 103 105 return oxr_error(&log, XR_ERROR_SESSION_RUNNING, "Session is already running"); 104 106 } 105 107
+3 -1
src/xrt/state_trackers/oxr/oxr_api_verify.h
··· 10 10 11 11 #pragma once 12 12 13 + #include "oxr_frame_sync.h" // iwyu pragma: keep 14 + 13 15 #ifdef __cplusplus 14 16 extern "C" { 15 17 #endif ··· 255 257 256 258 #define OXR_VERIFY_SESSION_RUNNING(log, sess) \ 257 259 do { \ 258 - if (!sess->has_begun) { \ 260 + if (!oxr_frame_sync_is_session_running(&sess->frame_sync)) { \ 259 261 return oxr_error(log, XR_ERROR_SESSION_NOT_RUNNING, "Session is not running"); \ 260 262 } \ 261 263 } while (false)
+144
src/xrt/state_trackers/oxr/oxr_frame_sync.c
··· 1 + // Copyright 2024, Collabora, Ltd. 2 + // Copyright 2024, QUALCOMM CORPORATION. 3 + // SPDX-License-Identifier: BSL-1.0 4 + /*! 5 + * @file 6 + * @brief The objects that handle session running status and blocking of xrWaitFrame. 7 + * @author Jakob Bornecrantz <jakob@collabora.com> 8 + * @author Rylie Pavlik <rylie.pavlik@collabora.com> 9 + * @author Yulou Liu <quic_yuloliu@quicinc.com> 10 + * @ingroup oxr_main 11 + */ 12 + 13 + #include "oxr_frame_sync.h" 14 + 15 + #include <util/u_misc.h> 16 + 17 + int 18 + oxr_frame_sync_init(struct oxr_frame_sync *ofs) 19 + { 20 + U_ZERO(ofs); 21 + 22 + int ret = pthread_mutex_init(&ofs->mutex, NULL); 23 + if (ret != 0) { 24 + return ret; 25 + } 26 + 27 + ret = pthread_cond_init(&ofs->cond, NULL); 28 + if (ret) { 29 + pthread_mutex_destroy(&ofs->mutex); 30 + return ret; 31 + } 32 + ofs->canWaitFrameReturn = 0; 33 + ofs->initialized = true; 34 + ofs->running = false; 35 + 36 + return 0; 37 + } 38 + 39 + XRT_CHECK_RESULT XrResult 40 + oxr_frame_sync_wait_frame(struct oxr_frame_sync *ofs) 41 + { 42 + pthread_mutex_lock(&ofs->mutex); 43 + while (ofs->running) { 44 + if (1 == ofs->canWaitFrameReturn) { 45 + ofs->canWaitFrameReturn = 0; 46 + break; 47 + } else if (0 == ofs->canWaitFrameReturn) { 48 + pthread_cond_wait(&ofs->cond, &ofs->mutex); 49 + continue; 50 + } else { 51 + // we are not suppose to be here 52 + // if canWaitFrameReturn is neither 1 nor 0 53 + // something goes wrong while session is running 54 + pthread_mutex_unlock(&ofs->mutex); 55 + return XR_ERROR_SESSION_NOT_RUNNING; 56 + } 57 + } 58 + if (ofs->running) { 59 + pthread_mutex_unlock(&ofs->mutex); 60 + return XR_SUCCESS; 61 + } 62 + pthread_mutex_unlock(&ofs->mutex); 63 + return XR_ERROR_SESSION_NOT_RUNNING; 64 + } 65 + 66 + XRT_CHECK_RESULT XrResult 67 + oxr_frame_sync_release(struct oxr_frame_sync *ofs) 68 + { 69 + pthread_mutex_lock(&ofs->mutex); 70 + if (ofs->running) { 71 + if (0 == ofs->canWaitFrameReturn) { 72 + ofs->canWaitFrameReturn = 1; 73 + pthread_cond_signal(&ofs->cond); 74 + pthread_mutex_unlock(&ofs->mutex); 75 + return XR_SUCCESS; 76 + } 77 + } 78 + pthread_mutex_unlock(&ofs->mutex); 79 + return XR_ERROR_SESSION_NOT_RUNNING; 80 + } 81 + 82 + XRT_CHECK_RESULT XrResult 83 + oxr_frame_sync_begin_session(struct oxr_frame_sync *ofs) 84 + { 85 + pthread_mutex_lock(&ofs->mutex); 86 + if (!ofs->running) { 87 + ofs->canWaitFrameReturn = 1; 88 + ofs->running = true; 89 + pthread_cond_signal(&ofs->cond); 90 + pthread_mutex_unlock(&ofs->mutex); 91 + return XR_SUCCESS; 92 + } 93 + pthread_mutex_unlock(&ofs->mutex); 94 + return XR_ERROR_SESSION_NOT_RUNNING; 95 + } 96 + 97 + XRT_CHECK_RESULT XrResult 98 + oxr_frame_sync_end_session(struct oxr_frame_sync *ofs) 99 + { 100 + pthread_mutex_lock(&ofs->mutex); 101 + if (ofs->running) { 102 + ofs->running = false; 103 + pthread_cond_signal(&ofs->cond); 104 + pthread_mutex_unlock(&ofs->mutex); 105 + return XR_SUCCESS; 106 + } 107 + pthread_mutex_unlock(&ofs->mutex); 108 + return XR_ERROR_SESSION_NOT_RUNNING; 109 + } 110 + 111 + XRT_CHECK_RESULT bool 112 + oxr_frame_sync_is_session_running(struct oxr_frame_sync *ofs) 113 + { 114 + pthread_mutex_lock(&ofs->mutex); 115 + bool ret = ofs->running; 116 + pthread_mutex_unlock(&ofs->mutex); 117 + return ret; 118 + } 119 + 120 + void 121 + oxr_frame_sync_fini(struct oxr_frame_sync *ofs) 122 + { 123 + // The fields are protected. 124 + pthread_mutex_lock(&ofs->mutex); 125 + assert(ofs->initialized); 126 + 127 + if (ofs->running) { 128 + // Stop the thread. 129 + ofs->running = false; 130 + // Wake up the thread if it is waiting. 131 + pthread_cond_signal(&ofs->cond); 132 + } 133 + 134 + ofs->canWaitFrameReturn = 0; 135 + ofs->running = false; 136 + 137 + // No longer need to protect fields. 138 + pthread_mutex_unlock(&ofs->mutex); 139 + 140 + // Destroy resources. 141 + pthread_mutex_destroy(&ofs->mutex); 142 + pthread_cond_destroy(&ofs->cond); 143 + ofs->initialized = false; 144 + }
+103
src/xrt/state_trackers/oxr/oxr_frame_sync.h
··· 1 + // Copyright 2024, Collabora, Ltd. 2 + // Copyright 2024, QUALCOMM CORPORATION. 3 + // SPDX-License-Identifier: BSL-1.0 4 + /*! 5 + * @file 6 + * @brief The objects that handle session running status and blocking of xrWaitFrame. 7 + * @author Jakob Bornecrantz <jakob@collabora.com> 8 + * @author Rylie Pavlik <rylie.pavlik@collabora.com> 9 + * @author Yulou Liu <quic_yuloliu@quicinc.com> 10 + * @ingroup oxr_main 11 + */ 12 + 13 + #pragma once 14 + 15 + #include "xrt/xrt_compiler.h" 16 + #include "xrt/xrt_config_os.h" 17 + #include "xrt/xrt_openxr_includes.h" 18 + 19 + #include <stdbool.h> 20 + 21 + #if defined(XRT_OS_LINUX) || defined(XRT_OS_WINDOWS) 22 + #include <pthread.h> 23 + #include <assert.h> 24 + #else 25 + #error "OS not supported" 26 + #endif 27 + 28 + #ifdef __cplusplus 29 + extern "C" { 30 + #endif 31 + 32 + /*! 33 + * Helper that handles synchronizing the xr{Wait,Begin,End}Frame calls. 34 + */ 35 + struct oxr_frame_sync 36 + { 37 + pthread_mutex_t mutex; 38 + pthread_cond_t cond; 39 + 40 + bool canWaitFrameReturn; 41 + bool initialized; 42 + bool running; 43 + }; 44 + 45 + /*! 46 + * Initialize the frame sync helper. 47 + * 48 + * @public @memberof oxr_frame_sync 49 + */ 50 + int 51 + oxr_frame_sync_init(struct oxr_frame_sync *ofs); 52 + 53 + /*! 54 + * Handle mutual exclusion in xrWaitFrame w.r.t. xrBeginFrame 55 + * 56 + * @public @memberof oxr_frame_sync 57 + */ 58 + XRT_CHECK_RESULT XrResult 59 + oxr_frame_sync_wait_frame(struct oxr_frame_sync *ofs); 60 + 61 + /*! 62 + * Release at most one blocked xrWaitFrame to run, e.g. from xrBeginFrame. 63 + * 64 + * @public @memberof oxr_frame_sync 65 + */ 66 + XRT_CHECK_RESULT XrResult 67 + oxr_frame_sync_release(struct oxr_frame_sync *ofs); 68 + 69 + /*! 70 + * Begin the session, resetting state accordingly. 71 + * 72 + * @public @memberof oxr_frame_sync 73 + */ 74 + XRT_CHECK_RESULT XrResult 75 + oxr_frame_sync_begin_session(struct oxr_frame_sync *ofs); 76 + 77 + /*! 78 + * End the session 79 + * 80 + * @public @memberof oxr_frame_sync 81 + */ 82 + XRT_CHECK_RESULT XrResult 83 + oxr_frame_sync_end_session(struct oxr_frame_sync *ofs); 84 + 85 + /*! 86 + * Is the session running?. 87 + * 88 + * @public @memberof oxr_frame_sync 89 + */ 90 + XRT_CHECK_RESULT bool 91 + oxr_frame_sync_is_session_running(struct oxr_frame_sync *ofs); 92 + 93 + /*! 94 + * Clean up. 95 + * 96 + * @public @memberof oxr_frame_sync 97 + */ 98 + void 99 + oxr_frame_sync_fini(struct oxr_frame_sync *ofs); 100 + 101 + #ifdef __cplusplus 102 + } // extern "C" 103 + #endif
+3 -2
src/xrt/state_trackers/oxr/oxr_objects.h
··· 32 32 #include "oxr_extension_support.h" 33 33 #include "oxr_subaction.h" 34 34 #include "oxr_defines.h" 35 + #include "oxr_frame_sync.h" 35 36 36 37 #if defined(XRT_HAVE_D3D11) || defined(XRT_HAVE_D3D12) 37 38 #include <dxgi.h> ··· 1758 1759 struct oxr_session *next; 1759 1760 1760 1761 XrSessionState state; 1761 - bool has_begun; 1762 + 1762 1763 /*! 1763 1764 * There is a extra state between xrBeginSession has been called and 1764 1765 * the first xrEndFrame has been called. These are to track this. ··· 1782 1783 int64_t begun; 1783 1784 } frame_id; 1784 1785 1785 - struct os_semaphore sem; 1786 + struct oxr_frame_sync frame_sync; 1786 1787 1787 1788 /*! 1788 1789 * Used to implement precise extra sleeping in wait frame.
+35 -18
src/xrt/state_trackers/oxr/oxr_session.c
··· 9 9 * @ingroup oxr_main 10 10 */ 11 11 12 + #include "oxr_frame_sync.h" 12 13 #include "xrt/xrt_device.h" 13 14 #include "xrt/xrt_session.h" 14 15 #include "xrt/xrt_config_build.h" // IWYU pragma: keep ··· 263 264 oxr_session_change_state(log, sess, XR_SESSION_STATE_VISIBLE, 0); 264 265 oxr_session_change_state(log, sess, XR_SESSION_STATE_FOCUSED, 0); 265 266 } 266 - 267 - sess->has_begun = true; 267 + XrResult ret = oxr_frame_sync_begin_session(&sess->frame_sync); 268 + if (ret != XR_SUCCESS) { 269 + return oxr_error(log, ret, 270 + "Frame sync object refused to let us begin session, probably already running"); 271 + } 268 272 269 273 return oxr_session_success_result(sess); 270 274 } ··· 326 330 oxr_session_change_state(log, sess, XR_SESSION_STATE_READY, 0); 327 331 #endif // !XRT_OS_ANDROID 328 332 } 329 - 330 - sess->has_begun = false; 333 + XrResult ret = oxr_frame_sync_end_session(&sess->frame_sync); 334 + if (ret != XR_SUCCESS) { 335 + return oxr_error(log, ret, "Frame sync object refused to let us end session, probably not running"); 336 + } 331 337 sess->has_ended_once = false; 332 338 333 339 return oxr_session_success_result(sess); ··· 791 797 * multiple threads. We do this before so we call predicted after any 792 798 * waiting for xrBeginFrame has happened, for better timing information. 793 799 */ 794 - os_semaphore_wait(&sess->sem, 0); 800 + XrResult ret = oxr_frame_sync_wait_frame(&sess->frame_sync); 801 + if (XR_SUCCESS != ret) { 802 + // session not running 803 + return ret; 804 + } 795 805 796 806 if (sess->frame_timing_spew) { 797 807 oxr_log(log, "Finished waiting for previous frame begin at %8.3fms", ts_ms(sess)); ··· 802 812 int64_t predicted_display_period = 0; 803 813 XrTime converted_time = 0; 804 814 805 - XrResult ret = do_wait_frame_and_checks( // 806 - log, // log 807 - sess, // sess 808 - &frame_id, // out_frame_id 809 - &predicted_display_time, // out_predicted_display_time 810 - &predicted_display_period, // out_predicted_display_period 811 - &converted_time); // out_converted_time 815 + ret = do_wait_frame_and_checks( // 816 + log, // log 817 + sess, // sess 818 + &frame_id, // out_frame_id 819 + &predicted_display_time, // out_predicted_display_time 820 + &predicted_display_period, // out_predicted_display_period 821 + &converted_time); // out_converted_time 812 822 if (ret != XR_SUCCESS) { 813 823 // On error we need to release the semaphore ourselves as xrBeginFrame won't do it. 814 - os_semaphore_release(&sess->sem); 815 - 824 + // Should not get an error. 825 + XrResult release_ret = oxr_frame_sync_release(&sess->frame_sync); 826 + assert(release_ret == XR_SUCCESS); 827 + (void)release_ret; 816 828 // Error already logged. 817 829 return ret; 818 830 } ··· 894 906 sess->frame_id.waited = -1; 895 907 } 896 908 897 - os_semaphore_release(&sess->sem); 909 + // beginFrame is about to succeed, we can release an xrWaitFrame, if available. 910 + XrResult osh_ret = oxr_frame_sync_release(&sess->frame_sync); 911 + if (XR_SUCCESS != osh_ret) { 912 + // session not running 913 + return osh_ret; 914 + } 898 915 899 916 return ret; 900 917 } ··· 927 944 xrt_session_destroy(&sess->xs); 928 945 929 946 os_precise_sleeper_deinit(&sess->sleeper); 930 - os_semaphore_destroy(&sess->sem); 947 + oxr_frame_sync_fini(&sess->frame_sync); 931 948 os_mutex_destroy(&sess->active_wait_frames_lock); 932 949 933 950 free(sess); ··· 950 967 // What system is this session based on. 951 968 sess->sys = sys; 952 969 953 - // Init the begin/wait frame semaphore and related fields. 954 - os_semaphore_init(&sess->sem, 1); 970 + // Init the begin/wait frame handler. 971 + oxr_frame_sync_init(&sess->frame_sync); 955 972 956 973 // Init the wait frame precise sleeper. 957 974 os_precise_sleeper_init(&sess->sleeper);