From f7a630d6142edc1bc575bb403a0fffc2d924b7f8 Mon Sep 17 00:00:00 2001
From: Mathias Agopian <mathias@google.com>
Date: Fri, 1 Oct 2021 11:13:41 -0700
Subject: [PATCH] Use a dedicated thread for dispatching callbacks

This isolates our backend thread from user code and the java VM.
---
 filament/backend/src/Driver.cpp   | 37 ++++++++++++++++++++++++++++---
 filament/backend/src/DriverBase.h |  9 ++++++++
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/filament/backend/src/Driver.cpp b/filament/backend/src/Driver.cpp
index 3e0bcf4010..09f651a537 100644
--- a/filament/backend/src/Driver.cpp
+++ b/filament/backend/src/Driver.cpp
@@ -39,9 +39,40 @@ namespace backend {
 
 DriverBase::DriverBase(Dispatcher* dispatcher) noexcept
         : mDispatcher(dispatcher) {
+
+    // This thread services user callbacks
+    mServiceThread = std::thread([this]() {
+        do {
+            auto& serviceThreadCondition = mServiceThreadCondition;
+            auto& serviceThreadCallbackQueue = mServiceThreadCallbackQueue;
+
+            // wait for some callbacks to dispatch
+            std::unique_lock<std::mutex> lock(mServiceThreadLock);
+            while (serviceThreadCallbackQueue.empty() && !mExitRequested) {
+                serviceThreadCondition.wait(lock);
+            }
+            if (mExitRequested) {
+                break;
+            }
+            // move the callbacks to a temporary vector
+            auto callbacks(std::move(serviceThreadCallbackQueue));
+            lock.unlock();
+            // and make sure to call them without our lock held
+            for (auto [handler, callback, user] : callbacks) {
+                handler->post(user, callback);
+            }
+        } while (true);
+    });
 }
 
 DriverBase::~DriverBase() noexcept {
+    // quit our service thread
+    std::unique_lock<std::mutex> lock(mServiceThreadLock);
+    mExitRequested = true;
+    mServiceThreadCondition.notify_one();
+    lock.unlock();
+    mServiceThread.join();
+
     delete mDispatcher;
 }
 
@@ -67,9 +98,9 @@ void DriverBase::CallbackData::release(CallbackData* data) {
 
 void DriverBase::scheduleCallback(CallbackHandler* handler, void* user, CallbackHandler::Callback callback) {
     if (handler) {
-        // TODO: since we're calling user-code here, we might want to do this from a
-        //       dedicated thread, so they can't mess with our GL state for instance.
-        handler->post(user, callback);
+        std::lock_guard<std::mutex> lock(mServiceThreadLock);
+        mServiceThreadCallbackQueue.emplace_back(handler, callback, user);
+        mServiceThreadCondition.notify_one();
     } else {
         std::lock_guard<std::mutex> lock(mPurgeLock);
         mCallbacks.emplace_back(user, callback);
diff --git a/filament/backend/src/DriverBase.h b/filament/backend/src/DriverBase.h
index c540c8e621..28819bb094 100644
--- a/filament/backend/src/DriverBase.h
+++ b/filament/backend/src/DriverBase.h
@@ -30,9 +30,12 @@
 #include "private/backend/SamplerGroup.h"
 
 #include <array>
+#include <condition_variable>
 #include <memory>
 #include <mutex>
+#include <thread>
 #include <utility>
+#include <vector>
 
 #include <stdint.h>
 
@@ -231,6 +234,12 @@ protected:
 private:
     std::mutex mPurgeLock;
     std::vector<std::pair<void*, CallbackHandler::Callback>> mCallbacks;
+
+    std::thread mServiceThread;
+    std::mutex mServiceThreadLock;
+    std::condition_variable mServiceThreadCondition;
+    std::vector<std::tuple<CallbackHandler*, CallbackHandler::Callback, void*>> mServiceThreadCallbackQueue;
+    bool mExitRequested = false;
 };