[SystemZ][ZOS] Porting pthread_t related functionality within libc++ to z/OS

[zibi]

Summary

This patch will address the differences of definition of pthread_t on z/OS vs. Linux.
Main trick to make the code work on z/OS relies on redefining libcpp_thread_id type and
_LIBCPP_NULL_THREAD macro. This is necessary to separate initialization of libcxx_thread_id from the one of __libcxx_thread_t;

[mclow.lists]

libcxx/include/__threading_support

495

This is what this code used to be. It's wrong, because pthread_t is not a type that we control, and there's no guarantee that it has an operator==. That's why there's a call pthread_equal

[zibi]

libcxx/include/__threading_support

495

This is what this code used to be. It's wrong, because pthread_t is not a type that we control, and there's no guarantee that it has an operator==. That's why there's a call pthread_equal

Thx Marshall, the problem with the original call to 'thread_equal is that it was passing wrong parameters by mixing thread_id with thread_t. We need clear separation between them and don't use them interchangeably.

[uweigand]

libcxx/include/__threading_support

495

Note that even in the current code, __libcpp_thread_id is implicitly assumed to be a numeric type, see in particular the routine __libcpp_thread_is_less immediately below.

My suggestion has been to use __libcpp_thread_t for the platform-specific opaque thread handle (e.g. pthread_t), and to use __libcpp_thread_id for a numeric ID. On platforms where pthread_t happens to be numeric itself, the types can be the same; other platforms need to provide a different definition for __libcpp_thread_id (as done by this patch for z/OS).

[zibi]

libcxx/include/__threading_support

495

Note that even in the current code, __libcpp_thread_id is implicitly assumed to be a numeric type, see in particular the routine __libcpp_thread_is_less immediately below.

Yes Ulrich, I noticed that as well.

If we want to use pthread_equal() we will have to operate on __libcpp_thread_t in both __libcpp_thread_id_less and __libcpp_thread_id_equal like we do already do in `__libcpp_thread_isnull'.
However, that will get us to a ripple effect of changing template __thread_id. Furthermore __thread_id is being used in hash<>.

Marshall, let me know if you want me to pursue this road.

[hubert.reinterpretcast]

libcxx/include/__threading_support

93

The macro needs extra parentheses so that sizeof _LIBCPP_NULL_THREAD is not parsed as sizeof(<function type>).

[zibi]

Putting extra paren. for macro to address Hubert comment.

[zibi]

libcxx/include/__threading_support

495

One of the usage of __thread_id is 'recursive_timed_mutex which does retrieves the id of the thread before calling checking if it's the same as the current thread. Here is the snippet of this:

__thread_id __id = this_thread::get_id();
unique_lock<mutex> lk(__m_);
if (__id == __id_)

Therefore we are assuming that thread_id is integral type and using == is perfectly fine.
Alternatively, we could change the code to use pthread_equal(), but that would require the argument type to change and other ripple effects.

[zibi]

Marshall, I really want to hear your opinion about the need to call pthread_equal().
See my last comment. The 'operator== of __thread_id is calling __libcpp_thread_id_equal passing two arguments of type __libcpp_thread_id which have integral type. Therefore, as it is now it's perfectly fine to do operator equal as apposed to call pthread_equal().

In a meantime I experimented with '__libcpp_thread_t so we can actually call pthread_equal(). I introduced __thread_t' class in place of __thread_id and all the ripple effect code associated with it. Let me know if you want to proceed this route and/or see my draft first.

[zibi]

@mclow.lists Please see my proposal draft so we can call pthread_equal().

[zibi]

ping

[mclow.lists]

libcxx/include/__threading_support

495

Now you're mixing types. __thread_id is a different type than __libcpp_thread_t

__thread_id has a full complement of comparison operators, and hash support as well.

[mclow.lists]

libcxx/include/mutex

283 ↗

(On Diff #296779)

Can't make this change. This is an ABI break if sizeof(__thread_id) != sizeof(__thread_t)

[zibi]

Thank you, Marshall.

So far we have only 2 concrete solutions. Does anybody see another alternative?

1. use built-in comparision operator instead of 'thread_equal()
2. change '__thread_id to '`__thread_t'

