Thread: BUG #16321: Memory leaks in PostmasterMain
The following bug has been logged on the website: Bug reference: 16321 Logged by: Hugh Wang Email address: hghwng@gmail.com PostgreSQL version: 12.2 Operating system: Linux Description: The argument parsing duplicates strings, but never frees them. For example, when you pass "-D $DATA_DIR" to postmaster, postmaster duplicates the string here: https://github.com/postgres/postgres/blob/master/src/backend/postmaster/postmaster.c#L698 The duplicated string is passed to `SelectConfigFiles`, which does everything except freeing the string.
PG Bug reporting form <noreply@postgresql.org> writes: > The argument parsing duplicates strings, but never frees them. This hardly amounts to enough of a problem to worry about. The string might be leaked, or it might not, but tracking whether it is is more trouble than it's worth. Generally we only worry about memory leaks if they (a) can waste a lot of memory or (b) can repeat, and thereby accumulate to waste a lot of memory. Surely neither one applies to postmaster argument parsing. > For example, when you pass "-D $DATA_DIR" to postmaster, postmaster > duplicates the string here: > https://github.com/postgres/postgres/blob/master/src/backend/postmaster/postmaster.c#L698 > The duplicated string is passed to `SelectConfigFiles`, which does > everything except freeing the string. This is a great example of a case where the cure is likely to be worse than the disease. SelectConfigFiles surely has little business freeing its input string (indeed, it couldn't do so without casting away the "const"). On the other hand, the caller doesn't really know whether SelectConfigFiles is going to stash away a copy of the pointer; it wouldn't be unreasonable for it to do so. So in order to not perhaps-leak a few dozen bytes, we'd have to make that API more complicated and more fragile. It's not a win. As for why we strdup the argument in the first place, see here: https://www.postgresql.org/message-id/flat/20121008184026.GA28752%40momjian.us regards, tom lane
Hi Tom,
On Fri, Mar 27, 2020 at 2:52 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
PG Bug reporting form <noreply@postgresql.org> writes:
> The argument parsing duplicates strings, but never frees them.
This hardly amounts to enough of a problem to worry about. The
string might be leaked, or it might not, but tracking whether it
is is more trouble than it's worth. Generally we only worry about
memory leaks if they (a) can waste a lot of memory or (b) can
repeat, and thereby accumulate to waste a lot of memory. Surely
neither one applies to postmaster argument parsing.
Your analysis is pretty educational! If the leak is small and has low impact, then the leak itself is not important; yet fixing the bug brings more complexity.
However, from the perspective of automated bug finding, I think removing the bug is beneficial. I'm trying to find bugs in PostgreSQL with sanitizers (the leak is reported by LeakSanitizer). If the bug cannot be fixed, LeakSanitizer stops at this shallow point, which prevents detecting more bugs in deep logic.
> For example, when you pass "-D $DATA_DIR" to postmaster, postmaster
> duplicates the string here:
> https://github.com/postgres/postgres/blob/master/src/backend/postmaster/postmaster.c#L698
> The duplicated string is passed to `SelectConfigFiles`, which does
> everything except freeing the string.
This is a great example of a case where the cure is likely to be
worse than the disease. SelectConfigFiles surely has little business
freeing its input string (indeed, it couldn't do so without casting
away the "const"). On the other hand, the caller doesn't really
know whether SelectConfigFiles is going to stash away a copy of the
pointer; it wouldn't be unreasonable for it to do so. So in order
to not perhaps-leak a few dozen bytes, we'd have to make that API
more complicated and more fragile. It's not a win.
As for why we strdup the argument in the first place, see here:
https://www.postgresql.org/message-id/flat/20121008184026.GA28752%40momjian.us
regards, tom lane
Thanks,
Hugh
On Mon, Mar 30, 2020 at 4:11 AM Hugh Wang <hghwng@gmail.com> wrote: > On Fri, Mar 27, 2020 at 2:52 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> PG Bug reporting form <noreply@postgresql.org> writes: >> > The argument parsing duplicates strings, but never frees them. ... > Your analysis is pretty educational! If the leak is small and has low impact, then the leak itself is not important; yetfixing the bug brings more complexity. Bear in mind some of this cases are not leaks. You may need the string live for the whole program lifetime and elect to free them by exiting. I've had similar cases with a category of my programs which work by reading many input files and building structures in memory, which I then use to print some reports. After that I free them by exiting the program, which is much faster, no leak there. There are a whole lot of programs which do this, they memory use raises and raises while they work, and they need all of them, and they free all at once by exiting, its a valid way to free resources when the only resource is used memory, the OS frees it on exit. > However, from the perspective of automated bug finding, I think removing the bug is beneficial. I'm trying to find bugsin PostgreSQL with sanitizers (the leak is reported by LeakSanitizer). If the bug cannot be fixed, LeakSanitizer stopsat this shallow point, which prevents detecting more bugs in deep logic. This may not be a bug. Doesn't leak sanitizer have a way to mark some data as never freed? A leak is something which you should track and are not, which may eventually lead to your memory use raising. This doesn't seem to be the case. Francisco Olarte.