[libcxx] Implement semaphores for windows

[mstorsjo]

Summary

Also add WIN32_LEAN_AND_MEAN before including windows.h, for consistency with other sources.

[curdeius]

libcxx/src/support/win32/thread_win32.cpp

11–12

A bit unrelated to this patch...

I know that compile times are not a problem in libc++, but could you define WIN32_LEAN_AND_MEAN before including windows.h?

I don't think we use any of the following (https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers#faster-builds-with-smaller-header-files):

Define WIN32_LEAN_AND_MEAN to exclude APIs such as Cryptography, DDE, RPC, Shell, and Windows Sockets.

310

No need for chrono::.
Maybe it would be clearer to write + milliseconds(1) - nanoseconds(1)?
Or even:

milliseconds __ms = std::chrono::ceil<milliseconds>(__ns);

Even if ceil is a C++17 addition, we only compile libc++ with C++17 or C++20.

[mstorsjo]

libcxx/src/support/win32/thread_win32.cpp

11–12

I guess I could add that here as well.

310

That looks nice to me. FWIW this bit is a copy from line 250 (but I wouldn't go and change that in this patch).

[mstorsjo]

Added WIN32_LEAN_AND_MEAN, using chrono::ceil.

[curdeius]

LGTM apart from unnecessary std:: (I wrote std:: without thinking too much before).

libcxx/src/support/win32/thread_win32.cpp

308

OK. Now, there's no need for std::. :)

[mstorsjo]

libcxx/src/support/win32/thread_win32.cpp

308

Doh, I thought I had removed the superfluous namespaces, will fix locally.

[rnk]

Looks good to me, but I am not a libc++ owner, so please wait for @ldionne.

[amccarth]

Looks good enough to me. (But I'm also not a libc++ owner.)

Given that these semaphores cannot be shared across processes, there's possibly a more efficient way to implement them other than mapping them to the corresponding kernel synchronization object (just as a CRITICAL_SECTION is a more efficient mutex if you only need to exclude other threads in the same process). But that's probably not important at this stage.

(I'm amused by the bike-shedding over a nanosecond on the timeout. Windows only times things down to 100 ns.)

libcxx/src/support/win32/thread_win32.cpp

284

The MS docs caution against using SEMAPHORE_ALL_ACCESS if you don't need it, but the explanation isn't clear. https://docs.microsoft.com/en-us/windows/win32/sync/synchronization-object-security-and-access-rights

Instead, they seem to be suggesting STANDARD_RIGHTS_ALL | SEMAPHORE_MODIFY_STATE, but I'm pretty sure that evaluates to the same thing.

This _probably_ doesn't matter, since I don't think the end user will have a way to share these across processes; they aren't named, and--with nullptr for the security attributes--they cannot be inherited. Whether other processes can read or change the ownership and ACL for the object seems moot.

[• Quuxplusone]

libcxx/include/__threading_support

154

Does this file actually #include <limits>?
Also: This is a sneaky way to prevent people from using the macro outside of namespace std. 😛

[ldionne]

I trust other reviewers with the implementation.

I've said it before, but I would be happier if we had a tester setup for windows. @rnk @amccarth Do any of you think that would be something feasible? The BuildKite setup is really easy to interface with.

libcxx/include/__threading_support

153

Let's use ::std::numeric_limits<long>::max(), just as good hygiene for a macro.

[mstorsjo]

In D97539#2604919, @ldionne wrote:

I've said it before, but I would be happier if we had a tester setup for windows. @rnk @amccarth Do any of you think that would be something feasible? The BuildKite setup is really easy to interface with.

Yeah, that'd be very much appreciated. With the latest couple patches and the updated instructions in D97166, it now runs pretty well out of the box overall. Continuous testing is sorely needed though, given the recent handful of breakages in MSVC configurations and/or when running tests overall. With current top of tree, there's still around 50 or so failed tests in a full check-cxx, but if someone would set up a buildbot, we could slap XFAIL on those, to be able to guard against further regressions and track what's left if things get cleaned up further.

libcxx/include/__threading_support

153

Ok, will do. FWIW I copied the current form from the <semaphore> header, from the _LIBCPP_NO_NATIVE_SEMAPHORES case (which is the one used on e.g. apple platforms afaik), which also lacks the ::std:: prefix and the explicit <limits> include.

154

It doesn't include <limits> yet, I can add that before pushing.

libcxx/include/__threading_support

 #endif
 
 // Condition Variable
 typedef void* __libcpp_condvar_t;
 #define _LIBCPP_CONDVAR_INITIALIZER 0
 
 // Semaphore
 typedef void* __libcpp_semaphore_t;
+#if defined(_LIBCPP_HAS_THREAD_API_WIN32)
+# define _LIBCPP_SEMAPHORE_MAX (numeric_limits<long>::max())
+#endif
 
 // Execute Once
 typedef void* __libcpp_exec_once_flag;
 #define _LIBCPP_EXEC_ONCE_INITIALIZER 0
 
 // Thread ID
 typedef long __libcpp_thread_id;
 

libcxx/src/support/win32/thread_win32.cpp

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include <__threading_support>
+#define NOMINMAX
+#define WIN32_LEAN_AND_MEAN
 #include <windows.h>
 #include <process.h>
 #include <fibersapi.h>
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 static_assert(sizeof(__libcpp_mutex_t) == sizeof(SRWLOCK), "");
 static_assert(alignof(__libcpp_mutex_t) == alignof(SRWLOCK), "");

 static_assert(alignof(__libcpp_thread_id) == alignof(DWORD), "");
 
 static_assert(sizeof(__libcpp_thread_t) == sizeof(HANDLE), "");
 static_assert(alignof(__libcpp_thread_t) == alignof(HANDLE), "");
 
 static_assert(sizeof(__libcpp_tls_key) == sizeof(DWORD), "");
 static_assert(alignof(__libcpp_tls_key) == alignof(DWORD), "");
 
+static_assert(sizeof(__libcpp_semaphore_t) == sizeof(HANDLE), "");
+static_assert(alignof(__libcpp_semaphore_t) == alignof(HANDLE), "");
+
 // Mutex
 int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m)
 {
   InitializeCriticalSection((LPCRITICAL_SECTION)__m);
   return 0;
 }
 
 int __libcpp_recursive_mutex_lock(__libcpp_recursive_mutex_t *__m)

 
 int __libcpp_tls_set(__libcpp_tls_key __key, void *__p)
 {
   if (!FlsSetValue(__key, __p))
     return GetLastError();
   return 0;
 }
 
+// Semaphores
+bool __libcpp_semaphore_init(__libcpp_semaphore_t* __sem, int __init)
+{
+  *(PHANDLE)__sem = CreateSemaphoreEx(nullptr, __init, _LIBCPP_SEMAPHORE_MAX,
+                                      nullptr, 0, SEMAPHORE_ALL_ACCESS);
+  return *__sem != nullptr;
+}
+
+bool __libcpp_semaphore_destroy(__libcpp_semaphore_t* __sem)
+{
+  CloseHandle(*(PHANDLE)__sem);
+  return true;
+}
+
+bool __libcpp_semaphore_post(__libcpp_semaphore_t* __sem)
+{
+  return ReleaseSemaphore(*(PHANDLE)__sem, 1, nullptr);
+}
+
+bool __libcpp_semaphore_wait(__libcpp_semaphore_t* __sem)
+{
+  return WaitForSingleObjectEx(*(PHANDLE)__sem, INFINITE, false) ==
+         WAIT_OBJECT_0;
+}
+
+bool __libcpp_semaphore_wait_timed(__libcpp_semaphore_t* __sem,
+                                   chrono::nanoseconds const& __ns)
+{
+  chrono::milliseconds __ms = std::chrono::ceil<chrono::milliseconds>(__ns);
+  return WaitForSingleObjectEx(*(PHANDLE)__sem, __ms.count(), false) ==
+         WAIT_OBJECT_0;
+}
+
 _LIBCPP_END_NAMESPACE_STD