public inbox for [email protected]help / color / mirror / Atom feed
Fix memory leak in postmasterMain 7+ messages / 5 participants [nested] [flat]
* Fix memory leak in postmasterMain @ 2026-02-21 14:27 Henrik TJ <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Henrik TJ @ 2026-02-21 14:27 UTC (permalink / raw) To: pgsql-hackers 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 From 78a88ba10f8f6aea2b4ccf831ab6775bae68603c Mon Sep 17 00:00:00 2001 From: Henrik TJ <[email protected]> Date: Wed, 18 Feb 2026 15:28:52 +0100 Subject: [PATCH] Fix userDoption not getting freed in postmaster --- src/backend/postmaster/postmaster.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 3fac46c402b..7480c28f08e 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -784,6 +784,8 @@ PostmasterMain(int argc, char *argv[]) */ if (!SelectConfigFiles(userDoption, progname)) ExitPostmaster(2); + if (userDoption != NULL) + free(userDoption); if (output_config_variable != NULL) { -- 2.53.0 Attachments: [text/plain] v1-0001-Fix-userDoption-not-getting-freed-in-postmaster.patch (765B, 2-v1-0001-Fix-userDoption-not-getting-freed-in-postmaster.patch) download | inline diff: From 78a88ba10f8f6aea2b4ccf831ab6775bae68603c Mon Sep 17 00:00:00 2001 From: Henrik TJ <[email protected]> Date: Wed, 18 Feb 2026 15:28:52 +0100 Subject: [PATCH] Fix userDoption not getting freed in postmaster --- src/backend/postmaster/postmaster.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 3fac46c402b..7480c28f08e 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -784,6 +784,8 @@ PostmasterMain(int argc, char *argv[]) */ if (!SelectConfigFiles(userDoption, progname)) ExitPostmaster(2); + if (userDoption != NULL) + free(userDoption); if (output_config_variable != NULL) { -- 2.53.0 ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix memory leak in postmasterMain @ 2026-04-21 14:56 Henrik TJ <[email protected]> parent: Henrik TJ <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Henrik TJ @ 2026-04-21 14:56 UTC (permalink / raw) To: pgsql-hackers 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 From 493a0756082acac48cb02ea4fbbe4c8f4c34b0b7 Mon Sep 17 00:00:00 2001 From: Henrik TJ <[email protected]> Date: Sat, 18 Apr 2026 16:20:31 +0200 Subject: [PATCH] Fix userDoption not getting freed in postmaster --- src/backend/postmaster/postmaster.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 90c7c4528e8..476e87001f1 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -789,6 +789,8 @@ PostmasterMain(int argc, char *argv[]) */ if (!SelectConfigFiles(userDoption, progname)) ExitPostmaster(2); + if (userDoption != NULL) + free(userDoption); if (output_config_variable != NULL) { -- 2.53.0 Attachments: [text/plain] v2-0001-Fix-userDoption-not-getting-freed-in-postmaster.patch (765B, 2-v2-0001-Fix-userDoption-not-getting-freed-in-postmaster.patch) download | inline diff: From 493a0756082acac48cb02ea4fbbe4c8f4c34b0b7 Mon Sep 17 00:00:00 2001 From: Henrik TJ <[email protected]> Date: Sat, 18 Apr 2026 16:20:31 +0200 Subject: [PATCH] Fix userDoption not getting freed in postmaster --- src/backend/postmaster/postmaster.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 90c7c4528e8..476e87001f1 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -789,6 +789,8 @@ PostmasterMain(int argc, char *argv[]) */ if (!SelectConfigFiles(userDoption, progname)) ExitPostmaster(2); + if (userDoption != NULL) + free(userDoption); if (output_config_variable != NULL) { -- 2.53.0 ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix memory leak in postmasterMain @ 2026-04-22 06:45 Chao Li <[email protected]> parent: Henrik TJ <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Chao Li @ 2026-04-22 06:45 UTC (permalink / raw) To: Henrik TJ <[email protected]>; +Cc: pgsql-hackers > On Apr 21, 2026, at 22:56, Henrik TJ <[email protected]> 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 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<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/ ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix memory leak in postmasterMain @ 2026-04-22 15:29 Fujii Masao <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Fujii Masao @ 2026-04-22 15:29 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: Henrik TJ <[email protected]>; pgsql-hackers On Wed, Apr 22, 2026 at 3:46 PM Chao Li <[email protected]> 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 ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix memory leak in postmasterMain @ 2026-04-22 17:55 Andres Freund <[email protected]> parent: Fujii Masao <[email protected]> 0 siblings, 2 replies; 7+ messages in thread From: Andres Freund @ 2026-04-22 17:55 UTC (permalink / raw) To: Fujii Masao <[email protected]>; +Cc: Chao Li <[email protected]>; Henrik TJ <[email protected]>; pgsql-hackers Hi, On 2026-04-23 00:29:29 +0900, Fujii Masao wrote: > On Wed, Apr 22, 2026 at 3:46 PM Chao Li <[email protected]> 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 ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix memory leak in postmasterMain @ 2026-04-23 05:22 Chao Li <[email protected]> parent: Andres Freund <[email protected]> 1 sibling, 0 replies; 7+ messages in thread From: Chao Li @ 2026-04-23 05:22 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Fujii Masao <[email protected]>; Henrik TJ <[email protected]>; pgsql-hackers > On Apr 23, 2026, at 01:55, Andres Freund <[email protected]> wrote: > > Hi, > > On 2026-04-23 00:29:29 +0900, Fujii Masao wrote: >> On Wed, Apr 22, 2026 at 3:46 PM Chao Li <[email protected]> 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 this case, 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 benefit of 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 example SelectConfigFiles() 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/ ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix memory leak in postmasterMain @ 2026-04-23 07:44 Michael Paquier <[email protected]> parent: Andres Freund <[email protected]> 1 sibling, 0 replies; 7+ messages in thread From: Michael Paquier @ 2026-04-23 07:44 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Fujii Masao <[email protected]>; Chao Li <[email protected]>; Henrik TJ <[email protected]>; pgsql-hackers 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 Attachments: [application/pgp-signature] signature.asc (833B, 2-signature.asc) download ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-04-23 07:44 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-02-21 14:27 Fix memory leak in postmasterMain Henrik TJ <[email protected]> 2026-04-21 14:56 ` Henrik TJ <[email protected]> 2026-04-22 06:45 ` Chao Li <[email protected]> 2026-04-22 15:29 ` Fujii Masao <[email protected]> 2026-04-22 17:55 ` Andres Freund <[email protected]> 2026-04-23 05:22 ` Chao Li <[email protected]> 2026-04-23 07:44 ` Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox