[libcxx-commits] [PATCH] D65895: Fix a layering violation in mutex - prep for fixing PR42918

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists created this revision.
mclow.lists added reviewers: EricWF, ldionne, howard.hinnant.
Herald added subscribers: dexonsmith, christof.
mclow.lists marked an inline comment as done.
mclow.lists added inline comments.


================
Comment at: libcxx/include/mutex:199
 #include <__threading_support>
+#include <thread>
 
----------------
If I were to commit this; I would probably move `__thread_id` and `this_thread::` into `<__threading_support>` and remove this include.


While investigating https://llvm.org/PR42918, I noticed that the `recursive_timed_mutex` used the primitive thread-ids directly, rather than the convenience class `__thread_id`.

This fixes that, and makes it so we can fix the above issue only in `__thread_id`, rather than enshrining the "not a thread" knowledge in each of the OS-specific bits.

I'm a bit worried about ABI here, even though the field that I'm replacing is the same size.


https://reviews.llvm.org/D65895

Files:
  libcxx/include/mutex
  libcxx/include/thread
  libcxx/src/mutex.cpp


Index: libcxx/src/mutex.cpp
===================================================================
--- libcxx/src/mutex.cpp
+++ libcxx/src/mutex.cpp
@@ -132,7 +132,7 @@
 
 recursive_timed_mutex::recursive_timed_mutex()
     : __count_(0),
-      __id_(0)
+      __id_{}
 {
 }
 
@@ -144,9 +144,9 @@
 void
 recursive_timed_mutex::lock()
 {
-    __libcpp_thread_id id = __libcpp_thread_get_current_id();
+    __thread_id id = this_thread::get_id();
     unique_lock<mutex> lk(__m_);
-    if (__libcpp_thread_id_equal(id, __id_))
+    if (id ==__id_)
     {
         if (__count_ == numeric_limits<size_t>::max())
             __throw_system_error(EAGAIN, "recursive_timed_mutex lock limit reached");
@@ -162,9 +162,9 @@
 bool
 recursive_timed_mutex::try_lock() _NOEXCEPT
 {
-    __libcpp_thread_id id = __libcpp_thread_get_current_id();
+    __thread_id id = this_thread::get_id();
     unique_lock<mutex> lk(__m_, try_to_lock);
-    if (lk.owns_lock() && (__count_ == 0 || __libcpp_thread_id_equal(id, __id_)))
+    if (lk.owns_lock() && (__count_ == 0 || id == __id_))
     {
         if (__count_ == numeric_limits<size_t>::max())
             return false;
@@ -181,7 +181,7 @@
     unique_lock<mutex> lk(__m_);
     if (--__count_ == 0)
     {
-        __id_ = 0;
+        __id_.reset();
         lk.unlock();
         __cv_.notify_one();
     }
Index: libcxx/include/thread
===================================================================
--- libcxx/include/thread
+++ libcxx/include/thread
@@ -238,6 +238,9 @@
         bool operator>=(__thread_id __x, __thread_id __y) _NOEXCEPT
         {return !(__x < __y);}
 
+    _LIBCPP_INLINE_VISIBILITY
+    void reset() { __id_ = 0; }
+    
     template<class _CharT, class _Traits>
     friend
     _LIBCPP_INLINE_VISIBILITY
Index: libcxx/include/mutex
===================================================================
--- libcxx/include/mutex
+++ libcxx/include/mutex
@@ -196,6 +196,7 @@
 #endif
 #include <version>
 #include <__threading_support>
+#include <thread>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
@@ -280,7 +281,7 @@
     mutex              __m_;
     condition_variable __cv_;
     size_t             __count_;
-    __libcpp_thread_id __id_;
+    __thread_id        __id_;
 public:
      recursive_timed_mutex();
      ~recursive_timed_mutex();
@@ -307,9 +308,9 @@
 recursive_timed_mutex::try_lock_until(const chrono::time_point<_Clock, _Duration>& __t)
 {
     using namespace chrono;
-    __libcpp_thread_id __id = __libcpp_thread_get_current_id();
+    __thread_id __id = this_thread::get_id();
     unique_lock<mutex> lk(__m_);
-    if (__libcpp_thread_id_equal(__id, __id_))
+    if (__id == __id_)
     {
         if (__count_ == numeric_limits<size_t>::max())
             return false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65895.213963.patch
Type: text/x-patch
Size: 2795 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190807/1e095b36/attachment.bin>