agora inbox for pgsql-committers@postgresql.org  
help / color / mirror / Atom feed
pgsql: Introduce a new mechanism for registering shared memory areas
3+ messages / 3 participants
[nested] [flat]

* pgsql: Introduce a new mechanism for registering shared memory areas
@ 2026-04-05 23:27 Heikki Linnakangas <heikki.linnakangas@iki.fi>
  2026-04-06 11:55 ` Re: pgsql: Introduce a new mechanism for registering shared memory areas Aleksander Alekseev <aleksander@tigerdata.com>
  0 siblings, 1 reply; 3+ messages in thread

From: Heikki Linnakangas @ 2026-04-05 23:27 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org

Introduce a new mechanism for registering shared memory areas

This replaces the [Subsystem]ShmemSize() and [Subsystem]ShmemInit()
functions called at postmaster startup with a new set of callbacks.
The new mechanism is designed to be more ergonomic. Notably, the size
of each shmem area is specified in the same ShmemRequestStruct() call,
together with its name. The same mechanism is used in extensions,
replacing the shmem_{request/startup}_hooks.

ShmemInitStruct() and ShmemInitHash() become backwards-compatibility
wrappers around the new functions. In future commits, I will replace
all ShmemInitStruct() and ShmemInitHash() calls with the new
functions, although we'll still need to keep them around for
extensions.

Co-authored-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/CAExHW5vM1bneLYfg0wGeAa=52UiJ3z4vKd3AJ72X8Fw6k3KKrg@mail.gmail...

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/283e823f9dcb03d0be720928b261628af06d3fd4

Modified Files
--------------
doc/src/sgml/system-views.sgml          |   4 +-
doc/src/sgml/xfunc.sgml                 | 158 ++++--
src/backend/bootstrap/bootstrap.c       |   2 +
src/backend/postmaster/launch_backend.c |   4 +
src/backend/postmaster/postmaster.c     |  19 +-
src/backend/storage/ipc/ipci.c          |  29 +-
src/backend/storage/ipc/shmem.c         | 824 ++++++++++++++++++++++++++++----
src/backend/storage/ipc/shmem_hash.c    |  79 ++-
src/backend/storage/lmgr/proc.c         |   3 +
src/backend/tcop/postgres.c             |  10 +-
src/backend/utils/hash/dynahash.c       |   4 +-
src/include/storage/shmem.h             | 159 +++++-
src/include/storage/shmem_internal.h    |  41 +-
src/tools/pgindent/typedefs.list        |   7 +-
14 files changed, 1144 insertions(+), 199 deletions(-)



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: pgsql: Introduce a new mechanism for registering shared memory areas
  2026-04-05 23:27 pgsql: Introduce a new mechanism for registering shared memory areas Heikki Linnakangas <heikki.linnakangas@iki.fi>
@ 2026-04-06 11:55 ` Aleksander Alekseev <aleksander@tigerdata.com>
  2026-04-06 12:38   ` Re: pgsql: Introduce a new mechanism for registering shared memory areas Heikki Linnakangas <hlinnaka@iki.fi>
  0 siblings, 1 reply; 3+ messages in thread

From: Aleksander Alekseev @ 2026-04-06 11:55 UTC (permalink / raw)
  To: pgsql-committers@lists.postgresql.org; +Cc: Heikki Linnakangas <heikki.linnakangas@iki.fi>

Hi Heikki,

> Introduce a new mechanism for registering shared memory areas
>
> [...]

This commit introduced a memory leak which Valgrind is very much upset about.

ShmemRequestStructWithOpts() allocates a copy of `options` in
TopMemoryContext and passes it to ShmemRequestInternal(). It appends
it to pending_shmem_requests as request->options. Later in
ShmemInitRequested() when the list is freed `->options` leak. There
are similar issues in ShmemAttachRequested() and
CallShmemCallbacksAfterStartup() which free pending_shmem_requests
without freeing `->options`.

I propose to fix it as attached.


--
Best regards,
Aleksander Alekseev

Attachments:

  [text/x-patch] v1-0001-Fix-memory-leaks-introduced-by-commit-283e823f9dc.patch (1.5K, ../../CAJ7c6TN9tp8MTc0WXM0zfSWqjfBqU8gpe+o5KqHB1-cQ7409Kw@mail.gmail.com/2-v1-0001-Fix-memory-leaks-introduced-by-commit-283e823f9dc.patch)
  download | inline diff:
From 5edf1d194c545e7eb59364c50d26f0c087fef3a2 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@tigerdata.com>
Date: Mon, 6 Apr 2026 14:27:09 +0300
Subject: [PATCH v1] Fix memory leaks introduced by commit 283e823f9dcb

When freeing pending_shmem_requests we should also free
->options pointers.

Author: Aleksander Alekseev <aleksander@tigerdata.com>
Discussion: https://postgr.es/m/E1w9WsZ-00399s-08%40gemulon.postgresql.org
---
 src/backend/storage/ipc/shmem.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index 92c96257588..79adb4f6c61 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -435,6 +435,7 @@ ShmemInitRequested(void)
 	foreach_ptr(ShmemRequest, request, pending_shmem_requests)
 	{
 		InitShmemIndexEntry(request);
+		pfree(request->options);
 	}
 	list_free_deep(pending_shmem_requests);
 	pending_shmem_requests = NIL;
@@ -477,6 +478,7 @@ ShmemAttachRequested(void)
 	foreach_ptr(ShmemRequest, request, pending_shmem_requests)
 	{
 		AttachShmemIndexEntry(request, false);
+		pfree(request->options);
 	}
 	list_free_deep(pending_shmem_requests);
 	pending_shmem_requests = NIL;
@@ -947,6 +949,8 @@ CallShmemCallbacksAfterStartup(const ShmemCallbacks *callbacks)
 			AttachShmemIndexEntry(request, false);
 		else
 			InitShmemIndexEntry(request);
+
+		pfree(request->options);
 	}
 	list_free_deep(pending_shmem_requests);
 	pending_shmem_requests = NIL;
-- 
2.43.0



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: pgsql: Introduce a new mechanism for registering shared memory areas
  2026-04-05 23:27 pgsql: Introduce a new mechanism for registering shared memory areas Heikki Linnakangas <heikki.linnakangas@iki.fi>
  2026-04-06 11:55 ` Re: pgsql: Introduce a new mechanism for registering shared memory areas Aleksander Alekseev <aleksander@tigerdata.com>
