Re: BUG #16321: Memory leaks in PostmasterMain

From: Hugh Wang <hghwng(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #16321: Memory leaks in PostmasterMain
Date: 2020-03-30 02:10:10
Message-ID: CACGj_g8Wk=HRstDULifRPMKmQcKhGBP3bH9d3Bp6tPv4R0_LkQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Hi Tom,

On Fri, Mar 27, 2020 at 2:52 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> PG Bug reporting form <noreply(at)postgresql(dot)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

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2020-03-30 02:20:48 BUG #16330: psql accesses null pointer in connect.c:do_connect
Previous Message PG Bug reporting form 2020-03-29 21:00:01 BUG #16329: Valgrind detects an invalid read when building a gist index with buffering