From 81ba9ec54a8ed1dfd10386a6afb7b3555c244d56 Mon Sep 17 00:00:00 2001
From: Saran Tunyasuvunakool <stunya@google.com>
Date: Thu, 16 Mar 2023 11:05:38 -0700
Subject: [PATCH] Fix a rare crash when running simulate on macOS.

The bug was introduced in 2f0fb1e4ef81ae4eb281e09fb67c6b36f27dc427. The glfwSwapBuffers call should occur on the main UI thread, not the CoreVideo DisplayLink thread.

PiperOrigin-RevId: 517172202
Change-Id: I2491f2194765019748198cd0cc61ca9263331b01
---
 simulate/glfw_adapter.cc   |  8 ++------
 simulate/glfw_corevideo.h  |  7 ++++---
 simulate/glfw_corevideo.mm | 17 +++++------------
 3 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/simulate/glfw_adapter.cc b/simulate/glfw_adapter.cc
index 9bcbfe5d..3ad7da02 100644
--- a/simulate/glfw_adapter.cc
+++ b/simulate/glfw_adapter.cc
@@ -175,14 +175,10 @@ bool GlfwAdapter::ShouldCloseWindow() const {
 void GlfwAdapter::SwapBuffers() {
 #ifdef __APPLE__
   if (core_video_.has_value()) {
-    core_video_->EnqueueSwap();
-    core_video_->WaitForSwap();
-  } else {
-    Glfw().glfwSwapBuffers(window_);
+    core_video_->WaitForDisplayRefresh();
   }
-#else
-  Glfw().glfwSwapBuffers(window_);
 #endif
+  Glfw().glfwSwapBuffers(window_);
 }
 
 void GlfwAdapter::ToggleFullscreen() {
diff --git a/simulate/glfw_corevideo.h b/simulate/glfw_corevideo.h
index 5b6cb125..d855ff31 100644
--- a/simulate/glfw_corevideo.h
+++ b/simulate/glfw_corevideo.h
@@ -19,6 +19,7 @@
 #error "This header only works on macOS."
 #endif
 
+#include <atomic>
 #include <condition_variable>
 #include <mutex>
 
@@ -37,8 +38,8 @@ class GlfwCoreVideo {
  public:
   GlfwCoreVideo(GLFWwindow* window);
   ~GlfwCoreVideo();
-  void EnqueueSwap();
-  void WaitForSwap();
+
+  void WaitForDisplayRefresh();
   int DisplayLinkCallback();
   void UpdateDisplayLink();
 
@@ -46,7 +47,7 @@ class GlfwCoreVideo {
   GLFWwindow* window_;
   CVDisplayLinkRef display_link_;
 
-  bool second_buffer_has_content_;
+  std::atomic_bool waiting_;
   std::mutex mu_;
   std::condition_variable cond_;
 };
diff --git a/simulate/glfw_corevideo.mm b/simulate/glfw_corevideo.mm
index 4780384b..453d16f9 100644
--- a/simulate/glfw_corevideo.mm
+++ b/simulate/glfw_corevideo.mm
@@ -45,23 +45,16 @@ GlfwCoreVideo::~GlfwCoreVideo() {
   CVDisplayLinkRelease(display_link_);
 }
 
-void GlfwCoreVideo::EnqueueSwap() {
+void GlfwCoreVideo::WaitForDisplayRefresh() {
   std::unique_lock lock(mu_);
-  second_buffer_has_content_ = true;
-}
-
-void GlfwCoreVideo::WaitForSwap() {
-  if (second_buffer_has_content_) {
-    std::unique_lock lock(mu_);
-    cond_.wait(lock, [this]() { return !this->second_buffer_has_content_; });
-  }
+  waiting_.store(true);
+  cond_.wait(lock, [this]() { return !this->waiting_; });
 }
 
 int GlfwCoreVideo::DisplayLinkCallback() {
-  if (second_buffer_has_content_) {
+  if (waiting_.load()) {
     std::unique_lock lock(mu_);
-    Glfw().glfwSwapBuffers(window_);
-    second_buffer_has_content_ = false;
+    waiting_.store(false);
     cond_.notify_one();
   }
   return kCVReturnSuccess;