pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Move cancel key generation to after forking the backend
5+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2024-07-29 0393f542d7 Fix double-release of spinlock
* pgsql: Move cancel key generation to after forking the backend
@ 2024-07-29 13:18 Heikki Linnakangas <heikki.linnakangas@iki.fi>
0 siblings, 1 reply; 5+ messages in thread
From: Heikki Linnakangas @ 2024-07-29 13:18 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Move cancel key generation to after forking the backend
Move responsibility of generating the cancel key to the backend
process. The cancel key is now generated after forking, and the
backend advertises it in the ProcSignal array. When a cancel request
arrives, the backend handling it scans the ProcSignal array to find
the target pid and cancel key. This is similar to how this previously
worked in the EXEC_BACKEND case with the ShmemBackendArray, just
reusing the ProcSignal array.
One notable change is that we no longer generate cancellation keys for
non-backend processes. We generated them before just to prevent a
malicious user from canceling them; the keys for non-backend processes
were never actually given to anyone. There is now an explicit flag
indicating whether a process has a valid key or not.
I wrote this originally in preparation for supporting longer cancel
keys, but it's a nice cleanup on its own.
Reviewed-by: Jelte Fennema-Nio
Discussion: https://www.postgresql.org/message-id/508d0505-8b7a-4864-a681-e7e5edfe32aa@iki.fi
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/9d9b9d46f3c509c722ebbf2a1e7dc6296a6c711d
Modified Files
--------------
src/backend/postmaster/auxprocess.c | 2 +-
src/backend/postmaster/launch_backend.c | 10 +-
src/backend/postmaster/postmaster.c | 196 +-------------------------------
src/backend/storage/ipc/ipci.c | 11 --
src/backend/storage/ipc/procsignal.c | 185 +++++++++++++++++++++++-------
src/backend/tcop/backend_startup.c | 9 +-
src/backend/tcop/postgres.c | 17 +++
src/backend/utils/init/globals.c | 3 +-
src/backend/utils/init/postinit.c | 2 +-
src/include/miscadmin.h | 1 +
src/include/postmaster/postmaster.h | 9 --
src/include/storage/procsignal.h | 10 +-
12 files changed, 193 insertions(+), 262 deletions(-)
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgsql: Move cancel key generation to after forking the backend
@ 2024-07-29 13:23 Daniel Gustafsson <daniel@yesql.se>
parent: Heikki Linnakangas <heikki.linnakangas@iki.fi>
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Gustafsson @ 2024-07-29 13:23 UTC (permalink / raw)
To: Heikki Linnakangas <heikki.linnakangas@iki.fi>; +Cc: pgsql-committers@lists.postgresql.org
> On 29 Jul 2024, at 15:18, Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote:
>
> Move cancel key generation to after forking the backend
longfin seems a tad sad about this one:
procsignal.c:87:3: error: redefinition of typedef 'ProcSignalHeader' is a C11 feature [-Werror,-Wtypedef-redefinition]
} ProcSignalHeader;
^
../../../../src/include/storage/procsignal.h:77:33: note: previous definition is here
typedef struct ProcSignalHeader ProcSignalHeader;
^
1 error generated.
make[4]: *** [procsignal.o] Error 1
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgsql: Move cancel key generation to after forking the backend
@ 2024-07-29 13:25 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 5+ messages in thread
From: Heikki Linnakangas @ 2024-07-29 13:25 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; Heikki Linnakangas <heikki.linnakangas@iki.fi>; +Cc: pgsql-committers@lists.postgresql.org
On 29/07/2024 16:23, Daniel Gustafsson wrote:
>> On 29 Jul 2024, at 15:18, Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote:
>>
>> Move cancel key generation to after forking the backend
>
> longfin seems a tad sad about this one:
>
> procsignal.c:87:3: error: redefinition of typedef 'ProcSignalHeader' is a C11 feature [-Werror,-Wtypedef-redefinition]
> } ProcSignalHeader;
> ^
> ../../../../src/include/storage/procsignal.h:77:33: note: previous definition is here
> typedef struct ProcSignalHeader ProcSignalHeader;
> ^
> 1 error generated.
> make[4]: *** [procsignal.o] Error 1
Yep, thanks, just noticed and fixed it myself. I should add that to the
list of compiler options I use...
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgsql: Move cancel key generation to after forking the backend
@ 2024-07-29 14:30 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 5+ messages in thread
From: Heikki Linnakangas @ 2024-07-29 14:30 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: pgsql-committers@lists.postgresql.org
On 29/07/2024 16:25, Heikki Linnakangas wrote:
> On 29/07/2024 16:23, Daniel Gustafsson wrote:
>>> On 29 Jul 2024, at 15:18, Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote:
>>>
>>> Move cancel key generation to after forking the backend
>>
>> longfin seems a tad sad about this one:
>>
>> procsignal.c:87:3: error: redefinition of typedef 'ProcSignalHeader' is a C11 feature [-Werror,-Wtypedef-redefinition]
>> } ProcSignalHeader;
>> ^
>> ../../../../src/include/storage/procsignal.h:77:33: note: previous definition is here
>> typedef struct ProcSignalHeader ProcSignalHeader;
>> ^
>> 1 error generated.
>> make[4]: *** [procsignal.o] Error 1
>
> Yep, thanks, just noticed and fixed it myself. I should add that to the
> list of compiler options I use...
The --disable-spinlocks animals are still failing, however. Weird. I can
reproduce it locally, and started to debug but I have no clue what's
wrong. I'll keep debugging, but I'm all ears if anyone has ideas.
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgsql: Move cancel key generation to after forking the backend
@ 2024-07-29 15:28 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 0 replies; 5+ messages in thread
From: Heikki Linnakangas @ 2024-07-29 15:28 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: pgsql-committers@lists.postgresql.org
On 29/07/2024 17:30, Heikki Linnakangas wrote:
> On 29/07/2024 16:25, Heikki Linnakangas wrote:
>> On 29/07/2024 16:23, Daniel Gustafsson wrote:
>>>> On 29 Jul 2024, at 15:18, Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote:
>>>>
>>>> Move cancel key generation to after forking the backend
>>>
>>> longfin seems a tad sad about this one:
>>>
>>> procsignal.c:87:3: error: redefinition of typedef 'ProcSignalHeader' is a C11 feature [-Werror,-Wtypedef-redefinition]
>>> } ProcSignalHeader;
>>> ^
>>> ../../../../src/include/storage/procsignal.h:77:33: note: previous definition is here
>>> typedef struct ProcSignalHeader ProcSignalHeader;
>>> ^
>>> 1 error generated.
>>> make[4]: *** [procsignal.o] Error 1
>>
>> Yep, thanks, just noticed and fixed it myself. I should add that to the
>> list of compiler options I use...
>
> The --disable-spinlocks animals are still failing, however. Weird. I can
> reproduce it locally, and started to debug but I have no clue what's
> wrong. I'll keep debugging, but I'm all ears if anyone has ideas.
Found & fixed. I released the spinlock twice in EmitProcSignalBarrier. Oops.
I'm a little surprised there isn't an assertion for that. Would be
useful. This might have gone unnoticed for a long time if not for the
--disable-spinlocks implementation, and we've been talking about
removing that.
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2024-07-29 15:28 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-07-29 13:18 pgsql: Move cancel key generation to after forking the backend Heikki Linnakangas <heikki.linnakangas@iki.fi>
2024-07-29 13:23 ` Daniel Gustafsson <daniel@yesql.se>
2024-07-29 13:25 ` Heikki Linnakangas <hlinnaka@iki.fi>
2024-07-29 14:30 ` Heikki Linnakangas <hlinnaka@iki.fi>
2024-07-29 15:28 ` Heikki Linnakangas <hlinnaka@iki.fi>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox