Automatic memory deallocation



  • So, when a dev was leaving, we had a handoff meeting.
    I knew it was bad when he said during the handoff that there were cases he had been investigating where an operation would fail and the app would need
    to be killed and restarted.

    I fired up the app, ran a task, and it immediately crashed. This is the kind of thing I'm dealing with (written in C++, obfuscated)

    Calling code looks like this:

    ParamObject* param = createParam( ... parameters for creating object ... );
    class1Object->sendRequest(param, false);
    

    The param object that created is not freed anywhere from the caller.

    This is what the class to send the request looks like:

    Class1::sendRequest(ParamObject* param, bool useHttps)
    {
      m_msg = new Message();
      m_param = param;
    
      SessionID* sessionID = new SessionID();
      msg->setParams(sessionID, m_param);
    
      ... code to send message ...
    }
    
    Class1::handleResponse(ResponseObject* response)
    {
      if ( response->httpStatus == 301 )
      {
        delete m_msg;
        sendRequest(m_param, true);
      }
    
      ... other response handling code ...
    }
    

    And here is the automatic freeing of the memory.

    Message::~Message()
    {
      if ( m_param ) free(m_param);
    }
    

    So, you can see what happens when you get a redirect due to SSL being enabled.


  • Discourse touched me in a no-no place

    Times like this are when you start to think “maybe garbage collection isn't all bad after all”.



  • I don't know... those that write code like this could find ways to confuse the garbage collector into not releasing memory.



  • @JoeCool said:

    Message::~Message()
    {
    if ( m_param ) free(m_param);
    }

    So, apart from the redundant test and the use of free() instead of delete...?



  • @PJH said:

    So, apart from the redundant test and the use of free() instead of delete...?

    Yeah, apart from that, m_param gets sent off in another sendRequest() call right after the original delete.


  • Discourse touched me in a no-no place

    @PJH said:

    So, apart from the redundant test and the use of free() instead of delete...?

    It's the mysterious squelching of what the code is about to use that is the beaut in this. Quick, pull the rug out from under your own feet!

    In my experience, it's useful to use a custom memory allocator that fills every memory chunk with crap on deallocation so that it can't be used meaningfully after a delete or free(), as that stops ever so many shenanigans in their tracks.



  • @dkf said:

    In my experience, it's useful to use a custom memory allocator that fills every memory chunk with crap on deallocation so that it can't be used meaningfully after a delete or free(), as that stops ever so many shenanigans in their tracks.

    Or just use Valgrind or similar.



  • In my opinion, memory freeing should NOT be done. As contrary point of view, if you free memory, it will give wrong impression to user that program is not doing anything. So just keep using all the memory and give impression to user that this program is doing a very difficult job.


  • Winner of the 2016 Presidential Election

    @Nagesh said:

    In my opinion, memory freeing should NOT be done.

    Bridget, is that you?


    Filed under: Are you in the Swamp Shack?


  • BINNED

    @error said:

    Filed under: Are you in the Swamp Shack?

    Thanks for that, now I can't stop trying to compose a Weird Al style Love Shack parody called Swamp Shack.


    Filed under: Fucking hell why does the tag syntax survive the quoting?


  • Discourse touched me in a no-no place

    @PJH said:

    Or just use Valgrind or similar.

    That has more overhead, but it is an excellent tool. (It also catches more bugs; the overhead is very justifiable sometimes. Just not always.)



  • @dkf said:

    [Valgrind] has more overhead, but it is an excellent tool. (It also catches more bugs; the overhead is very justifiable sometimes. Just not always.)

    It's something you do when you're tracking down a suspected memory bug or periodically to check stuff.



  • free and delete are mutually incompatible. Memory allocated with new must be released with delete, memory allocated with new[] released with delete[] and memory allocated with malloc released with free, otherwise it will crash (in some cases and which cases depends on platform, phase of the moon and value of kilogramsalad field).



  • @Bulb said:

    otherwise it will crash

    You misspelled 'undefined behavior. '



  • @PJH said:

    You misspelled 'undefined behavior. '

    Sure, other effects like formatting harddrive, setting the computer on fire, eating all your cookies or making daemons fly out of your ears are also permitted. So far I have only observed crashes, though.



  • Except, of course, the times when the undefined behavior was 'not crash and carry on as if nothing wrong happened. '


  • Discourse touched me in a no-no place

    Ear dæmons are a new one on me; I thought they preferred noses…



  • @Bulb said:

    free and delete are mutually incompatible. Memory allocated with new must be released with delete, memory allocated with new[] released with delete[] and memory allocated with malloc released with free, otherwise it will crash (in some cases and which cases depends on platform, phase of the moon and value of kilogramsalad field).

    Except this has nothing to do with the snippet I provided. I did not give the implementation details of the createParam() function. Just assume it calls malloc.

    [Edit]
    In hindsight, my obfuscation created this confusion. I shouldn't have called it ParamObject



  • @JoeCool said:

    Except this has nothing to do with the snippet I provided. I did not give the implementation details of the createParam() function. Just assume it calls malloc.

    Yes, for all we know this particular case may be matched correctly. But I think it is still relevant as general point. When you have code that mixes new and malloc(), the risk that somebody somewhere will use the wrong deallocation function is considerable. So it's a serious code smell and it's a thing you have to watch for when debugging it.

    Of course using allocation and deallocation directly is a big code smell in C++ itself. Modern C++ style is to use containers, smart pointers and such and avoid direct use of new and delete outside classes dedicated to resource management.



  • Valid point.
    This whole app is a big WTF.
    Infinite hang when an outside service is not available.
    Another crash because of no error checking, and the code just continues on.



  • Oh, and I didn't mention code is not formatted well, has mixed styling of using braces or not for one-line if's, sometimes braces are at the end, sometimes on a new line. One developer did this. Not as bad as the actual coding issues themselves, but still.

    And how about returning values from a void function!
    This was done in multiple functions.... are warnings output during compile just decorations?



  • @JoeCool said:

    are warnings output during compile just decorations?

    It keeps your console warmed up.



  • @JoeCool said:

    And how about returning values from a void function!

    Ok. How did this code compile?



  • @Nagesh said:

    Ok. How did this code compile?

    With warnings saying that a value was being returned from a void function.



  • @JoeCool said:

    With warnings saying that a value was being returned from a void function.

    C++ programmer is trained to ignore warning, right from engineering courses. Once we go warning. Professor told us to go ahead and execute anyway. Then he said - "Learn to ignore warnings". Thankfully I switched to Java and later to C#. So C++ remain like distant fond memory of that pet dog who used to fetch stick, when you throw one.



  • @Nagesh said:

    C++ programmer is trained to ignore warning, right from engineering courses. Once we go warning. Professor told us to go ahead and execute anyway. Then he said - "Learn to ignore warnings". Thankfully I switched to Java and later to C#. So C++ remain like distant fond memory of that pet dog who used to fetch stick, when you throw one.

    I don't know what I expected...



  • @JoeCool said:

    I don't know what I expected...
    @Nagesh said:
    C++ programmer is trained to ignore warning, right from engineering courses. Once we go warning. Professor told us to go ahead and execute anyway. Then he said - "Learn to ignore warnings". Thankfully I switched to Java and later to C#. So C++ remain like distant fond memory of that pet dog who used to fetch stick, when you throw one.

    Expectations are the reason why women get disappointed in men and men get disappointed in life.



  • @Nagesh said:

    Expectations are the reason why women get disappointed in men and men get disappointed in life.

    I expect nonsense from Nagesh. I am not disappointed.


  • Discourse touched me in a no-no place

    @Nagesh said:

    So C++ remain like distant fond memory of that pet dog who used to fetch stick, when you throw one.

    Why are you throwing a pet dog? Aren't they a bit heavy?



  • @dkf said:

    Why are you throwing a pet dog? Aren't they a bit heavy?

    It depends on the breed. Chihuahuas and other little yip-yip dogs are small enough to throw quite easily.

    Filed under: But I prefer drop-kicking



  • @Nagesh said:

    Expectations are the reason why women get disappointed in men and men get disappointed in life.

    You know, Nagesh, I'm starting to get worried about you. Bad breakup lately?



  • @Maciejasjmj said:

    You know, Nagesh, I'm starting to get worried about you. Bad breakup lately?

    In order to breakup, one must first join with someone. here the joining itself has been series of rollbacks.


  • Winner of the 2016 Presidential Election

    @Nagesh said:

    series of rollbacks.

    You're just afraid to commit.



  • @error said:

    You're just afraid to commit.

    Commit will require series of analysis that all output are as per expectations. Then one can commit.



  • @Nagesh said:

    Commit will require series of analysis that all output are as per expectations. Then one can commit.

    Don't you know? Nowadays, you need to commit early, commit often.


    Filed under: that's why programmers suck at life



  • @Maciejasjmj said:

    Don't you know? Nowadays, you need to commit early, commit often.

    Maybe if you are on Microsoft technologies like Sql Server. If you read Tom Kyte, then early commits are thing of past with Oracle.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.