The case of the critical section that let multiple threads enter a block of code
devblogs.microsoft.com
devblogs.microsoft.com
I understand the temptation of "Let's just use an int return type, that way later on we can easily let them indicate different flavours of error if we want". But then you leave yourself open to this.
The full-BDSM approach is for every component Foo that works with callbacks like this to define its own FooResult type with no automatic type conversions to it. In C, enum suffices, though a determined idiot can just cast to override; using FooResult as defined below makes it harder to accidentally do the wrong thing:
enum FooResultImpl { FOO_SUCCESS_IMPL, FOO_FAILURE_IMPL };
struct FooResult {
enum FooResultImpl USE_A_CONVERSION_FUNCTION_INSTEAD_OF_ACCESSING_THIS_DIRECTLY;
} FOO_SUCCESS = { FOO_SUCCESS_IMPL }, FOO_FAILURE = { FOO_FAILURE_IMPL };
Then again, it's been years since I used this kind of C, so maybe my instincts are rusty here (no rust-pun intended!)
While it's annoying boilerplate for the Foo component to have to write, I don't think clients of Foo see much additional complexity. They can still write "return FOO_SUCCESS;", etc.
Essentially you got a generic error_code struct which has two members, an int to hold a given error value or zero if there's no error, and a reference to an error category which helps interpret the error value.
Effectively the error category is an interface, so in C terms it would be a reference to an error_category struct filled with function pointers.
There's then some machinery which allows you to compare specific error codes to generic error conditions like "file not found", abstracting away the specifics of the error code.
It's not problem free[3], but I've used this pattern in languages like Pascal and felt it worked well for me.
[1]: https://www.boost.org/doc/libs/1_82_0/libs/system/doc/html/s...
[2]: https://en.cppreference.com/w/cpp/header/system_error
[3]: https://akrzemi1.wordpress.com/2017/10/14/error-codes-some-c...
There's also the scaffolding around it, like the way error codes compare for equivalence[1] against error conditions in a symmetric way[2].
The result is you can easily add new a domain-specific error category, and error codes using your new category can be fed to existing code and they'll do something sensible without further modification. Your code with the new category could even be loaded at runtime[3].
Not something you can do with plain sum types, ie tagged unions in C. At least as far as I know, though I'm no expert.
[1]: https://www.boost.org/doc/libs/1_82_0/libs/system/doc/html/s...
[2]: https://www.boost.org/doc/libs/1_82_0/libs/system/doc/html/s...
[3]: though there might be dragons, see my previous reference
He’s kind of yelling slowly and kind of talking in some East European language, and yet you can kind of understand what he’s saying.
(And if I ever have to open an MFC codebase again, I’m going to be thinking of how Nosferatu springs up naked and rotting from his coffin)
Now for the team writing say Excel, they have a typed language so they don't need to distinguish booleans from counters, but their language doesn't have or encourage distinct Row and Column types, those are both just integers, so "Apps Hungarian" uses the name to annotate variables with such information, clInsert is the column while rwInsert is the row, if I am reviewing code which is to inspect columns and it checks clFooC, clBar and rwBaz well why is it using a row number for a column? That warrants closer inspection.
Unfortunately, this practice was divorced from its rationale and infected teams at Microsoft who had a typed language and didn't have kind information beyond that, but felt the need to use this notation anyway, producing "Systems Hungarian" where we mark out pFoo (it's a pointer named foo), and lBar (it's a long integer named bar). This is very silly, but at this point it has infested an entire division.
One further nuance is that return types of APIs also affect the naming of the APIs in Apps Hungarian. Boolean return values are typically called f for flag. An API that returns a boolean would be called FApiFunction - the capital F (camel case is used) at the beginning of the API name indicates that the API returns a boolean. If you write code that stores the return value in a variable that doesn't begin with an 'f' then any code reader would see the mismatch.
FTA, one problem is that the API returns a ULONG where 0 is failure and non-zero is success.
But the developer thought that the API returned an NTSTATUS error code, where 0 (== STATUS_SUCCESS) is success and non-zero is failure. This is the opposite of the documented return value for the API in question.
In Apps Hungarian, the naming of the API would be obvious that it didn't return an NTSTATUS variable vs a ULONG. Systems Hungarian lost that tidbit somewhere. So developers must be more diligent about knowing the return values of various APIS (but assuming NTSTATUS usually works, expect in this case).
But Apps Hungarian rarely names variables with native type prefixes. Variables are more than just the native underlying type (ULONG, int). Variables have a purpose. The return type of strlen or Win32 equivalent is an int or for more modern-designed APIs, an unsigned int (since string lengths can't be negative). In Systems Hungarian, you'd name the variable ulen. But in Apps Hungarian, strlen returns an int representing the count of characters. Apps Hungarian uses 'ch' for character and 'c' to denote a count. So the variable used to store the return value from strlen would be cch. (And in Apps Hungarian, you'd actually alias strlen to CchStrlen so you'd know just looking at the API that it returns a 'cch', count of characters, and you'd need a CCH type to store the return value).
I'm not sure you could place utf characters 'atop' of 'ch', more alongside/as well.
Win32 had support for wide-characters (UCS-2, I believe) which mapped to WCHAR/'wch'. They also support, in the windows.h headers, for compiling code to support both single-byte characters and wide characters using one code base via macros/typedefs with the TCHAR type et al. This resulted in code that used 'tch' as the underlying character type.
So code that only wanted 16-bit Unicode would have variables like 'wch' and 'cwch'. Code that supported both single and wide characters would have 'tch' and 'ctch'.
UTF support in Win32 appeared after I stopped Win32 programming. Looking up a few Win32 UTF-related programming docs, MS seems to have shoved UTF-8 support under the old Win32 Ansi-character/single byte string APIs. I'd probably create some Utf-type related APIs that called down to the Win32-related APIs. But it would also depend on the UTF-8 API design (strlen doesn't map 1-1 to UTF chars anymore) - parsing UTF-8 characters/code points is different than single-byte character set. I'd guess there'd be 'uch' for a UTF-8 character, probably a 'ucp' for a UTF-8 character point, etc.
[edit] You wouldn't just repurpose single-byte string code with 'cch' for UTF-8 since the semantics aren't the same (not even if you include double-byte character sets like Japanese), one UTF-8 character can be one or more bytes which typical one/double-character set string code doesn't deal with.
'sz' is a null-terminated string, so naming a variable 'szOutStr' is redundant since 'sz' automatically tells you it's a string. 'szOut' would be fine. The 'pp' at the front tells you it's a pointer to a pointer. You typically only use a 'psz' to refer to a string (I usually used 'sz' since most of the time, you're dealing with allocated memory and that means I had a pointer alread).
But if you want to reallocate the string memory , you'd have to take the address of the pointer (&psz) which meant you'd have a pointer to a pointer to a string: 'ppsz = &psz;'
When dealing with a 'ppsz', you'd know that you couldn't pass 'ppsz' directly to a function that handled 'psz' variables, you'd need to dereference ppsz (*ppsz) to get a 'psz' to pass to those functions. Useful when dealing with liberal C compilers, not as useful with stricter C++ compilers.
MSDN Library seems to have totally nuked any pre-2005-ish URLs and their search engine is just bad as well (archive.org seems to have gotten tons of 302s/301s when crawling for this article). So you'll have to make do with a copy of the MSDN Library article at purdue.edu.
see https://learn.microsoft.com/en-us/previous-versions/visualst...
… I don’t know, I kind of like my compiler catching issues like these rather than having to do it by eye and hoping for the best
I used to have different hungarian prefix conventions for local, global, argument and instance vars.
I actually still use them sometimes
You know how in C you can write "int k" or "char v" ? That's types. BCPL doesn't have that. Yes that's in the original K&R book, no you didn't need to wait until 1989 to have types in C, they were obligatory from the outset.
But C# is extremely beautiful, you can read and write it like a novel.
Segoe UI is also not the worst font. I give it a 7/10.
I find the title case ugly, not a fan of semicolons, nor the new keyword. Guess I'm too used to Kotlin and Linux/Mac fonts
It’s a conjunction of all 3 things.
Seriously? It's basically Microsoft-flavoured Java, and has much of the same horrid verboseness and architecture-astronaut overcomplexity.
K&R, early UNIX style, BSD is what I find "extremely beautiful".
I see/hear jokes often that it's Microsoft Java, and yeah definitely granted that it comes from the same origins, but it's got some of the greatest tools I've worked with, best documentation, and best support I've seen. Anytime I've asked myself "Can I do this with C#?", the only times the answer has been "no" is when looking for a C++ constexpr equivalent (closest you get is build-time code-gen, which ain't half bad), or discriminated unions (inexplicably).
I'm convinced that anyone who complains about C# has never used it.
While MS isn't utterly technically bereft obviously, and they had some correct choices like backward compatibility and usability relative to imbeciles like IBM, just look at the botched execution of Win8, their security disasters, and generally superior products that got nudged out by beta-ware quality releases.
But yes the mix of that plus camel case, make things confusing
Why not link it?
But they just did bug tracking and other jira type stuff in MS land, right?
They also did Trello. No idea if they got paid big for spinning off/selling Trello.
There was also some vncviewer/rdp client-type thing (copilot) which was more consumer-friendly - relatively quick install/didn't spew files/garbage all over your disk/registry.
He also co-founded stackoverflow, so I thought there was a big payday when stackoverflow got bought.
https://computernewb.com/~lily/files/Documents/NTDesignWorkb...
Nobody likes Hungarian though.
Microsoft saw all the footguns available and decided to just incorporate every one they possibly could and invent new ones whenever possible. For extra hilarity they put all this in a monorepo for all the good that has /never/ apparently done for them.
The title was just a little disengenuous - the critical section got all the blame!
The end goal is to initialize something no more than once, right? But the technology they're using (wrongly, but it did exist and they were clearly aware of it) to make a critical section does initialize a thing exactly once.
I also don't understand the use of SRWLock here, or rather, I sort of do but it's a hole in Microsoft's technology stack. SRWLock is a complicated (and as it turns out, buggy, but that's not relevant here) tool, but all we want here is a mutex, so we don't actually need SRWLock except that Microsoft doesn't provide the simple mutex, so this really is what you'd have written at the time† - conjure into existence the over-complicated SRWLock and just don't use most of its functionality.
† Today you should do the same trick as a Linux futex although you spell it differently in Windows. It's also optionally smaller, which is nice for this sort of job, a futex costs 4 bytes but in Windows we can spend just one byte.
This makes it in theory excellent for implementing this precise feature which you will see in for example C++ (the MSVC implementation is, in fact, just an SRWLock) or Rust
So immediately it's over-complicated for a mutex because we don't need it to handle multiple readers, we're never taking its shareable reader lock, we (as you see in Raymond's example code) just rely entirely on the exclusive lock which is about 10% of its function.
SRWLock is the size of a pointer - because of course in fact it is a pointer, to a lazily allocated structure - with some flag bits squirrelled away at the bottom of the pointer.
The exciting bug is that at some point whoever was implementing this for Windows screwed up while implementing the (necessary for performance) unfair lock stealing strategy.
Brief aside about "lock stealing": If I want to take a lock, and I notice that somebody else just released that lock, I can just take it and we're done - this is unfair because there may be a queue of waiters, and I cut in ahead of them, but it has better performance. This is a simple and common feature for a mutex - fair locks are sometimes useful but definitely should not be the default.
Now, in SRWLock the lock stealing code doesn't remember whether you're trying to take the shared reader lock or the exclusive writer lock. So, it just always steals the exclusive lock - even when that's not what you wanted. As a result in code relying on SRWLock if you take the reader lock and then wait on somebody else to also get that lock (which they should be able to because it's a shared lock) this sometimes deadlocks. Oops.
It looks like the Rust standard library migrated away from SRWLOCK on account of this issue [0].
There's a Microsoft internal ticket which I can't read (but STL can because he's a Microsoft employee) but there's also a GitHub issue for STL (this time the stdlib, a GitHub project, although it was opened by the person) https://github.com/microsoft/STL/issues/4448 and it's confirmed in that issue this is a bug in SRWLock, thus no work for STL† (either the project or the person).
It's not unusual (especially in C++ but this happens anywhere) that people would rather contort themselves to believe crazy things than accept that it's just a bug.
† Both Windows and C++ holds themselves to a very low standard, "Yeah, it's broken, too bad" is acceptable. Once you've determined that it's a bug you're done.
In terms of size, the pointer is bigger than you'd want - it's no pthread_mutex or the analogous Windows data structure where it's a multi cache line disaster of a data structure - but it's clearly worse than a futex or WaitOnAddress solution.
(The tracing handler getting registered twice is just a canary.)
I don't really care about narrower futexes, what I do wish Linux had was 8 byte futexes. It's at times hard to cram enough state into 4 bytes for more complicated things.
E.g. for postgres' buffer state it'd be very useful to be able to have buffer state bits, pin count, shared lockers and the head of the lock wait queue in one futex "protectable" unit.
There are also ABA style issues that would be easier to defend against with a wider futex.
Took me a while to understand as well. They unregister the handler at the end of the critical section, so the requirement is not "no more then once ever", it's "no more than once at the same time".
FTA the InitializeCriticalSectionOnDemand function calls RtlRunOnceExecuteOnce to perform the bookkeeping work of running a function once. RtlRunOnceExecuteOnce is passed a function pointer to do the actual grunt work of running once, in this case, initializing a critical section.
The problem is that RtlRunOnceExecuteOnce EXPECTS the function pointer to return non-zero for success and zero for failure.
InitializeCriticalSectionOnce ALWAYS returns STATUS_SUCCESS (== 0) because it CAN'T FAIL because InitializeCriticalSection can't fail. So InitializeCriticalSectionOnce is telling RtlRunOnceExecuteOnce that it failed when it actually succeeded.
NT kernel devs deal mostly with APIs that return NTSTATUS values, where 0 (== STATUS_SUCCESS) is success and non-zero is a failure.
The problem comes when another thread calls the same code. The RtlRunOnceExecuteOnce API believes that the underlying code failed before, so it tries initializing again, initializing the same critical section again (even if the crit-sect is held by another thread currently).
In goes another thread since the critical section has just been initialized and you've got multiple threads in the same 'protected' code now. Might as well not have a crit-sect.
This is unfortunately true of Unixes as well.
The one "Unix" API I know that returns > 0 on error is pthread, which returns +errno directly (but still 0 on success).
Which APIs return 0 on error? I can't think of any.
My guess is this is usually true in practice, but in C and C++ dereferencing a null pointer is UB so you really can't assume that.
What I'm saying is there isn't an ergonomic issue with NULL error return APIs. You might prefer algebraic error types (in other languages). I'm not even sure I do. NULL return APIs are clean and simple. If you do (accidentally) forget to check for NULL return code, you will for sure get a segfault on first access in practice (not on embedded platforms maybe). The compiler can't remove any following code unless it can prove only NULL will ever be returned. Don't fall trap to UB FUD.
It can reorder code, though. For example, it can change
int *x = malloc(sizeof(int));
*x = 42;
if (x)
launch_the_nukes();
int *x = malloc(sizeof(int));
launch_the_nukes();
*x = 42;Most of the UB FUD are contrived examples that hardly happen in practice. What I'm saying is not that you should write bugs, but that bugs (especially segfaults) will immediately turn up in practice.
Unix APIs are a couple of syscalls and library functions. If you want to achieve more (like video, audio...) you have to resort to "optional" libraries that may have bad APIs too, but they aren't core Unix and can be replaced by something that is hopefully better.
The underlying NT (microkernel) api (as found in used-to-be-undocumented NTDLL) returns STATUS_XXX from functions. With STATUS_SUCCESS being 0. This is still consistent as far as I know.
Win32 API, technically being a "subsystem" on top of NT, exposed to the user whatever convention its users were used to. Coming from Windows 3.1, it used BOOL (int actually) where FALSE was 0 and TRUE was 1, in a (mostly) consistent manner for success/failure.
An additional wrinkle may be APIs that return a HANDLE (or address for memory allocators) or NULL, which again if you treat as ints return non-zero for success and 0 for failure.
Posix Subsystem, on the other hand, could return 0s and non-zeros as desired.
So, if you knew Windows and thought about the API you were calling it made sense if you thought in terms of the types defined (BOOL, HANDLE, etc. for Win32) instead of 0 and non-zero.
No idea if it this is still consistent over the surface of the entire exposed API set now. So much accretion of cruft over the years under the eyes of an uninterested leadership has led to a lot of fragmentation and hence deterioration in the developer experience.
Except some Win32 API functions return INVALID_HANDLE_VALUE (= -1) on failure! Of course, there is a Raymond Chen blog post: https://devblogs.microsoft.com/oldnewthing/20040302-00/?p=40.... Win32 error handling is indeed a mess...
Looks like the RtlRunOnceExecuteOnce API and structures appeared in Windows Vista.
So one would have to do some code archaeology and dig into the Windows Git repo to look at the commits for RtlRunOnceExecuteOnce and see what the typical use cases were. For some reason, returning 0 for failure made sense. Given the pervasiveness of NTSTATUS, I have no idea what made them choose a non-NTSTATUS return value. Maybe there was another callback-type API which did something similar?
If one dev thought that RtlRunOnceExecuteOnce required NTSTATUS, then I'm sure other devs will have thought the same thing so I'd expect more such bugs in the current codebase and in the future. That's why Microsoft static code analyzers (PREfix, PREfast) would flag such footguns.
In cases where there is only one successful result, 0 makes sense. Then nonzero can be a range of error codes. In cases where there is multiple successful results and only one failure, 0 means failure; malloc() is the stereotypical example of this.
A valid criticism might be that one shouldn’t use true to mean success.
Could this have been avoided if they had used Rust? Or is this a bug that can happen even in Rust code too?
This specifically is the kind of bug that is avoided with strong typing. The compiler screams at you when using the wrong return type. For example, if a callback expects a Result type, you must return a Result type, not some random int-like value whose definition of success and failure is arbitrary.
If so, yw
If not, bye