Re: BUG #4496: Memory leak in pg_dump.c?

Tomáš Szépe wrote:
>> A pg_dump run is comparatively short-lived, so if Zdenek is right then
>> there's no important leak here -- we're counting on program exit to
>> release the memory. There's probably little point in releasing things
>> earlier than that.
>
> Well, I'd tend to consider any logical part of a program that fails to
> release the memory it uses to be bad coding practice. You never know
> when you're going to need to shuffle things around, change the context
> of the code in a way that makes it long-lived, in turn causing the leak
> to become a real problem.

I find that documenting where alloations are not correspondingly
free()'d is usually sufficient for this.

Personally I usually don't bother freeing some allocations in
short-lived programs, but I make sure I know what they are and I'll
usually have code in there to free them if the binary is built for
debugging mostly to stop people reporting bogus memory leaks.

There are good reasons not to free memory if a program will shortly be
terminating and thus all its resources will be freed by the operating
system. Sometimes explicitly freeing large piles of memory, especially
if there's any sort of automatic cleanup associated, can take real time
that you don't really want to waste when you're about to terminate
anyway. Additionally, sometimes it's hard to know when it's safe to free
a structure, but you always know it's safe for the OS to release the
program's memory mappings after it terminates.

This is a particular issue in libraries that use various caching and
reuse mechanisms, have long-lived service providers, or do other things
that may not be visible to the program using the library. It's a good
idea to provide a libraryname_cleanup() call or similar, but it's
probably only going to be useful if the app is being run under a memory
leak checker or the like, and even then it's only useful to reduce noise.

Memory leaks that actually matter are where memory is allocated and not
freed as part of a process that is repeated many times in a program, or
that consumes a huge amount of memory.

> Also, don't you like seeing the free()s paired
> to their mallocs()s in a way that makes the allocations intuitively
> correct? :)

No - I like to use lexically scoped instances of useful classes to hide
the minutae of memory management behind a nice RAII interface. Think
std::vector<T>. When forced, I like to use std::auto_ptr<> or
std::tr1::shared_ptr<> (or boost::shared_ptr<> if TR1 is not available)
as appropriate to manage the block. I in fact loathe seeing malloc() and
free() [ or operator new() and operator delete() ] pairs, because every
one is a potential leak or double free bug.

Then again, I use C++ by preference, where you have those more
sophisticated resource management options available to you.

In the case of PostgreSQL code, of course, you just use palloc() and let
it's management of memory contexts take care of everything for you. That
sort of domain-specific smart memory management seems to be the best way
to go whenever you can use it - see also, for example, Samba's talloc()
and GhostScript's reference counting allocator.

So ... in short, I see malloc() and free() as rather low level, and not
something I want to see at all in the significant parts of any
nontrivial program.

--
Craig Ringer