labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

LGTM, with some inline comments about additional c++11 goodies we can use to 
clean up this file further. (Also, it might be good to mention in the patch 
title that this is about modifying the test code, because my first though was 
that you are adding some locking to the actual lldb-server code.)



================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp:37-45
+#if defined(_WIN32)
+#include <windows.h>
+
+static unsigned int sleep(unsigned int seconds) {
+  ::Sleep(seconds * 1000);
+  return 0;
+}
----------------
Other tests already use `std::this_thread::sleep_for(std::chrono::seconds(X))`, 
so I expect you should be able to do the same here.


================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp:180-185
+  static std::mutex s_thread_index_mutex;
   static int s_thread_index = 1;
 
-  pthread_mutex_lock(&s_thread_index_mutex);
+  s_thread_index_mutex.lock();
   const int this_thread_index = s_thread_index++;
+  s_thread_index_mutex.unlock();
----------------
you could just make `s_thread_index` an `atomic<int>`, and avoid the manual 
locking around the increment operation.


================
Comment at: packages/Python/lldbsuite/test/tools/lldb-server/main.cpp:188-192
+    g_print_mutex.lock();
     printf("thread %d id: ", this_thread_index);
     print_thread_id();
     printf("\n");
+    g_print_mutex.unlock();
----------------
use std::scoped_lock instead of manual lock/unlock calls (throughout the patch).


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60496/new/

https://reviews.llvm.org/D60496



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to