[libcxx-commits] [PATCH] D97539: [libcxx] Implement semaphores for windows

[Adrian McCarthy via Phabricator via libcxx-commits]

amccarth accepted this revision.
amccarth added a comment.

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.)



================
Comment at: libcxx/src/support/win32/thread_win32.cpp:284
+  *(PHANDLE)__sem = CreateSemaphoreEx(nullptr, __init, _LIBCPP_SEMAPHORE_MAX,
+                                      nullptr, 0, SEMAPHORE_ALL_ACCESS);
+  return *__sem != nullptr;
----------------
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.


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

https://reviews.llvm.org/D97539