Thread: [TODO] Allow commenting of variables ...
I would like to implement following item from TODO list: * Allow commenting of variables in postgresql.conf to restore them to defaults. Currently, if a variable is commented out, it keeps the previous uncommented value until a server restarted. Does anybody work on it? I performed some investigation and I found that signal handler (SIGHUP_handler) contents a big code and contents signal nonsafe functions. It should generate deadlock or damage some internal data structure in the standard c library. See http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html for detail. By my opinion is necessary to rewritesignal handling in postmaster to avoid postgres malfunction. Zdenek
Zdenek Kotala wrote: > I performed some investigation and I found that signal handler > (SIGHUP_handler) contents a big code and contents signal nonsafe > functions. It should generate deadlock or damage some internal data > structure in the standard c library. See > http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html > for detail. By my opinion is necessary to rewrite signal handling in > postmaster to avoid postgres malfunction. Perhaps you missed these lines: /* * Block all signals until we wait again. (This makes it safe for our * signal handlers to do nontrivialwork.) */ PG_SETMASK(&BlockSig); postmaster.c 1227ff -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Here is some work someone has done: http://archives.postgresql.org/pgsql-patches/2006-03/msg00258.php See the followup for feedback. Seems it could be cleaned up. --------------------------------------------------------------------------- Zdenek Kotala wrote: > > I would like to implement following item from TODO list: > > * Allow commenting of variables in postgresql.conf to restore them to > defaults. Currently, if a variable is commented out, it keeps the > previous uncommented value until a server restarted. > > Does anybody work on it? > > I performed some investigation and I found that signal handler > (SIGHUP_handler) contents a big code and contents signal nonsafe > functions. It should generate deadlock or damage some internal data > structure in the standard c library. See > http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html for detail. By my opinion is necessary to rewritesignal handling in postmaster to avoid postgres malfunction. > > > Zdenek > > > ---------------------------(end of broadcast)--------------------------- > TIP 9: In versions below 8.0, the planner will ignore your desire to > choose an index scan if your joining column's datatypes do not > match > -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Alvaro Herrera wrote: <blockquote cite="mid20060510201744.GA26403@surnet.cl" type="cite"><pre wrap="">Zdenek Kotala wrote: </pre><blockquote type="cite"><pre wrap="">I performed some investigation and I found that signal handler (SIGHUP_handler) contents a big code and contents signal nonsafe functions. It should generate deadlock or damage some internal data structure in the standard c library. See <a class="moz-txt-link-freetext" href="http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html">http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_04.html</a> for detail. By my opinion is necessary to rewrite signal handling in postmaster to avoid postgres malfunction. </pre></blockquote><pre wrap=""> Perhaps you missed these lines: /* * Block all signals until we wait again. (This makes it safe for our * signal handlers to do nontrivialwork.) */ PG_SETMASK(&BlockSig); postmaster.c 1227ff </pre></blockquote> Blocking signal is false safe. It is race condition problem. You can receive signal before you blockit, but main problem is different. See example:<br /><br /> If you have following functions and signal handler:<br /><br/><code>char buffer[11];<br /><br /> void sig_handler(int signo)<br /> {<br /> f1('B');<br /> }<br /><br /> voidf1(char ch)<br /> {<br /> int n;<br /> for( n = 0 ; n < 10; n++)<br /> buffer[n] = ch; <br /> }<br /><br/></code>If you call function f1('A') you expect that content of buffer will be "AAAAAAAAAA". It will be true untilyou don't receive signal. Signal is asynchronous event and if you receive it during loop processing (for example whenn=3) then signal handler call f1 too, but with parametr 'B'. The result in buffer will be "BBBBBBBBBB" after signal processing.And now f1 continue with n=3, 4, 5 ... And your expected result is "BBBAAAAAAA". That is all. For example thisis reason why you don't use functions like printf, because they content static internal buffer. It is reason why thereare only small group of signal safe functions. <br /><br /> Decision is that Postgres uses signal dangerous functions(fopen, ...) and its signal handler is not save and should generate unpredictable behavior after signal processing. I would like to fix it, but there is some waiting patches for this source and I don't know how to correctly (with minimal merge complication) process.<br /><br /> Zdenek<br /><code><br /><br /></code><br /><br /><br/><br /><br /><br /><br /><br />
On Thu, May 11, 2006 at 01:59:46PM +0200, Zdenek Kotala wrote: > Decision is that Postgres uses signal dangerous functions (fopen, ...) > and its signal handler is not save and should generate unpredictable > behavior after signal processing. I would like to fix it, but there is > some waiting patches for this source and I don't know how to correctly > (with minimal merge complication) process. Look at the code more carefully. The restriction is that it is unsafe to call non-reentrant functions from within a signal handler while there may be a non-reentrant function run by the main program. If you look at the code in postmaster.c, the only place the signal handler can run is between the block (line 1223) and unblock (line 1231), the only function there is select() which is specifically listed as being safe. Running unsafe functions within a signal handler is not unsafe per-se. It's only unsafe if the main program could also be running unsafe functions. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Martijn van Oosterhout <kleptog@svana.org> writes: > Running unsafe functions within a signal handler is not unsafe per-se. > It's only unsafe if the main program could also be running unsafe > functions. I don't disagree with your reasoning, but does POSIX actually say this? -Doug
On Thu, May 11, 2006 at 08:24:02AM -0400, Douglas McNaught wrote:
> Martijn van Oosterhout <kleptog@svana.org> writes:
>
> > Running unsafe functions within a signal handler is not unsafe per-se.
> > It's only unsafe if the main program could also be running unsafe
> > functions.
>
> I don't disagree with your reasoning, but does POSIX actually say
> this?
On my machine, signal(2) has the following:
The routine handler must be very careful, since processing elsewhere was interrupted at some arbitrary
point.POSIX has the concept of "safe function". If a signal interrupts an unsafe function, and handler calls
anunsafe function, then the behavior is undefined. Safe functions are listed explicitly in the various
standards. The POSIX 1003.1-2003 list is
<long list including select(), signal() and sigaction()>
I havn't read POSIX myself though...
Have a nice day,
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.
Douglas McNaught <doug@mcnaught.org> writes:
> Martijn van Oosterhout <kleptog@svana.org> writes:
>> Running unsafe functions within a signal handler is not unsafe per-se.
>> It's only unsafe if the main program could also be running unsafe
>> functions.
> I don't disagree with your reasoning, but does POSIX actually say
> this?
The fact remains that the postmaster has *always* been coded like that,
and we have *never* seen any problems. Barring proof that there is a
problem, I'm uninterested in rewriting it just because someone doesn't
like it.
regards, tom lane
On Thu, May 11, 2006 at 10:11:00AM -0400, Tom Lane wrote: > Douglas McNaught <doug@mcnaught.org> writes: > > I don't disagree with your reasoning, but does POSIX actually say > > this? > > The fact remains that the postmaster has *always* been coded like that, > and we have *never* seen any problems. Barring proof that there is a > problem, I'm uninterested in rewriting it just because someone doesn't > like it. It should probably also be remembered that the "fix" would involve either polling the status by having select() return more often, or using sigsetjmp/siglongjmp. The cure is definitly worse than the disease. In a sense the test for errno == EINTR there is redundant since the backend has arranged that EINTR can never be returned (signals don't interrupt system calls) and there's a fair bit of code that relies on that... Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
* Martijn van Oosterhout: >> The fact remains that the postmaster has *always* been coded like that, >> and we have *never* seen any problems. Barring proof that there is a >> problem, I'm uninterested in rewriting it just because someone doesn't >> like it. > > It should probably also be remembered that the "fix" would involve either > polling the status by having select() return more often, or using > sigsetjmp/siglongjmp. The cure is definitly worse than the disease. The standard trick is to add a pipe to the select and write to that from the signal handler. I'm not sure if it's worth the effort, though.
On Wed, May 10, 2006 at 08:46:41PM +0200, Zdenek Kotala wrote: > I would like to implement following item from TODO list: > * Allow commenting of variables in postgresql.conf to restore them to > defaults. Currently, if a variable is commented out, it keeps the > previous uncommented value until a server restarted. > Does anybody work on it? I have a patch that seems to work, it could need some more testing however. I'll send it to you per PM. Joachim -- Joachim Wieland joe@mcknight.de C/ Usandizaga 12 1°B ICQ: 37225940 20002 Donostia / San Sebastian (Spain) GPG key available