[Lldb-commits] [PATCH] D60496: [lldb-server] Use std::thread/mutex for all platforms

[Pavel Labath via Phabricator via lldb-commits]

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