[Lldb-commits] [lldb] [lldb][windows] Preserve FIFO order for equal time MainLoop callbacks (PR #199056)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Charles Zablit (charles-zablit)

<details>
<summary>Changes</summary>

MainLoopBase::AddCallback / AddPendingCallback enqueue into a std::priority_queue keyed only on TimePoint. AddPendingCallback uses a default-constructed  TimePoint, so all pending callbacks share the same key and the heap returns them in arbitrary order.

This patch fixes this by adding an int counter to sort entries with equal time points.

This fixes `commands/expression/dont_allow_jit/TestAllowJIT.py` when running with `LLDB_USE_LLDB_SERVER=1`.

---
Full diff: https://github.com/llvm/llvm-project/pull/199056.diff


2 Files Affected:

- (modified) lldb/include/lldb/Host/MainLoopBase.h (+24-5) 
- (modified) lldb/source/Host/common/MainLoopBase.cpp (+6-5) 


``````````diff
diff --git a/lldb/include/lldb/Host/MainLoopBase.h b/lldb/include/lldb/Host/MainLoopBase.h
index 9529f2c214784..5703f64982e5b 100644
--- a/lldb/include/lldb/Host/MainLoopBase.h
+++ b/lldb/include/lldb/Host/MainLoopBase.h
@@ -14,9 +14,12 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <chrono>
+#include <cstdint>
 #include <functional>
 #include <mutex>
 #include <queue>
+#include <tuple>
+#include <vector>
 
 namespace lldb_private {
 
@@ -99,11 +102,27 @@ class MainLoopBase {
   std::optional<TimePoint> GetNextWakeupTime();
 
   std::mutex m_callback_mutex;
-  std::priority_queue<std::pair<TimePoint, Callback>,
-                      std::vector<std::pair<TimePoint, Callback>>,
-                      llvm::on_first<std::greater<TimePoint>>>
-      m_callbacks;
-  bool m_terminate_request : 1;
+
+  struct CallbackEntry {
+    TimePoint time_point;
+    Callback callback;
+
+    CallbackEntry(TimePoint tp, uint64_t seq, Callback cb)
+        : time_point(std::move(tp)), sequence(seq), callback(std::move(cb)) {}
+
+    bool operator<(const CallbackEntry &other) const {
+      if (time_point != other.time_point)
+        return time_point > other.time_point;
+      return sequence > other.sequence;
+    }
+
+  private:
+    uint64_t sequence;
+  };
+
+  std::priority_queue<CallbackEntry> m_callbacks;
+  uint64_t m_callback_sequence = 0;
+  bool m_terminate_request = false;
 
 private:
   class ReadHandle {
diff --git a/lldb/source/Host/common/MainLoopBase.cpp b/lldb/source/Host/common/MainLoopBase.cpp
index 232b9bc0aa354..f50551bdfd89d 100644
--- a/lldb/source/Host/common/MainLoopBase.cpp
+++ b/lldb/source/Host/common/MainLoopBase.cpp
@@ -19,8 +19,9 @@ bool MainLoopBase::AddCallback(const Callback &callback, TimePoint point) {
     std::lock_guard<std::mutex> lock{m_callback_mutex};
     // We need to interrupt the main thread if this callback is scheduled to
     // execute at an earlier time than the earliest callback registered so far.
-    interrupt_needed = m_callbacks.empty() || point < m_callbacks.top().first;
-    m_callbacks.emplace(point, callback);
+    interrupt_needed =
+        m_callbacks.empty() || point < m_callbacks.top().time_point;
+    m_callbacks.emplace(point, m_callback_sequence++, callback);
   }
   if (interrupt_needed)
     interrupt_succeeded = Interrupt();
@@ -33,9 +34,9 @@ void MainLoopBase::ProcessCallbacks() {
     {
       std::lock_guard<std::mutex> lock{m_callback_mutex};
       if (m_callbacks.empty() ||
-          std::chrono::steady_clock::now() < m_callbacks.top().first)
+          std::chrono::steady_clock::now() < m_callbacks.top().time_point)
         return;
-      callback = std::move(m_callbacks.top().second);
+      callback = std::move(m_callbacks.top().callback);
       m_callbacks.pop();
     }
 
@@ -47,5 +48,5 @@ std::optional<MainLoopBase::TimePoint> MainLoopBase::GetNextWakeupTime() {
   std::lock_guard<std::mutex> lock(m_callback_mutex);
   if (m_callbacks.empty())
     return std::nullopt;
-  return m_callbacks.top().first;
+  return m_callbacks.top().time_point;
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/199056