Re: [HACKERS] Bug in signal handler [Was: [TODO] Allow commenting

[Zdenek Kotala]

Alvaro Herrera wrote: 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 nontrivial work.) */ PG_SETMASK(&BlockSig); postmaster.c 1227ff Blocking signal is false safe. It is race condition problem. You can receive signal before you block it, but main problem is different. See example: If you have following functions and signal handler: char buffer[11]; void sig_handler(int signo) {    f1('B'); } void f1(char ch) {   int n;   for( n = 0 ; n <  10;  n++)     buffer[n] = ch; } If you call function f1('A') you expect that content of buffer will be  "AAAAAAAAAA". It will be true until you don't receive signal. Signal is asynchronous event and if you receive it during loop processing (for example when n=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 this is reason why you don't use functions like printf, because they content static internal buffer. It is reason why there are only small group of signal safe functions. 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.        Zdenek