mstorsjo added a comment.
In https://reviews.llvm.org/D38704#892479, @zturner wrote:
> I'm a little nervous about re-inventing a poor man's version of a reader
> writer lock. Can we not just copy LLVM's?
I guess I could have a look to see how much extra either kitchen sink it would
bring. Since it almost mapped 1:1 to the windows functions, I thought it
wouldn't end up too large - unfortunately the return values and unlock
functions made it a bit harder though.
================
Comment at: src/UnwindCursor.hpp:20
#ifndef _LIBUNWIND_HAS_NO_THREADS
- #include <pthread.h>
+ #ifdef _WIN32
+ #include <windows.h>
----------------
zturner wrote:
> Maybe you want to check `_MSC_VER` here? I think MinGW compilers will have
> `pthread` support.
MinGW compilers can have optional pthread support (it's not a feature of the
compiler itself but an extra library that one can choose to build), but not
everybody wants to use it and at least I wouldn't want to use it as mandatory
dependency for any C++ support based on libunwind.
================
Comment at: src/UnwindCursor.hpp:50
+static bool lockedForWrite = false;
+static int pthread_rwlock_rdlock(pthread_rwlock_t *lock) {
+ AcquireSRWLockShared(lock);
----------------
zturner wrote:
> This doesn't seem like what you want. a global `static` function / variable
> in a header file is going to be duplicated in every translation unit. i.e.
> two translation units will have different copies of `lockedForWrite`. Same
> goes for the rest of the functions.
Oh, right - I would need to make it match the static class member `_lock`
instead.
================
Comment at: src/UnwindCursor.hpp:61-64
+ if (lockedForWrite) {
+ lockedForWrite = false;
+ ReleaseSRWLockExclusive(lock);
+ } else {
----------------
zturner wrote:
> Doesn't `lockedForWrite` need to be atomic? If it is not atomic, there is no
> guarantee that thread 2 will see the results of thread 1's modifications with
> any kind of reasonable order.
I don't think it needs to be atomic, although the `rdlock` function perhaps
shouldn't touch it at all. It only ever gets set to true once we have an
exclusive lock, and in those cases gets set back to false before the exclusive
lock gets released. So without touching it in the `rdlock` function, we only
ever write to it while holding the exclusive write lock.
https://reviews.llvm.org/D38704
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
