Assertion failure on reload of GUC_SUPERUSER_ONLY parms

Started by Simon Riggsover 16 years ago2 messagesbugs
Jump to latest
#1Simon Riggs
simon@2ndQuadrant.com

TRAP: BadState("!(((bool) ((CurrentUserId) != ((Oid) 0))))", File:
"miscinit.c", Line: 295)

Gabriele Bartolini originally reported this to me as a bug in Hot
Standby, though I have now been able to reproduce in CVS head. We were
just unlucky enough to hit it while doing thorough HS testing.

backtrace
#0 0x00002b7beafd2b45 in raise () from /lib64/libc.so.6
#1 0x00002b7beafd40e0 in abort () from /lib64/libc.so.6
#2 0x00000000006c016d in ExceptionalCondition (
conditionName=<value optimized out>, errorType=<value optimized
out>,
fileName=<value optimized out>, lineNumber=<value optimized out>)
at assert.c:57
#3 0x00000000006ccf4d in GetUserId () at miscinit.c:295
#4 0x00000000006db1b9 in superuser () at superuser.c:48
#5 0x00000000006d5a2b in GetConfigOption (name=0xb2b6e8 "log_filename")
at guc.c:5226
#6 0x00000000006d8a2c in ProcessConfigFile (context=PGC_SIGHUP)
at guc-file.l:319
#7 0x00000000005e49e2 in SIGHUP_handler (
postgres_signal_arg=<value optimized out>) at postmaster.c:2062
#8 <signal handler called>
#9 0x00002b7beb060bc3 in select () from /lib64/libc.so.6
#10 0x00000000005e1bf7 in ServerLoop () at postmaster.c:1360
#11 0x00000000005e2b14 in PostmasterMain (argc=3, argv=0xb246a0)
at postmaster.c:1065

It turns out that when you modify a GUC_SUPERUSER_ONLY parameter, such
as log_filename, in postgresql.conf and then reload you will get the
assertion failure.

It looks to me like the correct fix would be to use
GetUserIdAndContext() instead, though I would suggest inventing
GetUserIdIfAny() which would skip the assertion test for use in
superuser().

--
Simon Riggs www.2ndQuadrant.com

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Simon Riggs (#1)
Re: Assertion failure on reload of GUC_SUPERUSER_ONLY parms

Simon Riggs <simon@2ndQuadrant.com> writes:

It looks to me like the correct fix would be to use
GetUserIdAndContext() instead, though I would suggest inventing
GetUserIdIfAny() which would skip the assertion test for use in
superuser().

You are thinking at the wrong abstraction level. Even to ask if
the user is superuser is inappropriate in the postmaster, since
there is no user. So I think the bug is that GetConfigOption
is calling superuser() when it is now possible for it to be used
in the postmaster.

The quick & dirty fix would be to add IsUnderPostmaster to the
check in GetConfigOption. Or we could refactor things so that
the GUC_SUPERUSER_ONLY check is applied at some higher level;
but that's probably more work than it's worth.

It looks to me like the pstrdup that was added in guc-file.l
represents a permanent memory leak in the postmaster, too :-(

regards, tom lane