Live data from Hacker News

Bug in reader/writer locks in Windows API

old.reddit.com

61–70 of 142 posts

Re: Bug in reader/writer locks in Windows API

#61

Earlier quoted context omitted.

$200M is approximately $0.15/user. How much support do you expect to get for that?

How many of these 1 billion users actually need support? Only a tiny fraction.

Uh... what? If there was a free number you could call to get competent technical support, people would spend their entire day on the phone with it instead of reading documentation or hiring IT staff.

Re: Bug in reader/writer locks in Windows API

#62
post #32

> 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). :'‑(

Unfortunately, the opposite of that is everybody trying to file random things that aren't actionable. See the collection of mail sent to the curl maintainer…

https://github.com/bagder/emails/blob/main/2015/2015-06-08.m...

Re: Bug in reader/writer locks in Windows API

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

Re: Bug in reader/writer locks in Windows API

#64

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…

How did you patch the kernel? Like how is that possible?

With a hex editor, debugger, and skills that most developers these days seem to lack.

I patched the kernel in memory first, using a kernel debugger, to verify my fix worked before editing the file on disk.

Re: Bug in reader/writer locks in Windows API

#65
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…

You seem to have missed the part where an actual MS employee confirmed it was a bug in their API.

Re: Bug in reader/writer locks in Windows API

#66

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…

How did you patch the kernel? Like how is that possible?

Windows xp and server 2003 sources (though incomplete AFAIR) were leaked back in 2020.

Re: Bug in reader/writer locks in Windows API

#67
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…

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

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.

Re: Bug in reader/writer locks in Windows API

#68
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…

> 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. 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 main thread is running the child thread creation in an endless loop, repeatedly setting numThreads to 0 and to 5, back to 0 and to 5 again. Can the caches of the CPU's consistently keep up with the changes?

Re: Bug in reader/writer locks in Windows API

#69
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…

You seem to have missed the part where an actual MS employee confirmed it was a bug in their API.

He just read OP's code, the C++/STL Standard, and the Microsoft Learn document and made that conclusion. That's a rather haste determination. Unless he read the actual Windows lock implementation and found a bug there, I don't think his conclusion is correct.

Re: Bug in reader/writer locks in Windows API

#70
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…

`numThreads` is written before the child threads that read it are started, so there is an explicit happens-before relationship and no data race. Before `numThreads` is reset, the child thread are joined.

There is no bug in the program, it is legal to use non-atomic variables across threads as long as they're correctly sequenced.

Post reply on HN