- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
volatile bool rfist;
Admin
This is not a compiler "mistake". The code is just plain wrong.
And this "while (thingIsDone == false)" explains a lot. A self declared developer, who doesn't understand booleans, for sure does not understand multi threading.
Admin
The error is probably that Calvin forgot to check whether thingIsDone == FILE_NOT_FOUND
Admin
And what about having an infinite loop with no wait/sleep? That's also a WTF.
Burn 100% CPU in one thread until you have finished isn't nice.
Admin
so that code makes an asynchronous call, and then waits for the answer. should there not be a join-method for that?
also, it never actually checks for success, but "it finished" is success enough?
Admin
TRWTF is using different builds in different environments. IT should be the same binaries that are utilized across all deployment environments (including test, dev validation, etc.)
Admin
Yes, and by implication he went ahead and automated the build process and went live with it without a representative test environment in place to check for weirdness like this.
I mean from all the descriptions he should have expected the, umm, unexpected.
Admin
Indeed...
Whenever you change something major like this, expects gremlins to show up
I mean NOBODY expects the Spanish inquisition... (Sorry could'nt resist)
Admin
What about the code in question demonstrates a lack of understanding of booleans?
Admin
Awesome.
Reminds me of a company I worked for in the late '90s, using Borland C++ for everything. That compiler was way better than Microsoft's except for one thing: they had multiple code generation bugs in their optimization code. For a while I'd write up each one and send it to them - minimal repro, with detailed notes on how the assembly was wrong. But each new release would have a different set of bugs. Since our code had to run 24/7/365.25, we ended up running debug builds in production. All the time. :/
Admin
I'm curious. What is "different" about C#'s volatile when compared to Java's? Both just provide a method of telling the compiler that the data the variable references may change outside of the code in question like another thread or interrupt handler. That volatility means that certain optimizations must be skipped.
The same thing exists in C and C++ for the same reason.
And now I realize I'm posting questions about Remy's comments and not the actual article.
Admin
C#'s volatile is a compiler directive to not optimize. Java's volatile is a runtime directive to not cache. Depending on how similar you say the two languages are in general, you could argue they're the same, or your could argue that they're different.
Admin
In this case, I would argue that caching is the only optimization that should matter. It seems to me that inlining an instance variable (especially accessed from more than one function) is the wrong thing to do at any optimization level, as it leads to radically different behaviors (i.e. exactly this bug).
Admin
"That compiler was way better than Microsoft's except for one thing: they had multiple code generation bugs in their optimization code."
Apparently your definition of "better" differs significantly from mine.
Admin
What's different? Possibly nothing, I suppose. volatile in C, back in the day as I remember (c 1990), was a solution for things like double-ported memory and other cases where the C memory model didn't guarantee consistency.
It's not really all that useful today, afaik, in any language (except, I suppose, and ironically, embedded C -- for its original purpose). Eric Lippert has an interesting summary, and ends with the recommendation that you don't want to use volatile so much as to use the appropriate locking/monitor/read-write barrier/whatever.
Which, I believe, was Remy's (and the submitter's) original point.
Admin
Ah. Okay. The implementations differ but the overall effect is the same. It really comes down to when the optimizations take place.
C# optimizes in the compiler stage and the run-time layer doesn't worry about it.
Java just flags the variable in the compiler and the run-time does the optimizations.
I come from an embedded OS world. There is no run-time to handle it that way.
Admin
|What about the code in question demonstrates a lack of understanding of booleans?
if (booleanExpression == true) or if (booleanExpression == false)
instead of
if (booleanExpression) or if (!booleanExpression)
is bad. Really bad. Why not if (((booleanExpression == true) == true) == true) - just to be really shure?
Admin
TIL: 'TIL'...Seriously...I've never seen it used before now...
Admin
Yeah Calvin, welcome to my world.
"The next day, the very first thing he did was refactor the code to actually properly handle multiple threads."
IF the PTB let you refactor code and let you deploy it to prod, instead of forcing you to only put bandaids on top of other bandaids, ad-infinitum. The best thing that ever happened to my team was the lead leaving - if anything ever broke it was always the fault of any change we tried to make for the better and his immediate response was to demand a rollback of that change.
It has taken years to make fixes to small portions of the code, and major changes simply are not allowed or torpedoed by non-devs. There is not a single anti-pattern that does not exist in our code, and many of them exist almost anywhere, including using a single connection for all persistence code, including multi-threaded code.
Admin
Can we just focus on the specific WTF here? (I say this in the full recognition that I did not.)
This code is abject. The implementation details of why it is abject -- the compiler details of how that abjectness came to be exposed to the light of day -- even the brain-dead corporate policy not to test the code against the release build -- none of that matters!
Well, obviously, it all matters. Practically everybody involved should be fired, if not buried in a peat bog. But it doesn't matter all that much.
As an earlier commenter pointed out, this dreadful thing essentially re-implements a spin-lock, without the lock. Now (assuming threads), either you are going to program against an asynchronous paradigm, or you are going to program against a synchronous paradigm. You can't really have both, which is why the compiler is eliding this wretched excuse for a loop.
Admin
STOP SAYING THAT!
Admin
Yeah, well quite obviously the developer who did that had a naive grasp of multi-threading and probably wasn't too hot on scoping. It may be a coincidence that they also happened to be deploying in debug and so masked this problem, but more likely someone wrestled with it for ages, never got to where the submitter did, and found that workaround. That's quite a big break from doing things the right way, and you'd think the guy who'd been there years might have mentioned it .... hahahaha
What does surprise me is that they'd not apparently had some stability issues with this thing already, surely if the "third party web service" fell over, malfunctioned, lost connectivity we'd be left with the process unresponsive and a core out of action?
Admin
Clearly this is how you do it in Abject Oriented Programming.
Admin
Apparently the "third party web service" is SOAP.
Problem solved right there. There is no plausible way that a SOAP service request will not fall over on a regular, yet unpredictable, basis.
Admin
Releasing the Debug build is a common meme in embedded development, because it seems like only one person at a given company (the TDWTF reader of course) will have a clue about keeping the project build configs in sync. And if there is no such person, well, let's just release the debug version! If you're lucky they'll manually set a #define to disable the debug messages when building the "de-lease" version.
And don't drop the SOAP request.
Admin
I hate the MS Debug/Release "mode" selection. Apart for allowing different build config, it also does a lot of other secret things (Like padding memory allocations with lots of extra space - to hide buffer overrun. And possibly just including different libraries). Also, IIRC, Release mode still allows for symbols-debug information to be generated, and still allows step-debugging, so it's just about letting you ignore some terrible bugs in your code) Whenever in a MS C++/C# project, I forbid everyone from building/running in debug mode.
Admin
Once compiled, is there even a difference between if(var == false) and if(!var) ?
I feel a bit dirty if I write if(var == false), but what makes me occasionally do it is if I'm not 100% sure the next person reading the code will read the conditional logic properly on the first go, and there is no more verbose way of inverting the statement.
if(!var)
and
if(var)
look really similar. Some languages let you do something like if(not var), but if I have to use an exclamation mark and its really important that people understand Im checking for a negative, then I'll use var == false. (Or write a function called isNull, isFalse etc so I can if(isfalse(var)).
But I don't work with code very deeply.
Admin
In my last project, convention was to use if(FALSE == booleanVar), however this was embedded C for modems and it removes the chance of accidentally typing if(booleanVar = FALSE) and distinguishes it from a pointer check if you use if(ptr). Some people also think it's clearer. Outside of that I prefer if(bool) but have used either or depending on the readability when updating legacy code.
Admin
And if you DO expect the Spanish Inquisition, it is the Portuguese that show up at your door.
Admin
This is just CodeSOD posing as Feature article. Write some bullshit about a guy who got to a horrible environment, insert whatever CodeSOD you got, and wrap it up with "And then he fixed it". Tadaaa!
I know it's summer but...
Admin
| Once compiled, is there even a difference between if(var == false) and if(!var) ?
With a modern compiler - I doubt that there a differences. But I don't complain about micro-optimizations. It's about proper code and proper handling of boolean expressions.
if (booleanExpression == false) is redundant and does not explain anything. It's just a distraction. And why stop here, we can take it further: if ((booleanExpression == false) == true) - just to be really clear?
The ! as NOT operator is not very visible, you are right. But even a sloppy reader should not overlook it. There are many other places where tiny operators/ characters are important - eg | (or operator) or - (negation).
Admin
| In my last project, convention was to use if(FALSE == booleanVar), however this was | embedded C for modems and it removes the chance of accidentally typing | if(booleanVar = FALSE) and distinguishes it from a pointer check
With ancient C Compilers there is some merit in using
if (MAX_LIMIT == actualLimit)
But today every decent Compiler should emit a Warning message when you accidentially use just one = instead of two in an if statement. The inverse order just feels wrong (Insert here Sheldons face when Penny messes with his couch cushion). I think that you should better get rid of the warnings instead of introducing mental bumps in the source code.
Admin
Addendum 2017-08-04 05:00: Damn, forgot to use double newlines. Lack of preview and edit sucks.
Admin
So the code is redundant. Does that demonstrate a lack of understanding? I feel like you are inflating the severity of a redundant equality check, when the condition in question is precisely an equality check. It is redundant only because the result of the equality is the same type as the value being compared.
Admin
I always use "if (boolvar == true)". A boolean is nothing but another data type with a certain domain, and there's no logical reason why I should break pattern and treat "if (boolvar == true)" any differently than "if (numvar == 5)" or "if (objvar == null)".
Admin
I agree totally. In fact I was SOOO pissed off the first time I encountered Booleans (especially as return codes from routines) that I decided I would never ever ever ever use the "implicit" versions. For me, for ever, for all languages, I use the explicit "(if blnWhatever == true)" or "(if blnWhatever == false)" versions. If someone complains I tell them to piss off, the compiler code is identical. If they tell me implicit is the coding standard here, I say fine, no problem, I'll just put a "// the following statement tests for False" comment in front of all my (YOUR) implicit if's and we'll take it up with the QA team at the next release meeting, now piss off again and this time get a freaking life..
Admin
I forgot. The other option is to never use Booleans, especially as Database Columns. Use a one character string with a "Y" or an "N" in it. Wah wah, baby crying, but suppose it's blank, wah wah? Well then some idiot, including myself, didn't set it properly, fix it.). To this day NONE of my new databases/tables has a boolean/bit column.
Admin
That is a really weird hill to die on. But, you know. Have fun.
Admin
Yeah, because one thing we can be sure of is that some poor bastard is going to inherit that schema after another bunch of people have buggered up the interface and these Y/N fields are full of 0, "0", "-", null, "yes", "U", "File_Not_Found" and a bunch of bloody chinese unicode.
I'm all in on avoiding bit fields, their shitty implementation across various RDBMS makes them a very cautious option, booleans are unnecessarily greedy, but usable. That solution is about as good as the one I once had at one place, of having a "questions" field which was some form of integer and bitmasking a whole mess of booleans out of it. That one was supposedly out of necessity, but made for horrid code, I pity the poor shit who inherited it. Well, I knew him, he forgave me, or maybe he was being polite.
Admin
TEXT/*CHAR is all you need, brah! Strong types are for grown-ups anyway. :D
Admin
Ive worked places in the past where the solution to issues like that would have been to change the process to release the debug builds into production...
Admin
In C# you can use ManualResetEvents. They are designed for exactly these cases.
Admin
I love your stories. Seriously.