@ 2026-04-06 12:38   ` Heikki Linnakangas <hlinnaka@iki.fi>
  0 siblings, 0 replies; 3+ messages in thread

From: Heikki Linnakangas @ 2026-04-06 12:38 UTC (permalink / raw)
  To: Aleksander Alekseev <aleksander@tigerdata.com>; pgsql-committers@lists.postgresql.org

On 06/04/2026 14:55, Aleksander Alekseev wrote:
>> Introduce a new mechanism for registering shared memory areas
>>
>> [...]
> 
> This commit introduced a memory leak which Valgrind is very much upset about.
> 
> ShmemRequestStructWithOpts() allocates a copy of `options` in
> TopMemoryContext and passes it to ShmemRequestInternal(). It appends
> it to pending_shmem_requests as request->options. Later in
> ShmemInitRequested() when the list is freed `->options` leak. There
> are similar issues in ShmemAttachRequested() and
> CallShmemCallbacksAfterStartup() which free pending_shmem_requests
> without freeing `->options`.
> 
> I propose to fix it as attached.

LGTM, I will push this shortly. Thanks!

- Heikki






^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2026-04-06 12:38 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-05 23:27 pgsql: Introduce a new mechanism for registering shared memory areas Heikki Linnakangas <heikki.linnakangas@iki.fi>
2026-04-06 11:55 ` Aleksander Alekseev <aleksander@tigerdata.com>
2026-04-06 12:38   ` Heikki Linnakangas <hlinnaka@iki.fi>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox