Обсуждение: Removing dead support for pre-POSIX signals
A question in another project's mailing list prompted me to investigate whether our support for non-POSIX signals is actually still of any value. I grepped through the buildfarm server's configure-stage reports, and found that: 1. No buildfarm member in the available history (going back to 2012-01-01) has ever reported not having the POSIX signal interface, nor sig_atomic_t. (We don't run the POSIX-signals check on Windows systems, though.) 2. No non-Windows member has ever reported not having sigprocmask nor sigsetjmp, save for two one-time failures that look to be due to configure failing for being out of disk space. 3. The check for sys_siglist is still of use, which is unsurprising because it is not required by POSIX. The POSIX signal APIs, including sigprocmask and sigsetjmp, are required by Single Unix Spec v2, which is what we've usually considered to be our minimum baseline for supported Unix-ish systems. I think we should rip out the configure checks for HAVE_POSIX_SIGNALS, HAVE_SIGPROCMASK, HAVE_SIGSETJMP, and probably HAVE_SIG_ATOMIC_T, as well as the C code that tries to make up for not having these things (on non-Windows systems that is). It's not being exercised and it's fair to doubt that those code paths would even work reliably anymore. For instance, it seems likely that our current latch code has never been run on a system without these APIs, and even more likely that Andres' recent fooling around with signal handling (eg commit 6753333f5) has never been checked on such a system. HAVE_SIG_ATOMIC_T is a debatable case, in that the only thing we're doing with it is c.h's /* sig_atomic_t is required by ANSI C, but may be missing on old platforms */ #ifndef HAVE_SIG_ATOMIC_T typedef int sig_atomic_t; #endif which should be safe enough (if int isn't atomically stored/fetched we already have big problems elsewhere). Still, the configure test for it appears to be a complete waste of cycles. Comments? regards, tom lane
Hi, On 2015-08-30 14:59:41 -0400, Tom Lane wrote: > 1. No buildfarm member in the available history (going back to 2012-01-01) > has ever reported not having the POSIX signal interface, nor sig_atomic_t. > (We don't run the POSIX-signals check on Windows systems, though.) We, afaik, don't use any signals on windows anyway... > I think we should rip out the configure checks for HAVE_POSIX_SIGNALS, > HAVE_SIGPROCMASK, HAVE_SIGSETJMP, and probably HAVE_SIG_ATOMIC_T, as well > as the C code that tries to make up for not having these things (on > non-Windows systems that is). It's not being exercised and it's fair to > doubt that those code paths would even work reliably anymore. For > instance, it seems likely that our current latch code has never been run > on a system without these APIs, and even more likely that Andres' recent > fooling around with signal handling (eg commit 6753333f5) has never been > checked on such a system. Sounds good to me. > HAVE_SIG_ATOMIC_T is a debatable case, in that the only thing we're > doing with it is c.h's > > /* sig_atomic_t is required by ANSI C, but may be missing on old platforms */ > #ifndef HAVE_SIG_ATOMIC_T > typedef int sig_atomic_t; > #endif > > which should be safe enough (if int isn't atomically stored/fetched we > already have big problems elsewhere). Still, the configure test for it > appears to be a complete waste of cycles. What are you proposing to do instead? Replace sig_atomic_t by int everywhere? Or unconditionally do the typedef? Because the latter won't work well if it's already typedef'ed... Andres
Andres Freund <andres@anarazel.de> writes: > On 2015-08-30 14:59:41 -0400, Tom Lane wrote: >> HAVE_SIG_ATOMIC_T is a debatable case, in that the only thing we're >> doing with it is c.h's >> >> /* sig_atomic_t is required by ANSI C, but may be missing on old platforms */ >> #ifndef HAVE_SIG_ATOMIC_T >> typedef int sig_atomic_t; >> #endif >> >> which should be safe enough (if int isn't atomically stored/fetched we >> already have big problems elsewhere). Still, the configure test for it >> appears to be a complete waste of cycles. > What are you proposing to do instead? Replace sig_atomic_t by int > everywhere? Or unconditionally do the typedef? Because the latter won't > work well if it's already typedef'ed... No no no, I'm proposing to remove the above-quoted lines and the configure test. sig_atomic_t is required by C89; there is no reason anymore to cope with it not being provided by <signal.h>. regards, tom lane
On 2015-08-30 15:28:42 -0400, Tom Lane wrote: > No no no, I'm proposing to remove the above-quoted lines and the configure > test. sig_atomic_t is required by C89; there is no reason anymore to > cope with it not being provided by <signal.h>. Ok, that works for me. You seemed to be a bit more doubtful about the sig_atomic_t support, that's why I thought you might want to do something but rip it out. Seems like a pretty low risk thing to try. Andres
Andres Freund <andres@anarazel.de> writes: > On 2015-08-30 15:28:42 -0400, Tom Lane wrote: >> No no no, I'm proposing to remove the above-quoted lines and the configure >> test. sig_atomic_t is required by C89; there is no reason anymore to >> cope with it not being provided by <signal.h>. > Ok, that works for me. You seemed to be a bit more doubtful about the > sig_atomic_t support, that's why I thought you might want to do > something but rip it out. Seems like a pretty low risk thing to try. The distinction I was making was that it's fair to suspect that the pre-POSIX-signal code is actively broken. (For instance, as far back as commit 8408f6525 we were aware that libpq's thread-safe signal logic would not work with pre-POSIX signals. How many other cases do you think have snuck in unnoticed since then?) On the other hand, there's no reason to think that the substitute sig_atomic_t typedef wouldn't work fine if it were used. The argument for ripping that out is merely that it's silly to continue expending configure cycles on something that's required by C89. regards, tom lane