From 8e6aa05f76b4d547a65f7a686c7b60ba27f9de39 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 30 Apr 2025 21:18:02 -0700 Subject: [PATCH] GUACAMOLE-377: Correct race condition in guac_display flush planning. The actual addition of operations to the ops FIFO has to happen atomically, or else an earlier frame's operations may end up in the FIFO _after_ a later frame's operations. --- src/libguac/display-flush.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/libguac/display-flush.c b/src/libguac/display-flush.c index 10f9667c..4816576f 100644 --- a/src/libguac/display-flush.c +++ b/src/libguac/display-flush.c @@ -376,6 +376,8 @@ void guac_display_end_multiple_frames(guac_display* display, int frames) { frame_nonempty = PFW_LFW_guac_display_frame_complete(display); GUAC_DISPLAY_PLAN_END_PHASE(display, "commit", 5, 5); + guac_rwlock_release_lock(&display->last_frame.lock); + /* Not all frames are graphical. If we end up with a frame containing * nothing but layer property changes, then we must still send a frame * boundary even though there is no display plan to optimize. */ @@ -385,15 +387,12 @@ void guac_display_end_multiple_frames(guac_display* display, int frames) { }; guac_fifo_enqueue(&display->ops, &end_frame_op); } - - guac_rwlock_release_lock(&display->last_frame.lock); - -finished_with_pending_frame_lock: - guac_rwlock_release_lock(&display->pending_frame.lock); - - if (plan != NULL) { + else if (plan != NULL) { guac_display_plan_apply(plan); guac_display_plan_free(plan); } +finished_with_pending_frame_lock: + guac_rwlock_release_lock(&display->pending_frame.lock); + }