Representative line



  • Our spiffy new Ruby on Rails application is not the best code in the world, but it's a thousand times better than some of the custom C libraries it depends on. One favourite line: 

     malloc( 4096 ); // Can't figure out where the memory leak is, but this is a temp fix

    If I take it out, the whole application crashes on startup.



  • Pretty sure that's when you're supposed to fire things up in a debugger.



  • The original problem that malloc is fixing* is not a memory leak, but a stack overflow which is kinda the opposite of a memory leak(More data then allocated ram).

    But this code does create a new leak :-}

    *Fixing as in: Works for me now, don't touch anything.

     


  • Discourse touched me in a no-no place

    @tiller said:

    But this code does create a new leak :-}
    Only if it's executed more than once, without freeing the result from the previous invocation. What you're saying is akin to saying "declaring global variables creates memory leaks," in that memory is reserved at start up, and there's no user code to free that memory. In both cases (assuming the code is executed only once), there's a single allocation and in both cases it's freed when the program exits.[1]



    Both are bad practise, but neither necessarily create 'memory leaks' in the conventional sense of the term.



    I used to hear rumours that GameOS used to really leak memory under such circumstances - i.e. not free it up once the program had exited, but that was years ago, and I'm sure they've fixed it now. Firefox has now taken up the gauntlet...



  • @tiller said:

    The original problem that malloc is fixing* is not a memory leak, but a stack overflow which is kinda the opposite of a memory leak(More data then allocated ram).

    But this code does create a new leak :-}

    *Fixing as in: Works for me now, don't touch anything.

     

    The malloc fixes a buffer overflow by actually [i]creating[/i] a memory leak: that's what's funny/worrying about the comment.

    Immediately before this the program declares a variable-sized array:

    [code]float *some_array = malloc(sizeof(float) * (1 + magical_size_variable));[/code]

    Running [code]malloc(4096);[/code] "works" because the new memory is allocated straight after the array. A more subtle and slightly less brittle way of doing the same stupid thing is to add 4096 to the malloc where the array is allocated, and a proper solution would be to fix whatever horrendous calculation is used to come up with the magical_size_variable.

    I ended up not touching this code at all: it's executed once per page hit, so with a generous 1,000 hits per day =  leaks 4MB per day. If I started fixing everything wrong with this code, I could keep going for years, and worse, I'd get a reputation as "the guy who knows about FooProject". Everything about this code is poison by now: the source control system before we inherited it was a bunch of folders named fooproject_1, fooproject_2 etc., up to fooproject_20.

    Those numbers represent "branches", by the way, not revisions.



  • @PJH said:

    @tiller said:
    But this code does create a new leak :-}
    Only if it's executed more than once, without freeing the result from the previous invocation. What you're saying is akin to saying "declaring global variables creates memory leaks," in that memory is reserved at start up, and there's no user code to free that memory. In both cases (assuming the code is executed only once), there's a single allocation and in both cases it's freed when the program exits.[1]

    Both are bad practise, but neither necessarily create 'memory leaks' in the conventional sense of the term.

    I used to hear rumours that GameOS used to *really* leak memory under such circumstances - i.e. not free it up once the program had exited, but that was years ago, and I'm sure they've fixed it now. Firefox has now taken up the gauntlet...
     

    But as can be seen from the code, he newer store a pointer to the value allocated by malloc, so he can't free it again. 

     


  • Discourse touched me in a no-no place

    @tiller said:

    @PJH said:

    @tiller said:
    But this code does create a new leak :-}
    Only if it's executed more than once, without freeing the result from the previous invocation. What you're saying is akin to saying "declaring global variables creates memory leaks," in that memory is reserved at start up, and there's no user code to free that memory. In both cases (assuming the code is executed only once), there's a single allocation and in both cases it's freed when the program exits.[1]



    Both are bad practise, but neither necessarily create 'memory leaks' in the conventional sense of the term.



    I used to hear rumours that GameOS used to really leak memory under such circumstances - i.e. not free it up once the program had exited, but that was years ago, and I'm sure they've fixed it now. Firefox has now taken up the gauntlet...
     

    But as can be seen from the code, he newer store a pointer to the value allocated by malloc, so he can't free it again. 

     

    Indeed. So how does that negate the points I was making?



  • @PJH said:

    @tiller said:
    But this code does create a new leak :-}
    Only if it's executed more than once, without freeing the result from the previous invocation. What you're saying is akin to saying "declaring global variables creates memory leaks," in that memory is reserved at start up, and there's no user code to free that memory. In both cases (assuming the code is executed only once), there's a single allocation and in both cases it's freed when the program exits.[1]

    By a strict definition, any heap memory which is not freed at program termination has leaked. Global variables do not live in the heap, and thus are not memory leaks.

    @PJH said:
    I used to hear rumours that GameOS used to *really* leak memory under such circumstances - i.e. not free it up once the program had exited, but that was years ago, and I'm sure they've fixed it now. Firefox has now taken up the gauntlet...

    I'm not certain about memory, but that was true about certain resources such as GDI handles in the 9x series.



  • @synecdoche said:

    @tiller said:

    The original problem that malloc is fixing* is not a memory leak, but a stack overflow which is kinda the opposite of a memory leak(More data then allocated ram).

    But this code does create a new leak :-}

    *Fixing as in: Works for me now, don't touch anything.

     

    The malloc fixes a buffer overflow by actually creating a memory leak: that's what's funny/worrying about the comment.

    Immediately before this the program declares a variable-sized array:

    <font face="Lucida Console" size="2">float *some_array = malloc(sizeof(float) * (1 + magical_size_variable));</font>

    Running <font face="Lucida Console" size="2">malloc(4096);</font> "works" because the new memory is allocated straight after the array. A more subtle and slightly less brittle way of doing the same stupid thing is to add 4096 to the malloc where the array is allocated, and a proper solution would be to fix whatever horrendous calculation is used to come up with the magical_size_variable.

    I ended up not touching this code at all: it's executed once per page hit, so with a generous 1,000 hits per day =  leaks 4MB per day. If I started fixing everything wrong with this code, I could keep going for years, and worse, I'd get a reputation as "the guy who knows about FooProject". Everything about this code is poison by now: the source control system before we inherited it was a bunch of folders named fooproject_1, fooproject_2 etc., up to fooproject_20.

    Those numbers represent "branches", by the way, not revisions.

     

     Once per page hit? Are the company writing a webservice in c++. And with only 1000 expected hits per day. I think we have the true wtf right here :}

    Why would anyone write a website with very low performance requirements in c++ ????

     



  • @tiller said:

    Once per page hit? Are the company writing a webservice in c++. And with only 1000 expected hits per day. I think we have the true wtf right here :}

    It could also be a custom PHP extention (or any other language). Also, I doubt it's C++, I'd lean more towards C.



  •  @Lingerance said:

    @tiller said:
    Once per page hit? Are the company writing a webservice in c++. And with only 1000 expected hits per day. I think we have the true wtf right here :}
    It could also be a custom PHP extention (or any other language). Also, I doubt it's C++, I'd lean more towards C.

     The web service is mostly Ruby on Rails: this is an extension written in C (not C++). And yes, the true WTF is that this useless site was ever built in the first place: it's for a government agency, and building this site lets them check a box somewhere.


Log in to reply