libcxx/include/__threading_support

495

Mixing? Sorry, I don't see it, the call is at line 765 were we pass __libcpp_thread_t type member '__t_ as follows:

return __libcpp_thread_equal(__x.__t_, __y.__t_);

We can beef up the comparison operators for __thread_t that is not a problem. I included the minimum for the purpose of validating/approving this approach.

libcxx/include/mutex

283 ↗

(On Diff #296779)

Yes, you are right and I noticed that as well but do we know for fact that there is a platform where sizeof(thread_id) != sizeof(thread_t) ? I compiled the following assertions with not issues on z/OS and RHEL 7.4.

static_assert(sizeof(__th.__get_id()) == sizeof(__th), "zibi trace __thread_id != __thread_t");

As a matter of fact __thread_id is the same type as __thread_t until we discovered on z/OS that this can not be the case. If this is really no go I'm open for suggestions.

[mclow.lists]

libcxx/include/mutex

283 ↗

(On Diff #296779)

There are three different implementations in this file.

• One for pthreads
• One for C11 threads
• One for external threading API

On two of them, __thread_id is the same type as __thread_t.
On two of them, the type __thread_t is defined outside of libc++ (to be precise, __thread_t is a typedef for a type supplied by the OS). Neither of those cases require that type to be an integral type.

We have to keep all of these working.

[zibi]

libcxx/include/__threading_support

495

Note that even in the current code, __libcpp_thread_id is implicitly assumed to be a numeric type, see in particular the routine __libcpp_thread_is_less immediately below.

My suggestion has been to use __libcpp_thread_t for the platform-specific opaque thread handle (e.g. pthread_t), and to use __libcpp_thread_id for a numeric ID. On platforms where pthread_t happens to be numeric itself, the types can be the same; other platforms need to provide a different definition for __libcpp_thread_id (as done by this patch for z/OS).

495

Note that even in the current code, __libcpp_thread_id is implicitly assumed to be a numeric type, see in particular the routine __libcpp_thread_is_less immediately below.

Marshall (@mclow.lists) going back to the beginning can we work to address Ulrich's comment first? Ideally that should be addressed independently from z/OS changes.

[zibi]

The 'pthread_equal() was called as intended with ptread_t prior to D19412 which introduced __libcpp_thread_id and used that type on the call to 'pthread_equal. Unfortunately this change made the assumption that __libcpp_pthread_t is same as __libcpp_pthread_id which is not the case on z/OS and prompted this discussion and the need for another patch.

In regards to API break pointed by @mclow.lists __thread_id had only one member of type pthread_t which only after D19412 became __libcpp_pthread_id. Same is true for __id_ member of recursive_timed_mutex. To me the API was broken at that time and my proposal will revert that. However, I'm not sure if that is the route we want to take.

Up to now only _LIBCPP_HAS_THREAD_API_EXTERNAL implementation had different underlying types as follows:

typedef long __libcpp_thread_id;
typedef void* __libcpp_thread_t;

This implementation did not run into the issue we discussing here since it didn't need and did not provide __libcpp_thread_id_equal.
Let me ping the original authors @rmaprath, @espositofulvio for input how to proceed.

[zibi]

ping, any feedback?

[zibi]

Resubmit the patch with small delta to make use of member function thread_t::get_id() and validate CI.

[zibi]

I'm abending this since there is no interest and current implementation will work on z/OS.

libcxx/include/__config

 #  if defined(__FreeBSD__) || \
       defined(__wasi__) || \
       defined(__NetBSD__) || \
       defined(__linux__) || \
       defined(__GNU__) || \
       defined(__APPLE__) || \
       defined(__CloudABI__) || \
       defined(__sun__) || \
+      defined(__MVS__) || \
       (defined(__MINGW32__) && __has_include(<pthread.h>))
 #    define _LIBCPP_HAS_THREAD_API_PTHREAD
 #  elif defined(__Fuchsia__)
      // TODO(44575): Switch to C11 thread API when possible.
 #    define _LIBCPP_HAS_THREAD_API_PTHREAD
 #  elif defined(_LIBCPP_WIN32API)
 #    define _LIBCPP_HAS_THREAD_API_WIN32
 #  else

libcxx/include/__threading_support

 
 #if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
 # include <__external_threading>
 #elif !defined(_LIBCPP_HAS_NO_THREADS)
 
 #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
 # include <pthread.h>
 # include <sched.h>
-# ifdef __APPLE__
+# if defined(__APPLE__) || defined(__MVS__)
 #  define _LIBCPP_NO_NATIVE_SEMAPHORES
 # endif
 # ifndef _LIBCPP_NO_NATIVE_SEMAPHORES
 # include <semaphore.h>
 # endif
 #elif defined(_LIBCPP_HAS_THREAD_API_C11)
 # include <threads.h>
 #endif

 # define _LIBCPP_SEMAPHORE_MAX SEM_VALUE_MAX
 #endif
 
 // Execute once
 typedef pthread_once_t __libcpp_exec_once_flag;
 #define _LIBCPP_EXEC_ONCE_INITIALIZER PTHREAD_ONCE_INIT
 
 // Thread id
+#if defined(__MVS__)
+  typedef unsigned long long __libcpp_thread_id;
+#else
   typedef pthread_t __libcpp_thread_id;
+#endif
 
 // Thread
+#if defined(__MVS__)
+  #define _LIBCPP_NULL_THREAD (__libcpp_thread_t())
+#else
   #define _LIBCPP_NULL_THREAD 0U
+#endif
 
 typedef pthread_t __libcpp_thread_t;
 
 // Thread Local Storage
 typedef pthread_key_t __libcpp_tls_key;
 
 #define _LIBCPP_TLS_DESTRUCTOR_CC
 #elif defined(_LIBCPP_HAS_THREAD_API_C11)
int __libcpp_execute_once(__libcpp_exec_once_flag *flag,
                           void (*init_routine)()) {
   return pthread_once(flag, init_routine);
 }
 
 // Thread id
 // Returns non-zero if the thread ids are equal, otherwise 0
 bool __libcpp_thread_id_equal(__libcpp_thread_id t1, __libcpp_thread_id t2)
 {
-  return pthread_equal(t1, t2) != 0;
+  return t1 == t2;
 }
 
 // Returns non-zero if t1 < t2, otherwise 0
 bool __libcpp_thread_id_less(__libcpp_thread_id t1, __libcpp_thread_id t2)
 {
   return t1 < t2;
 }
 
 // Thread
 bool __libcpp_thread_isnull(const __libcpp_thread_t *__t) {
-  return *__t == 0;
+  return __libcpp_thread_get_id(__t) == 0;
 }
 
 int __libcpp_thread_create(__libcpp_thread_t *__t, void *(*__func)(void *),
                            void *__arg)
 {
   return pthread_create(__t, 0, __func, __arg);
 }
 
 __libcpp_thread_id __libcpp_thread_get_current_id()
 {
-  return pthread_self();
+  const __libcpp_thread_t thread = pthread_self();
+  return __libcpp_thread_get_id(&thread);
 }
 
 __libcpp_thread_id __libcpp_thread_get_id(const __libcpp_thread_t *__t)
 {
+#if defined(__MVS__)
+  return __t->__;
+#else
   return *__t;
+#endif
 }
 
 int __libcpp_thread_join(__libcpp_thread_t *__t)
 {
   return pthread_join(*__t, 0);
 }
 
 int __libcpp_thread_detach(__libcpp_thread_t *__t)