From 831e3b4dcd73f244cd0863fa4cecd461bd093da2 Mon Sep 17 00:00:00 2001
From: songruining <kevinsong@lilith.com>
Date: Fri, 6 Dec 2024 11:40:00 +0800
Subject: [PATCH] refactor: execute eval in the same thread as lua

---
 debugger/src/internal/breakpoint.cpp    |  2 +-
 debugger/src/internal/debug_bridge.cpp  | 67 ++++++++++++++++++-------
 debugger/src/internal/debug_bridge.h    |  5 +-
 debugger/src/internal/utils/dap_utils.h |  5 +-
 debugger/src/internal/variable.cpp      |  4 +-
 5 files changed, 58 insertions(+), 25 deletions(-)

diff --git a/debugger/src/internal/breakpoint.cpp b/debugger/src/internal/breakpoint.cpp
index b8951d5..3d0d275 100644
--- a/debugger/src/internal/breakpoint.cpp
+++ b/debugger/src/internal/breakpoint.cpp
@@ -3,7 +3,7 @@
 #include <internal/breakpoint.h>
 #include <internal/log.h>
 #include <internal/utils.h>
-#include "internal/utils/lua_utils.h"
+#include <internal/utils/lua_utils.h>
 
 namespace luau::debugger {
 
diff --git a/debugger/src/internal/debug_bridge.cpp b/debugger/src/internal/debug_bridge.cpp
index 77530c9..90a5f09 100644
--- a/debugger/src/internal/debug_bridge.cpp
+++ b/debugger/src/internal/debug_bridge.cpp
@@ -99,10 +99,7 @@ void DebugBridge::onDebugBreak(lua_State* L,
 
   session_->send(event);
 
-  break_vm_ = L;
-  resume_ = false;
-  resume_cv_.wait(lock, [this] { return resume_; });
-  break_vm_ = nullptr;
+  mainThreadWait(L, lock);
 }
 
 std::string DebugBridge::stopReasonToString(BreakReason reason) const {
@@ -408,9 +405,12 @@ void DebugBridge::enableDebugStep(bool enable) {
 }
 
 void DebugBridge::resumeInternal() {
-  std::unique_lock<std::mutex> lock(break_mutex_);
-  DEBUGGER_LOG_INFO("[resume] Resume execution");
-  resume_ = true;
+  {
+    std::unique_lock<std::mutex> lock(break_mutex_);
+    DEBUGGER_LOG_INFO("[resume] Resume execution");
+    resume_ = true;
+  }
+
   resume_cv_.notify_one();
 }
 
@@ -422,17 +422,21 @@ ResponseOrError<EvaluateResponse> DebugBridge::evaluate(
   if (!request.context.has_value())
     return Error{"Evaluate request must have context"};
 
-  auto context = request.context.value();
-  DEBUGGER_LOG_INFO("[evaluate] Evaluate context: {}", context);
-
-  if (context == "repl")
-    return evaluateRepl(request);
-  else if (context == "watch")
-    return evaluateWatch(request);
-  else {
-    DEBUGGER_LOG_ERROR("[evaluate] Invalid evaluate context: {}", context);
-    return Error{"Invalid evaluate context"};
-  }
+  ResponseOrError<EvaluateResponse> response;
+  executeInMainThread([&] {
+    auto context = request.context.value();
+    DEBUGGER_LOG_INFO("[evaluate] Evaluate context: {}", context);
+
+    if (context == "repl")
+      response = evaluateRepl(request);
+    else if (context == "watch")
+      response = evaluateWatch(request);
+    else {
+      DEBUGGER_LOG_ERROR("[evaluate] Invalid evaluate context: {}", context);
+      response = Error{"Invalid evaluate context"};
+    }
+  });
+  return response;
 }
 
 ResponseOrError<EvaluateResponse> DebugBridge::evaluateRepl(
@@ -577,4 +581,31 @@ std::vector<StackFrame> DebugBridge::updateStackFrames() {
   return frames;
 }
 
+void DebugBridge::mainThreadWait(lua_State* L,
+                                 std::unique_lock<std::mutex>& lock) {
+  break_vm_ = L;
+  resume_ = false;
+  while (!resume_) {
+    resume_cv_.wait(lock, [this] { return resume_ || main_fn_ != nullptr; });
+
+    // Execute main_fn_ if it's not empty
+    if (main_fn_ != nullptr) {
+      main_fn_();
+      main_fn_ = nullptr;
+      resume_cv_.notify_one();
+    }
+  }
+  break_vm_ = nullptr;
+}
+
+void DebugBridge::executeInMainThread(std::function<void()> fn) {
+  DEBUGGER_ASSERT(isDebugBreak());
+
+  // Fill the main_fn_ and wait for it to be executed
+  std::unique_lock<std::mutex> lock(break_mutex_);
+  main_fn_ = std::move(fn);
+  resume_cv_.notify_one();
+  resume_cv_.wait(lock, [this] { return main_fn_ == nullptr; });
+}
+
 }  // namespace luau::debugger
\ No newline at end of file
diff --git a/debugger/src/internal/debug_bridge.h b/debugger/src/internal/debug_bridge.h
index e93cf7e..bb513e5 100644
--- a/debugger/src/internal/debug_bridge.h
+++ b/debugger/src/internal/debug_bridge.h
@@ -67,7 +67,6 @@ class DebugBridge final {
   StackTraceResponse getStackTrace();
 
   // Called from **DAP** client to get scopes for a frame
-  void extracted(int& level, lua_State*& L);
   ScopesResponse getScopes(int level);
 
   // Called from **DAP** client to get variable by variable reference
@@ -133,6 +132,9 @@ class DebugBridge final {
 
   std::vector<StackFrame> updateStackFrames();
 
+  void mainThreadWait(lua_State* L, std::unique_lock<std::mutex>& lock);
+  void executeInMainThread(std::function<void()> fn);
+
  private:
   friend class LuaStatics;
 
@@ -145,6 +147,7 @@ class DebugBridge final {
 
   lua_State* break_vm_ = nullptr;
   std::mutex break_mutex_;
+  std::function<void()> main_fn_;
   bool resume_ = false;
   std::condition_variable resume_cv_;
 
diff --git a/debugger/src/internal/utils/dap_utils.h b/debugger/src/internal/utils/dap_utils.h
index 145b2cc..db393a7 100644
--- a/debugger/src/internal/utils/dap_utils.h
+++ b/debugger/src/internal/utils/dap_utils.h
@@ -1,10 +1,9 @@
 #pragma once
+#include <nlohmann_json_serializer.h>
+#include <concepts>
 
 #include <dap/protocol.h>
 #include <dap/typeof.h>
-#include <nlohmann_json_serializer.h>
-#include <concepts>
-#include <exception>
 
 #include <Luau/BytecodeBuilder.h>
 #include <Luau/Common.h>
diff --git a/debugger/src/internal/variable.cpp b/debugger/src/internal/variable.cpp
index 07577e0..42b6096 100644
--- a/debugger/src/internal/variable.cpp
+++ b/debugger/src/internal/variable.cpp
@@ -1,6 +1,4 @@
 #include <format>
-#include "internal/scope.h"
-#include "internal/utils/lua_utils.h"
 
 #include <lapi.h>
 #include <lobject.h>
@@ -9,7 +7,9 @@
 #include <lualib.h>
 
 #include <internal/log.h>
+#include <internal/scope.h>
 #include <internal/utils.h>
+#include <internal/utils/lua_utils.h>
 #include <internal/variable.h>
 #include <internal/variable_registry.h>