Обсуждение: Fix memory leak in postmasterMain

Hi

This is fairly inconsequential as memory leaks goes, but if -D is used 
when starting postgres, the memory allocated by stdrup() will never be 
freed. Found with valgrind.

Technically, this also happens with output_config_variable (-C), but since 
postmaster prints and exits with that, there isn't much point in fixing 
it.


   best regards, Henrik

Hi

On Sat, 21 Feb 2026, Henrik TJ wrote:

> This is fairly inconsequential as memory leaks goes, but if -D is used when 
> starting postgres, the memory allocated by stdrup() will never be freed. 
> Found with valgrind.

Rebased version of this patch attached.

To see valgrind catch the leak:

1. Compile with valgrind.
2. Run postgres with valgrind:
    valgrind --leak-check=full ./pgrun/bin/postgres -D pgdata/

The -D argument is required, as it is the argument from there that does 
not get freed. This should yield:

==444240== 8 bytes in 1 blocks are definitely lost in loss record 29 of 849
==444240==    at 0x4840B26: malloc (vg_replace_malloc.c:447)
==444240==    by 0x5114A2E: strdup (strdup.c:42)
==444240==    by 0x6B7CE0: PostmasterMain (../src/backend/postmaster/postmaster.c:656)
==444240==    by 0x602555: main (../src/backend/main/main.c:231)


   best regards, Henrik

> On Apr 21, 2026, at 22:56, Henrik TJ <henrik@0x48.dk> wrote:
>
> Hi
>
> On Sat, 21 Feb 2026, Henrik TJ wrote:
>
>> This is fairly inconsequential as memory leaks goes, but if -D is used when starting postgres, the memory allocated
bystdrup() will never be freed. Found with valgrind. 
>
> Rebased version of this patch attached.
>
> To see valgrind catch the leak:
>
> 1. Compile with valgrind.
> 2. Run postgres with valgrind:
>   valgrind --leak-check=full ./pgrun/bin/postgres -D pgdata/
>
> The -D argument is required, as it is the argument from there that does not get freed. This should yield:
>
> ==444240== 8 bytes in 1 blocks are definitely lost in loss record 29 of 849
> ==444240==    at 0x4840B26: malloc (vg_replace_malloc.c:447)
> ==444240==    by 0x5114A2E: strdup (strdup.c:42)
> ==444240==    by 0x6B7CE0: PostmasterMain (../src/backend/postmaster/postmaster.c:656)
> ==444240==    by 0x602555: main (../src/backend/main/main.c:231)
>
>
>  best regards, Henrik<v2-0001-Fix-userDoption-not-getting-freed-in-postmaster.patch>

From my experience, most of the time the postmaster is started with -D. On Linux and macOS, that path can be quite
long,PATH_MAX is often 4096 on many Unix-like systems, and I am not sure about Windows. So I think this leak is worth
fixing.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Wed, Apr 22, 2026 at 3:46 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> From my experience, most of the time the postmaster is started with -D. On Linux and macOS, that path can be quite
long,PATH_MAX is often 4096 on many Unix-like systems, and I am not sure about Windows. So I think this leak is worth
fixing.

We can also free()/pfree() userDoption in postgres.c and bootstrap.c?

Since userDoption typically uses only a small amount of memory, I'm not sure
this patch provides much practical benefit from a memory-leak perspective.
So I don't think it needs to be backpatched to the stable branches.

OTOH, if it helps keep Valgrind quiet for userDoption, I may be ok with
applying it to master.

Regards,

--
Fujii Masao

Hi,

On 2026-04-23 00:29:29 +0900, Fujii Masao wrote:
> On Wed, Apr 22, 2026 at 3:46 PM Chao Li <li.evan.chao@gmail.com> wrote:
> >
> > From my experience, most of the time the postmaster is started with -D. On Linux and macOS, that path can be quite
long,PATH_MAX is often 4096 on many Unix-like systems, and I am not sure about Windows. So I think this leak is worth
fixing.
> 
> We can also free()/pfree() userDoption in postgres.c and bootstrap.c?
> 
> Since userDoption typically uses only a small amount of memory, I'm not sure
> this patch provides much practical benefit from a memory-leak perspective.

I don't think this is a leak at all. We *never* can reach the end of the scope
in which userDoption is allocated. We abort() if the end of PostmasterMain()
is ever reached.  What is the leak here supposed to actually be?


> So I don't think it needs to be backpatched to the stable branches.

Definitely not.


> OTOH, if it helps keep Valgrind quiet for userDoption, I may be ok with
> applying it to master.

I'm doubtful it makes sense to fix it this way.

If we do it, we should actually be a bit more systematic and also free
output_config_variable.

ISTM those strdup()s should actually be pstrdup()s?  I suspect changing that
would also silence valgrind.

Greetings,

Andres Freund

> On Apr 23, 2026, at 01:55, Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2026-04-23 00:29:29 +0900, Fujii Masao wrote:
>> On Wed, Apr 22, 2026 at 3:46 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>>
>>> From my experience, most of the time the postmaster is started with -D. On Linux and macOS, that path can be quite
long,PATH_MAX is often 4096 on many Unix-like systems, and I am not sure about Windows. So I think this leak is worth
fixing.
>>
>> We can also free()/pfree() userDoption in postgres.c and bootstrap.c?
>>
>> Since userDoption typically uses only a small amount of memory, I'm not sure
>> this patch provides much practical benefit from a memory-leak perspective.
>
> I don't think this is a leak at all. We *never* can reach the end of the scope
> in which userDoption is allocated. We abort() if the end of PostmasterMain()
> is ever reached.  What is the leak here supposed to actually be?
>

Yes, calling it “memory leak” is too strict. Usually, memory leak implies repeatedly losing memory over time, but in
thiscase, userDoption is no longer used after feeding to SelectConfigFiles(). So calling it “unnecessary retained
memory”might be more accurate. 

In theory, it occupies at most PATH_MAX bytes, but in practice it should be much less than that. So, I agree the
benefitof fixing is trivial. 

>
> ISTM those strdup()s should actually be pstrdup()s?  I suspect changing that
> would also silence valgrind.
>

I also had the question when I read the code. But looks like startup phase all uses malloc/strdup etc. C functions, for
exampleSelectConfigFiles() in guc.c also uses malloc/free. I am not sure what is the standard. 

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

On Wed, Apr 22, 2026 at 01:55:36PM -0400, Andres Freund wrote:
> If we do it, we should actually be a bit more systematic and also free
> output_config_variable.
>
> ISTM those strdup()s should actually be pstrdup()s?  I suspect changing that
> would also silence valgrind.

I don't see immediately why it would not be OK to maintain this data
in the postmaster context.  There is no need to rush this change on
HEAD, IMO, I'd suggest to leave that as a v20 item..
--
Michael