• (nodebb)

    re: "at least until the half-created-but-never-cleaned-up sockets consume all the filehandles on the OS, but eventually."....

    Probably missing something (Still drinking second cup of morning coffee).. If you don't get back a handle...what opportunities are there for cleanup???

    Other than that, a far too common WTF implementation - especially the recursion.

  • T.T.O. (unregistered)

    What's even more interesting is that if the recursive call succeeds (against all expectations), this function will still go to the next iteration of while(true) and overwrite m_pSocket. It seems that even the programmer himself didn't expect it to possibly succeed, he was just expressing the idea "I WILL GET THIS SOCKET OR I'LL DIE TRYING".

  • Sally Flynn (unregistered)

    Some people just love to creat problems...

  • LCrawford (unregistered)

    Why is management so involved in the details of this fix? Do they get so involved in the line-by-line details of other fixes?

  • fragile (unregistered)

    What would happen if Gerry ran a shadow socket server in line with corporate policy and kept management in the dark?

  • Raj (unregistered)

    The code is not that terrible. I've done similar things when dealing with unreliable things that my piece of code had to interact with "no matter what", like a slow/flakey authentication service or a frequently overloaded 3rd party database. Sometimes you can't fix what's on the other side.

    I personally prefer using an exponential backoff approach, waiting a longer and longer time on every try, but a sleep call in there is acceptable.

    It does look like the last sleep(5) is too long but in a way it could make sense to keep it so it gives a chance to the next tentative to succeed with less contention if it needs a few seconds to initialize. It depends how often this thing fails and how many concurrent calls there are. Hardly a big WTF. I suspect I'd rather deal with the guy who wrote those sleep(5) than with the guy who whines about them.

  • Brian (unregistered)

    Correct me if I'm wrong... but isn't new guaranteed to never return null (I'm assuming C++ here, given the syntax)? I thought it threw an exception if it failed, making the subsequent check for (m_pSocket != null) pointless. Unless of course (the horror!) the Socket class overloaded new for some "clever" reason.

  • His Derpiness (unregistered)

    So TRWTF is that management that is not technically skilled makes technical decisions for the techically superior? Ok then.

  • (nodebb) in reply to Brian

    It's a bit unusual to NOT throw, but yes, there could be a Socket::operator new that returns NULL on certain kinds of failure, although it's hard to imagine why anyone would do that. I have written a class-specific operator new, but that was a debugging aid that was part of a UB-risking way to find out if one variable (whose address and size were known by the careful application of an address-of and a sizeof) was contained inside another (whose base address could be discovered and whose size could only be discovered by heap-block groping). I suppose that might qualify as "clever", of course.

    But maybe it was initially built for Visual C++ 6, whose default operator new would return NULL on an allocation failure instead of throwing std::bad_alloc.

  • dpm (unregistered)

    I would immediately leave any company in which management believes that there is a difference between removing a line of code and commenting it out. I'd probably leave anyway if they're micro-managing right down to the line; that's a level of granularity which is extremely difficult to tolerate on a repeated basis.

  • dpm (unregistered) in reply to Raj

    Hardly a big WTF.

    While you make a good point outside of the context of the story, TRWTF is that this code --- bulletproofed to an insane degree against very unusual edge cases --- is apparently used for all socket creations, communicating with all services. Everyone has to pay the price.

  • ancow (unregistered)

    Apart from the whole "re-run the socket creation loop at every recursion level when the CreatSocket() in the catch finally succeeds" thing, there's also the problem of limited stack size. At some point, recursing will cause the stack to overflow and the whole program to fault.

    I'm somewhat surprised that isn't supposed to have happened at some point.

  • (nodebb)

    I did LOL when I read about the little nap.

  • Mr Bits (unregistered)

    “It wouldn’t be there if it weren’t important. Instead of removing it, can you just comment it out?”

    Translation: "We don't have the stones to take out that line of code; we'll just let the preprocessor do it for us."

  • DQ (unregistered) in reply to Mr Bits

    Ofcourse, then you can blame the preprocessor!

  • Pjrz (unregistered) in reply to Mr Bits

    My guess is that the management have no concept of source control and are terrified that removing even a single innocuous line of code could cause a catastrophic collapse from which there would be no return because nobody would ever remember what the removed line was.

    Explaining the concept of source control may have helped (assuming the devs actually use it or are allowed to use it). But probably not.

    A phenomenon related to the management command of "We need this fixed urgently but don't change anything in case it breaks something else!"

  • I dunno LOL ¯\(°_o)/¯ (unregistered)

    I suppose TRWTF is management wanting to scrutinize every single-line patch, yet they somehow let this crap exist in the first place.

    Also, recursion is not a GOTO! If this gets multiple levels deep, on the way back, once a catch case gets a success, every return on the way back still has to get another success to exit the while! Each time that happens, the successful m_pSocket gets overwritten with a new one (or a null), so at least every recursion will result in a leaked socket handle. JR deleted one line too few, he should have deleted the recursive call too.

  • fragile (unregistered) in reply to I dunno LOL ¯\(°_o)/¯

    Not bad programmer, bad management

  • 🤷 (unregistered) 
    while(true)
{
    if(!(commentFirst==true))
    {
        sleep(5);
        deleteComment(max_comment_id);
        sleep(5);
    }
    else
    {
        m_PostComment->fristComment();
        break;
    }
}

    Eventually, this will be the frist comment under this story.

  • fragile (unregistered) in reply to 🤷

    Unless it's a gaslighting attempt by a global programming conspiracy to fool the courts!

  • (nodebb) in reply to Brian

    Brian - Your thoughts echoed mine :)

  • fragile (unregistered) in reply to TheCPUWizard

    The next time I'm in a situation like this, I'm really going to do that. Show management one thing, do another.

  • Duke of New York (unregistered) in reply to I dunno LOL ¯\(°_o)/¯

    "Also, recursion is not a GOTO!" look at this noob who never read the lambda papers

  • Carl Witthoft (google)

    As a user with limited patience^H^H^H^H^H^H^H attention span, TRWTF is failure to provide either a timeout or a nice little message that gets sent, say, every 10th try (50 or 100 seconds of sleep() ) that says "Couldn't get socket. Abort/Retry/AskClippy?"

  • TexasDex (unregistered)

    How is socket formd? How app get connected?

  • Sole Purpose of Visit (unregistered) in reply to Raj

    Believe me: with a series of feeble justifications like that, and no evident understanding of why stupidly random sleeps, recursion, and resource leakage are a problem ...

    ... Chances are, you will end up working with people like Gary, and not with the OP.

    I see this as a win-win resolution, so all is good.

  • isthisunique (unregistered)

    I've used the C socket APIs and it's notoriously difficult to get meaningful error reporting or documentation compared to other implementations. This is due to a combination of C's error reporting already being limited and things such as sockets reuse a lot of functionality from file handling (access to socket is generic, specifics for sockets often go through generic calls to the kernel like ioctl). When you have some libraries then wrapping this some things improve but other things can become even more vague. I wouldn't blame someone for not being too specific in their error handling.

    I didn't know new in C++ can return null without throwing an exception. I've seen other languages that can do that. I would also hope there's clean up in the socket's destructor.

    If you're going to sleep you should check the time elapsed. You don't want your socket to time out after 10 seconds then to sleep another 5 seconds. This is where a lot of time can be lost.

    It's possible to sleep is not only there to prevent thrashing the IO. Internally it'll bottle the CPU as well and might flood error output. The Unnecessary recursion (which will blow the stack if it's not optimised out) is a problem. Especially as it'll wastefully sleep for 5 seconds even on success. It's that which is preventing the sockets from GC.

  • Sole Purpose of Visit (unregistered) in reply to I dunno LOL ¯\(°_o)/¯

    Technically, tail recursion is indeed effectively a goto. I see what you mean, though. (And yes, C++ doesn't do tail recursion.)

  • Eric Gregory (github)

    Isn't TRWTF that this code designed to "prevent crashes" could still result in a stack overflow?

  • (nodebb)

    You don't comment out code.

    Either you skip it by preprocessor directives, or you use some settings file.

    if (!useFixNo[4185]) { if (!useFixNo[3098] { performVeryOldCode(); // see ticket #25088 } else { performNotQuiteAsOldCode(); // see ticket #88030 } } else { performNewCode(IPrayItWillWorkUntilIMOutOfTheBlameQueue); // will fail if condition #3 of ticket #25088 is met and condition #11 is not, though }

  • Duke of New York (unregistered)

    Non-recruit agreements between employers and employees are very weak in California, and tech startups like this because it makes technical labor readily available, but it isn't necessarily the case in other US states. If the employee accepts a specific payment for the agreement, or if it is part of a productive business arrangement (e.g. corporate partnership) then a court can be more inclined to enforce it.

  • Duke of New York (unregistered)

    oh dangit there I go again posting on the wrong article :(

  • guest (unregistered) in reply to dpm

    "I would immediately leave any company in which management believes that there is a difference between removing a line of code and commenting it out. "

    I'm wondering what their version control is like. And it makes me afraid.

  • (nodebb) in reply to guest

    The worst I've experienced so far is one folder for each version, named with the version number and containing a copy of every single code file of the project.

    Let's hope that was not worse than an Excel workbook with a sheet for each version containing a list of the location of the code files relevant for that version.

    It can't be so bad it couldn't be worse.

  • Dan (unregistered)

    “It wouldn’t be there if it weren’t important. Instead of removing it, can you just comment it out?”

    Reply: "Then maybe the behavior is there because it's important too?"

  • RichP (unregistered)

    And on the 1st day, God did creat a socket, and He tried to connect to it, and it failed, and God was most displeased. And on the 2nd day, God did creat a socket, and He tried to connect to it, and it failed, and God was most displeased. And on the 3rd day, God did creat a socket, and He tried to connect to it, and it failed, and God was most displeased. And on the 4th day, God did creat a socket, and He tried to connect to it, and it failed, and God was most displeased. And on the 5th day, God did creat a socket, and He tried to connect to it, and it failed, and God was most displeased. And on the 6th day, God did creat a socket, and He tried to connect to it, and it succeeded, and God was most pleased with what He had wrought.

    And on the 7th day, God slept(5)

  • RichP (unregistered)

    And on the 1st day, God did creat a socket, and He tried to connect to it, and it failed, and God was most displeased. And on the 2nd day, God did creat a socket, and He tried to connect to it, and it failed, and God was most displeased. And on the 3rd day, God did creat a socket, and He tried to connect to it, and it failed, and God was most displeased. And on the 4th day, God did creat a socket, and He tried to connect to it, and it failed, and God was most displeased. And on the 5th day, God did creat a socket, and He tried to connect to it, and it failed, and God was most displeased. And on the 6th day, God did creat a socket, and He tried to connect to it, and it succeeded, and God was most pleased with what He had wrought.

    And on the 7th day, God slept(5)

  • RichP (unregistered)

    (sorry for the double-post, damn re-captcha acting up)

  • fragile (unregistered) in reply to RichP

    No 6th Dimension Supergravity dimension exists providing the universe a self limiting demarc across all N multiverses denying the existence of a theoretically "antihuman" chain of being driving evolution into annihiliation.

  • Gerhard Visagie (google)

    Whoops. hope this isn't my code! If its not, then bad show for bringing Gerry's around the worlds name into disrepute!

  • trwtf (unregistered)

    This is gold

    Unfortunately, management balked at removing a line of code. “It wouldn’t be there if it weren’t important. Instead of removing it, can you just comment it out?”

  • (nodebb) in reply to isthisunique

    I didn't know new in C++ can return null without throwing an exception.

    In the early days of C++, that was how it worked. C++98 dictates that by default, it throws std::bad_alloc, but at least one, er, commercially important C++ compiler of that year (VC++6) will return NULL.

    And yes, you can either replace the global operator new OR write a class-hierarchy-specific one so that it will do whatever you want, including returning NULL on failure. In general, there is no real need to write your own, but there are occasions where it can help, but even then, you should write your own ONLY if it is really necessary.

  • Free Bird (unregistered) in reply to PWolff

    That isn't actually so bad, as long as you don't adopt the "commit early, commit often" philosophy or have some procedure for throwing out intermediate versions. If you use this system for releases only, it can work just fine. Of course you lose out on most of the advantages of real version control solutions, but it doesn't count as a WTF.

  • fragile (unregistered) in reply to Free Bird

    I like the international conspiracy of programmers, upheld by legal measuers, better.

    At the end of the day, the defense has to call some programmer who takes a look at the whole situation, and sides with the good guys.

  • Erik (unregistered) in reply to Raj

    The last sleep is after the recursive call suceeds. So you get the following; attempt to create socket->throw error->sleep(5) before recursive call->attempt to create socket->throw error->sleep(5) before recursive call->attempt to create socket->...->suceed->sleep(5) after recursive call->overwrite the sucessfully created socket because we don't break out of the while loop->suceed->sleep(5) after recursive call->...

  • I dunno LOL ¯\(°_o)/¯ (unregistered) in reply to RichP

    On what day did it sink into the swamp?

  • (nodebb)

    Version control is not as useful for deleted lines, although git blame --reverse helps.

  • anonymous (unregistered)

    I'm guessing they aren't using version control, hence the need to comment out lines that you don't need but aren't quite ready to get rid of forever.

  • Memez (unregistered)

    How is socket creatd? How port get bound?

  • Solomon Ucko (google)

    How would it take up a file handle if it's 

    null
    ?

Leave a comment on “How To Creat Socket?”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article