Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Bug in reader/writer locks in Windows API

old.reddit.com

Post reply on HN
Print view View original

81–90 of 142 posts

Re: Bug in reader/writer locks in Windows API

#81

Post by ww520 »

cesarb wrote: post #77
ww520 wrote: post #63

The program has a bug. It's mixing atomic and non-atomic variables in the yield() checking loop. Non-atomic variables have no guarantee on cache consistency for different threads. This can cause the loop to run forever. struct ThreadTestData { int32_t numThreads = 0; std::shared_mutex sharedMutex = {}; std::atomic readCounter = 0; }; // child thread DoStuff() { data->readCounter.fetch_add(1); while (data->readCounter…

> there's no memory barrier instruction to force its new value (5) to propagate to all CPU's running the threads. 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, befo…

Based on the Windows CreateThread API [1], it doesn't say anything about memory synchronization guarantee. Does it do internally?

[1] https://learn.microsoft.com/en-us/windows/win32/api/processt...

Re: Bug in reader/writer locks in Windows API

#82

Post by cesarb »

ww520 wrote: post #75
zozbot234 wrote: post #72

Earlier quoted context omitted.

> repeatedly setting numThreads to 0 and to 5, back to 0 and to 5 again. 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.

CPU’s not running the main thread don’t care about the execution order of the instructions of the main thread. 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. When a new thread lands on a CPU with the old 0 cache value, it will hang.

> CPU’s not running the main thread don’t care about the execution order of the instructions of the main thread.

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.

Re: Bug in reader/writer locks in Windows API

#83

Post by secondcoming »

AndrewStephens wrote: post #22

Subtle bugs in Reader/Writer locks do not surprise me. I worked on an in-house implementation based on Win32 (before C++11 and std::shared_mutex) and my recollection is that although the implementation sounds simple it is exceedingly easy to make subtle mistakes. 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,…

If you have many readers and a single writer there's no point in the readers blocking each other.

Re: Bug in reader/writer locks in Windows API

#84

Post by cesarb »

ww520 wrote: post #81
cesarb wrote: post #77

Earlier quoted context omitted.

> there's no memory barrier instruction to force its new value (5) to propagate to all CPU's running the threads. 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, befo…

Based on the Windows CreateThread API [1], it doesn't say anything about memory synchronization guarantee. Does it do internally? [1] https://learn.microsoft.com/en-us/windows/win32/api/processt...

That MSDN documentation is unfortunately silent on this, but the example in the documentation (at https://learn.microsoft.com/en-us/windows/win32/procthread/c...) only makes sense if the operating system guarantees the ordering.

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.

Re: Bug in reader/writer locks in Windows API

#85

Post by AndrewStephens »

secondcoming wrote: post #83
AndrewStephens wrote: post #22

Subtle bugs in Reader/Writer locks do not surprise me. I worked on an in-house implementation based on Win32 (before C++11 and std::shared_mutex) and my recollection is that although the implementation sounds simple it is exceedingly easy to make subtle mistakes. 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,…

If you have many readers and a single writer there's no point in the readers blocking each other.

True, but in my experience the overhead of std::shared_mutex outweighs the benefits. Other approaches include:

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

Re: Bug in reader/writer locks in Windows API

#86

Post by pugz »

amluto wrote: post #9

Once upon a time, you could buy various things from MS that came with support incidents. I had an MSDN subscription that came with two per year. Using an incident got you an actual support engineer who would be helpful and escalate issues if necessary. And, if your issue turned out to be a real bug of any significance in an MS product, your support incident would be credited back. This was great for developers (real…

Apple still has this program. I haven't worked as an iOS developer for a few years now, but I remember their paid "technical support incidents" were extremely high quality. It was $50 per incident, but I think that was just to reduce spam.

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.

Re: Bug in reader/writer locks in Windows API

#87

Post by GrumpySloth »

jansan wrote: post #13
masklinn wrote: post #10

Earlier quoted context omitted.

I generally assume that's the case for any large company. 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 th…

> I generally assume that's the case for any large company. Despite Google partly losing its marbles recently, reporting bugs to Chromium still works very well.

Not my experience. When I report a bug in Chromium, it’s usually quickly verified as legit, and then left for dead forever.

Re: Bug in reader/writer locks in Windows API

#88

Post by OsrsNeedsf2P »

zozbot234 wrote: post #79
userbinator wrote: post #54

I'm curious if this also occurs in WINE's implementation. 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…

The code is here https://source.winehq.org/source/dlls/ntdll/sync.c#0474 and it uses compare exchange operations throughout, so it should be unaffected. 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.

The WINE team has expressed frustrations in the past about implementing the Windows APIs to spec... Only to find out Microsoft didn't

Re: Bug in reader/writer locks in Windows API

#89

Post by OsrsNeedsf2P »

pugz wrote: post #86
amluto wrote: post #9

Once upon a time, you could buy various things from MS that came with support incidents. I had an MSDN subscription that came with two per year. Using an incident got you an actual support engineer who would be helpful and escalate issues if necessary. And, if your issue turned out to be a real bug of any significance in an MS product, your support incident would be credited back. This was great for developers (real…

Apple still has this program. I haven't worked as an iOS developer for a few years now, but I remember their paid "technical support incidents" were extremely high quality. It was $50 per incident, but I think that was just to reduce spam. 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…

Mullvad had to go public[0] with a security leak before Apple even responded to their reports in the release candidates for MacOS 14

[0]https://mullvad.net/en/blog/bug-in-macos-14-sonoma-prevents-...

Re: Bug in reader/writer locks in Windows API

#90

Post by Nition »

Avamander wrote: post #47
Nition wrote: post #43

Earlier quoted context omitted.

There's nothing worse than having a problem with a product, finding 250 other people on the feedback tracker that have had the same problem over the last three years, and the only official response is some support person on the first page who's saying your feedback is very important to us, and have you tried [troubleshooting that won't work]?

At that scale they have 250 000 people that didn't try the obvious troubleshooting first though. But yes, it's annoying.

They often seem to end up like this:

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.

Post reply on HN