Bug in reader/writer locks in Windows API
old.reddit.com
old.reddit.com
The repro code holds the shared lock while waiting for all other threads to acquire the shared lock, and thus deadlocks if any of the worker threads accidentally gets an exclusive lock. In "normal" use cases where the lock is used to protect some shared resource, threads holding the lock don't wait for each other, so there is no deadlock.
Interesting stuff!
For comparison, the Rust implementation for lightweight RWLocks on futex-capable *nix platforms is here: https://doc.rust-lang.org/stable/src/std/sys/unix/locks/fute... It sets the "reader counter" in the underlying atomic to a special value to signal that the lock is set for exclusive access. So a reader thread acquiring the lock as shared can never result in this kind of bug. Bits are used to signal whether readers or writers are currently waiting on a lock, but this just cannot turn a lock that's acquired for shared access into exclusive, or vice versa.
threads holding the lock don't wait for each other
Unless you're doing nested locking.
In my experience, needing to do nested locking is a sign you're up the wrong creek.
I've rewritten some interfaces and implementations from using nested locking to non-nested, and they became easier to use and much faster.
Not saying there's never a place for them, but I avoid nested locking like the plague.
Contract #1: In reader writer locks in general, after I share-acquire a lock, I know that there are no active exclusive owners of that lock and won't be until I release my shared lock. I can also expect that as long as I hold this shared lock, other threads can share-acquire the same lock without waiting. ony share-exclusive threads would have to wait.
This general contract seems useful to me.
The contract you're describing, contract #2, is one in which shared-acquire is an optional optimization over exclusive-acquire (not a contractual guarantee) and that the system is free to promote shared to exclusive lock acquisitions.
This other contract seems finnicky and error-prone.
Can't we have SRWLOCK implement contract #1?
Thats how we arrive at the rule that shared acquires in anything, but a trivial system (no exclusive acquires) may not be compatible. So, contract #1 is typically not satisfied. Of course, this particular case is slightly different and so you might decide to support it.
also look my 2 comments on reddit: https://www.reddit.com/r/cpp/comments/1b55686/comment/ktfhjs... and https://www.reddit.com/r/cpp/comments/1b55686/comment/ktfggu...
if say true - i here in very strange position. i am by self under debugger research exacly what happens in concrete case and create repro code. i think that implementation of the RtlReleaseSRWLockExclusive is not the best and may be really containing "bug" as i in more details describe in comment on reddit. but from another side, if you want pure formal c++ rules - can you explain - in what exactly was bug in concre case ? what rule/guarantee is violated ? why demo code decide that ALL shared waiters can at once acquire the lock. formal documentation not state this. this intuitive must be true, because no exclusive request more. but.. i really dont know. and i also for fun create own implementation of SRW lock ( if someone interested to look -https://github.com/rbmm/PushLock ) which free from this roblem - i always ( hope) do only single atomic change to lock state during api call. and finally - sorry for my english and too long answer
allow but NOT DEMAND this. If one thread has acquired the shared lock , other thread can acquire the shared lock too. but only CAN. in some case system not let another thread enter to lock, despite it also request share access. one case: if another rthread request exclusive acess - he begin wait and after this - any thread which acquire even shared access to lock - also begin wait
If lock_shared is called by a thread that already owns the mutex in any mode (exclusive or shared), the behavior is undefined.
and
Shared mode SRW locks should not be acquired recursively as this can lead to deadlocks when combined with exclusive acquisition.
why is this ? because if between 2 calls to lock_shared ( AcquireSRWLockShared ) another thread call AcquireSRWLockExclusive - the second call is block.
the code in example make assumption that ALL threads can enter to lock at once. that if one thread enter to lock in shared mode, another thread also ALWAYS can enter to lock in shared mode (if no exclusive requests). but i not view clear formalization of such requirement. and we must not based on this.
i be will add next rule:
thread inside lock must not wait on another thread to enter this lock
this is obvivius for exlusive access, but not obvivous to shared. but must be cleare stated along with the recursive rule ( should not be acquired recursively as this can lead to deadlocks, even in shared mode)
The experience left me with such a bad feeling for shared locks that I tend to avoid them unless absolutely required. When I last tested std::shared_mutex, performance was so poor compared to std::mutex that double-buffering the data protected by a simple mutex was much faster.
This was a great post by the original Redditor.
We tested cmpxchg16b and found the performance was terrible with more than 4 cores.
Ended up using spin-locks similar to Linux kernel RCU.
The C++ std::mutex provided in Microsoft's STL is ludicrously big, whereas SRWLocks are the same size as a pointer. In one sense being ludicrously big is great - less risk the std::mutex will mistakenly share a cache line with your unrelated data, but for most people this makes std::mutex annoyingly expensive.
Sadly, SRWLocks have yet another bug, they are very unfair. Write a loop which locks std::mutex inside the body, and no other thread will be able to grab the mutex despite the loop repeatedly releases then re-acquires the mutex. Critical sections are way more fair.
The most successful thread was only 25%, 15% and 9% ahead of the least successful one. On the contrary, in my simple usermode spinlock the unfairness would be 1000% or even 2000%.
The SRWLocks are indeed the size of a pointer, the std::mutex is not for ABI reasons.
* breaking up the lock so different threads can access different parts of your data structure concurrently.
* double-buffering (also called ping-pong buffers) where you effectively keep two copies of your data structure. The readers can access one without blocking, a single writer can modify the other and then swap.
* just accepting that reads will block each other with a std::mutex and work on minimizing the amount of time spent in the lock. This can actually work out quicker depending on your access patterns.
As always, careful profiling with real data is required to figure out what is better.
T1: writer populates copy1
T2: readers access copy1
T3: writer populates copy2, and swaps
T4: writer populates copy1, and swaps
At T4 a reader from T2 could still be accessing the old data structure.Unless I'm overthinking it.
they have slightly worse performance compared to MS, when high contention, but in test with this OP case - work well. in my test i not found bugs in implementation, but of course can not be sure that they not exist, very complex logic really. nobody test this, however very simply replace SRW calls to my implementation, by macros in h file
Their usage often fails to outperform a regular lock due to additional overhead. They seem to make sense only in specific high-contention scenarios where arrival rate is high and/or the critical section has a long duration [1].
[1]: https://petsta.net/blog/2022-09-30-rwmutex/ - Go specific, but I suspect these results hold true for most implementations of reader-writer locks.
This is a Windows API bug with the slim reader/writer (SRW) locks. It's just that the bug was discovered via std::shared_mutex as that is implemented using SRW locks.
SRW locks: https://learn.microsoft.com/en-us/windows/win32/sync/slim-re...
Confirmation from a Microsoft employee that the bug has been raised internally with the Windows API team: https://old.reddit.com/r/cpp/comments/1b55686/maybe_possible...
Rust's Mutex on Windows is also an SRWLock but it can't hit this bug because it deliberately only uses the exclusive locking.
The similar case occurs when you have: - 1+ threads holding a shared lock (Readers) - 1+ threads waiting to acquire an exclusive lock (Pending Writers) - 1+ threads trying to acquire the shared lock (Pending Readers) - 1+ Reader is waiting on a Pending Reader
In this case the Pending Readers will be unable to acquire the shared lock even though it is still in "read mode" because in a fair RW lock Pending Writers are prioritised above Pending Readers so as not to starve the writer side of the lock.
It's doing a pretty weird thing with the locks, I wonder what the actual use case was. Readers should almost never care about other readers. Typically you just grab the lock, read the thing, and release it. You always have to be super careful about deadlocks if you're holding a lock and also waiting around for something else to happen.
I also want to test this on my highly customised XP install which has been patched to add the SRW API among other extensions, and where I had also patched the kernel to fix a race condition causing a deadlock in the keyed event API that the SRW implementation is based on (maybe it's this same one, although in Vista+ they changed it significantly; but the same edge case could occur.)
I patched the kernel in memory first, using a kernel debugger, to verify my fix worked before editing the file on disk.
(Working out how to patch such that I wouldn't crash the system if a process happens to call that API while it was in a half-modified state was also a fun problem...)
I dug out the details on the bug I patched, and it isn't the same as this one; it's a race condition with timeouts on waiting for keyed events, which I believe isn't applicable in this situation as there are no timeouts.
http://www.malsmith.net/blog/patching-closed-software/
Note: not my blog. (Edit: removed a probably unnecessary, and likely inaccurate, detail).
Patching the kernel would involve a similar (but slightly more complicated) process.
The ReactOS implementation is more involved https://doxygen.reactos.org/d1/db8/srw_8c_source.html but still, it uses mostly CAS operations both for the shared and the exclusive case. So it should be largely free from issues.
Correction: Bug in reader/writer locks in Windows
Not in API.
struct ThreadTestData {
int32_t numThreads = 0;
std::shared_mutex sharedMutex = {};
std::atomic<int32_t> readCounter = 0;
};
// child thread
DoStuff() {
data->readCounter.fetch_add(1);
while (data->readCounter.load() != data->numThreads) {
std::this_thread::yield();
}
}
The numThreads field is not an atomic variable. It's initialized to 0 and set to 5 in the main thread. Its memory address is then passed to the child threads to be checked in the yielding loop. Since it's non-atomic, there's no memory barrier instruction to force its new value (5) to propagate to all CPU's running the threads. A child thread might get the old value 0. The logic of the yield checking loop using it would never exit.Since the main thread runs the code in an endless loop, the same numThreads memory allocated on the stack is being set to 0 and 5 repeatedly. Some of the child threads can get the old value in one pass of the loop. Thus the hanging.
I hope you realize how deeply ironic this statement is. If you read the comments you'll find he even produced a slightly reduced repro. And in other comments he tried minor tweaks like the one you suggested.
You have a thesis that the program has a bug. (It doesn't.) Go ahead and test your thesis and report back.
Each core would have to fetch the value from main memory, where it will be undoubtedly 5. There is no valid reordering (at least under x86) that would cause the thread to read 0.
The reset to 0 and to 5 happens at the start of the loop. There's a happens-before relationship between it and the threads being created, and then again between the threads being joined and the loop cycling back. So there shouldn't be any data race here.
On x86, they do (the x86 family is unusual in having strong memory ordering), but that's not the issue here.
> They only see their local caches of the same memory location got changed from 0 to 5, 5 to 0, and back to 5 repeatedly.
Their local caches of that memory see only a 5, since at the moment they read that cache line, the value in memory is 5; the operating system ensures that the write of the 5 value by the main thread is flushed to memory[*] before the main thread starts the child thread, and also that the cache of the child thread does not have stale data from before that moment. That memory location is only set back to 0 after all the child threads have exited, so there's no instant where the child thread could read a 0 on that location from main memory into its cache.
> When a new thread lands on a CPU with the old 0 cache value, it will hang.
When a new thread lands on a CPU core with an old 0 cache value for that memory location (which could happen if that CPU core had been running the main thread, and the main thread was migrated to another CPU core before it could set it back to 5), it will still see a 5 at that memory location, because the operating system invalidates the cache of a CPU core when necessary before starting a new thread on it.
[*] Actually, it only has to be flushed as far as the last level cache, or the "point of unification" in ARM terminology; I simplified a lot in this explanation.
There is no bug in the program, it is legal to use non-atomic variables across threads as long as they're correctly sequenced.
Because if you did attempt to run the program you'd find that changing numThreads to a constexpr makes no difference.
The equivalent of the memory barrier instructions is there, but it's hidden within the operating system code which creates and initializes a new thread. That is, the operating system ensures that the value in the current CPU (in this case, 5) is propagated to the CPU running the newly started thread, before the thread start routine (in this case, DoStuff) is called. The value is not modified while the child threads are running (it waits for the child threads to exit before clearing the value), so there's no chance of the child threads seeing the value being set back to zero.
[1] https://learn.microsoft.com/en-us/windows/win32/api/processt...
The C++ standard (at least a draft of it I found on a quick web search) is more explicit: it says (https://eel.is/c++draft/thread.thread.constr) "The completion of the invocation of the constructor synchronizes with the beginning of the invocation of the copy of f." (see https://eel.is/c++draft/intro.races for more detail on that "synchronizes with"). Since the code in question is using std::thread, even if the operating system did not have the relevant guarantees, the C++ standard library would have the required memory barriers.
> An object whose identifier is declared with no linkage and without the storage-class specifier static has automatic storage duration, as do some compound literals. The result of attempting to indirectly access an object with automatic storage duration from a thread other than the one with which the object is associated is implementation-defined
(I bet in practice almost all implementations behave as an equivalent C++ would, as per your notes, so only the ordering is relevant. But people maintaining C implementations have on occasion shown themselves to be their users' enemies, so don't quote me on this!)
Atomicity and cache coherence are different things. "Atomic" means that the access to the element will be done in a single access and can only show exactly the state resulting from any other atomic access to the same value. For C syntax variables, this pretty much is limited to multi-word access (you also sometimes talk about atomic compare-and-set instructions, but those don't appear as part of the language per se).
Cache coherence is actually guaranteed on almost all systems, you don't need to worry about it on anything big enough to be running Windows (in the embedded world we get to fight it though).
The other demon in this space is memory reordering, but atomics don't speak to that at all.
> It is extremely difficult for programmer-users to report bugs against the Windows API (we're supposed to direct you to Feedback Hub, but you may as well transmit your message into deep space). I've filed OS-49268777 "SRWLOCK can deadlock after an exclusive owner has released ownership and several reader threads are attempting to acquire shared ownership together" with a slightly reduced repro.
> Thanks for doing your homework and creating a self-contained repro, plus pre-emptively exonerating the STL. I've filed this OS bug as a special favor - bug reports are usually off-topic for r/cpp. The microsoft/STL GitHub repo is the proper channel for reporting STL misbehavior; it would have been acceptable here even though the root cause is in the Windows API because this situation is so rare. If you see STL misbehavior but it's clearly due to a compiler bug, reporting compiler bugs directly to VS Developer Community is the proper thing to do.
Sometimes I get pleasantly surprised, but generally speaking the internal incentives are skewed against, the primary focus is whatever the roadmap is followed by tickets from paying clients, public bugs generally have a very low hit ratio so they're unrewarding, unless you manage to snipe one of the company's employees (either nerd-snipe or interest / shock them enough to raise the issue internally) it's like playing the lottery.
Despite Google partly losing its marbles recently, reporting bugs to Chromium still works very well.
What works to some degree are dedicated maintenance teams providing development support. If their main task is fixing bugs and they are evaluated on this basis, support tickets reporting real bugs have a good chance to receive the required attention.
However, there is always the temptation for management to redirect resources from those teams. But at least in the B2B area costly customer escalations can remind management of the importance of good maintenance.
The Logic team, however, has been helpful, and in one or two cases (that were discussed in musician's forums) went out of their way.
My literal job for the last part of my time at AWS was "help triage bugs in the AWS SDK." This is by far the best repro I've ever seen for such an in-depth event.
Most of the tickets you get in open ticket trackers are incomplete [ https://github.com/boto/boto3/issues/4011 ] nonsensical [ https://github.com/boto/boto3/issues/4018 ] or weird [ https://github.com/boto/boto3/issues/358 ].
I can't imagine living in this hell. When I find bugs in Linux, I E-mail the actual engineers directly and get responses in under 24 hours: https://lore.kernel.org/lkml/Zcb3_fdyJWUlZQci@gmail.com/
Nice circle, now the reader will only hav to fill in the rest of the owl.
The benefit of the open open development model of Linux is that you don't have to have "someone you know" on the inside or essentially spam whatever contacts you can dig up until you find someone who has pity on you. You have actually publicly available developers (from many different companies, including hardware manufacturers) as well as real bug trackers where you can find if other users have had the same issue.
I have had a lot more things fixed in Windows or MSVC from nagging devs on there than from reporting through any official channel.
I have many complaints about Win32 (having half a dozen different error code types, for one), but outright bugs are really rare.
This was great for developers (real support was available, and spam was discouraged), and it was great for MS (they found real issues that affected paying customers and they got feedback to improve their documentation).
I wonder whether this program still exists. I have the impression the overall quality of MS documentation has declined.
Anectodically, I tried to get support for something unrelated: I’m trying to use IMAP sync support in Outlook.com, but it refuses to work properly with iCloud-IMAP but doesn’t give any error message either. As a paying M365 subscriber I expect proper support. I tried over five times to get support, and every time it ended in frustration. Every time I got someone who either doesn’t understand to product, says IMAP support is not available and deprecated (it is not! I was able to set-up a different IMAP provider just fine), or I got redirected to the Windows or Office support team, who then couldn’t help me and closed my ticket.
I cannot prove it, but I bet there is an internal number of replies before they can close without penalty.
What’s left?
Some issues are also due to the ecosystem itself. Avoiding POP3 goes a long way for example.
There are too many variants, all only vaguely compatible.
The iOS integration requires enrolling one’s phone in the email provider’s MDM, at least to some extent. This is nice if you’re an admin, but it’s not so nice if you’re a user who uses (in accordance with company policy!) a personal device.
The integration with Mail.app is abysmal. It makes my memories of Eudora seem happy.
Signing in is a real PITA.
The spam classifier is comically poor. I’m honestly surprised that (hundreds of) millions of dollars aren’t lost every year when a (paying, enterprise) customer emails someone at a different business (from the native app!), they reply with an utterly non-spammy reply, and the reply is classified as spam. Seriously, the open source spam classifiers from the early 2000s understand threading — how can Microsoft fail to classify individual replies as not-spam? Google is far better. Fastmail is far better. Everything is far better.
I will give MS some credit: the iOS Outlook app is actually pretty nice.
Outlook debuted Cached Exchange Mode in the early 00s, popularizing "offline first" before it was known as that.
Now: The "new" Outlook can't even show folder unread counts correctly, even when fully online. It seems to only load a small subset of messages locally, only populating folders when you scroll past the point it loaded. (In classic Outlook, this was a setting—I understand loading all mail was not enabled by default—but it could be enabled. No longer.) It sometimes gets stuck where it won't show new mail until restarted. (Gmail has this bug too.) It forgets open mail windows when restarted. It forgets expanded folders in the folder pane when restarted (but only sometimes).
Microsoft removed the ability to show the mail/contacts/calendar navigation bar below the folder pane, and forced it to be shown on its own huge vertical bar, almost all of which is wasted space. For good measure, they did this in classic Outlook as well as "new" Outlook. There was massive backlash to this, and Microsoft plowed forward anyway. On Windows there is/was a registry setting to revert this (but intentionally, no user-facing setting). I have not checked on Mac.
To the sibling comment: Outlook for iOS is indeed great, probably only because it was an acquisition. It is not in Microsoft's DNA to build an app like this themselves anymore.
As an admin: Microsoft seems to redo the Office 365 admin interface every 2-3 years. It is an incomprehensible mess. I am also a Google Workspace admin for the past several years, and theirs is far better, and it's more or less stable over the long term.
Office 365 has been hacked by state actors recently.
I still like the Outlook UI and feature set better than Gmail (despite the "new" Outlook being a major regression), so I begrudgingly stay with Outlook/Exchange because I dislike it less than Google Workspace.
Fastmail does not give signs that they are a relevant company—they created JMAP and basically did nothing with it. Why not make a first-class Windows/Mac client, offline first, with powerful organizational features, like the Outlook of yore? Or at least, contribute to adding first class JMAP support to Thunderbird? This is your sole business.
It really is a tough choice. Microsoft does have the best products for their and mobile platforms (including iOS,iPadOS) with really good Offline Features and so on. But it still is Microsoft and Exchange behind...
A better product is Fastmail. But also US Servers and AUS company and a lot of downsides concerning offline usage and other privacy points.
What else? I try to avoid Google as much as I can.... Well there is Proton with it's Proton Suite getting better and better. There is german mailbox.org (worse 2FA) but overall a good privacy mail provider with own domains. There is migadu.com from Switzerland with EU Servers (rented at OVH Data Centers) and some other players like Tuta and others...
At any point of this century, the response to those support incidents from MS was always to deny a problem, and if it's a known one, to try to gaslight the customer in a direction contrary to solving it.
I have seen organizations lose way too many people-hours trying to satisfy the MS support and apply what it recommended. That when the real solution was often reachable in a hour or two of research on 3rd party knowledge bases.
Sadly the hotfix had its own little bug: installing it took about 20 hours. Office productivity was rather low for the rest of the day. I don’t know everything that goes on under the hood with Microsoft’s installers, but wow they’re slow.
I, blissfully, don’t use Visual Studio on Windows any more :)
I remember being astounded by the technical depth of a particular answer and looking up the the engineer on LinkedIn. He had 20+ years of experience at Apple - and it showed in his answer.
[0]https://mullvad.net/en/blog/bug-in-macos-14-sonoma-prevents-...
:'‑(
For example Microsoft's *own* Pluton-enabled platforms fail Windows' Device Health Attestation checks due to an incomplete chain (https://call4cloud.nl/2023/04/are-you-there-intune-its-me-ha...).
- Create new feedback tracker
- Direct feedback to tracker
- Stop paying any attention to tracker
- Tracker is hundreds of pages of users shouting into the void, and much of it out-of-date
- Delete everything
- Create new feedback tracker...
Topic: Can't select blue wallpaper
Body: Hi, for some reason I can't select blue as a wallpaper colour. I can select any other colour, just not blue. I have Enable All Colours ticked. Anything else I'm missing?
Reply #1: Hi I have this problem too, does anyone know?
Reply #2: Same problem here.
Reply #3: Same problem, I can select any colour except blue.
Reply #4: Hi there User344925. Let me first say that your feedback is incredibly, impossibly important to us. I'm Bob and I'll be your Licensed Support Person today. Let's get started and see if we can solve your problem. I understand you're having trouble setting the colour blue for your wallpaper in personalisation settings. This is a common problem to have, and I'm pleased to say there is a simple fix available. Open Settings -> Personalisation -> Advanced Settings -> Advanced Personalisation Settings, and tick "Enable All Colours". Now you should be able to set any colour you like. Please remember to mark this question as SOLVED at the top and mark my post as the Approved Answer if this solved your problem. Have a great day!
Replies #5-#250: Users with the same problem.
What you really need is a way to report problems which is high friction. You have to submit a git pull request of your ssh public key so you can transmit your bug report via sftp. Now they only get bug reports from people who can figure out how to do that and can actually pay attention to them because it filters out all the spam from people asking Microsoft how to connect their Android to a Mac.
They could go and spend 200 million a quarter on decent customer support without making too much of a dent in their financial line.
[1] https://www.microsoft.com/en-us/investor/earnings/fy-2023-q2...
The point is to exclude people who don't know what they're talking about, not people who can't pay registration fees or produce documents.
I also found a bug in a win32 API and the feedback hub told me to reboot my PC
https://github.com/bagder/emails/blob/main/2015/2015-06-08.m...