Re: Missing initialization steps in --check and --single modes - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: Missing initialization steps in --check and --single modes |
Date | |
Msg-id | 2144139.1734401465@sss.pgh.pa.us Whole thread Raw |
In response to | Missing initialization steps in --check and --single modes (Tom Lane <tgl@sss.pgh.pa.us>) |
List | pgsql-hackers |
I wrote: > While comparing bootstrap.c to postmaster.c, I also noticed that > bootstrap mode is failing to call set_stack_base(). That means that > our checks for stack overflow are inoperative in bootstrap mode, > which doesn't seem great. > The same omissions appear in PostgresSingleUserMain, meaning that > --single mode also operates with few FDs and no stack depth > protection. That's considerably less than great. Actually ... instead of calling set_stack_base() in more places, how about we call it in fewer? I see no reason why we can't have a single call site in the backend's main() function. This ensures across-the-board coverage without fear of future omissions, and it gives a more consistent reference point than the existing code. (That point will be a few bytes more conservative than what we are doing now, but that seems fine.) I'm very tempted to move set_stack_base() and related functions and variables out of postgres.c altogether, except I'm not sure where they should go. main.c doesn't quite feel like the right place. See attached, which doesn't address the set_max_safe_fds() issue. That has to run after CreateSharedMemoryAndSemaphores(), so there probably isn't a better answer than to call it after each such call. (I guess we could call it *in* CreateSharedMemoryAndSemaphores, but that feels outside the charter of that function.) regards, tom lane diff --git a/src/backend/main/main.c b/src/backend/main/main.c index 2d98d97e8d..864714107c 100644 --- a/src/backend/main/main.c +++ b/src/backend/main/main.c @@ -113,6 +113,12 @@ main(int argc, char *argv[]) MyProcPid = getpid(); MemoryContextInit(); + /* + * Set reference point for stack-depth checking. (There's no point in + * enabling this before error reporting works.) + */ + (void) set_stack_base(); + /* * Set up locale information */ diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 6f37822c88..6f849ffbcb 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -985,11 +985,6 @@ PostmasterMain(int argc, char *argv[]) */ set_max_safe_fds(); - /* - * Set reference point for stack-depth checking. - */ - (void) set_stack_base(); - /* * Initialize pipe (or process handle on Windows) that allows children to * wake up from sleep on postmaster death. diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index e0a603f42b..eede65b567 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -128,8 +128,7 @@ typedef struct BindParamCbData static long max_stack_depth_bytes = 100 * 1024L; /* - * Stack base pointer -- initialized by PostmasterMain and inherited by - * subprocesses (but see also InitPostmasterChild). + * Stack base pointer -- initialized by main(). */ static char *stack_base_ptr = NULL; diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index 3b7b2ebec0..d24ac133fb 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -106,13 +106,6 @@ InitPostmasterChild(void) pgwin32_signal_initialize(); #endif - /* - * Set reference point for stack-depth checking. This might seem - * redundant in !EXEC_BACKEND builds, but it's better to keep the depth - * logic the same with and without that build option. - */ - (void) set_stack_base(); - InitProcessGlobals(); /*
pgsql-hackers by date: