Обсуждение: pgsql: Add PSQL_WATCH_PAGER for psql's \watch command.
Add PSQL_WATCH_PAGER for psql's \watch command. Allow a pager to be used by the \watch command. This works but isn't very useful with traditional pagers like "less", so use a different environment variable. The popular open source tool "pspg" (also by Pavel) knows how to display the output if you set PSQL_WATCH_PAGER="pspg --stream". To make \watch react quickly when the user quits the pager or presses ^C, and also to increase the accuracy of its timing and decrease the rate of useless context switches, change the main loop of the \watch command to use sigwait() rather than a sleeping/polling loop, on Unix. Supported on Unix only for now (like pspg). Author: Pavel Stehule <pavel.stehule@gmail.com> Author: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/CAFj8pRBfzUUPz-3gN5oAzto9SDuRSq-TQPfXU_P6h0L7hO%2BEhg%40mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/7c09d2797ecdf779e5dc3289497be85675f3d134 Modified Files -------------- doc/src/sgml/ref/psql-ref.sgml | 28 +++++++++ src/bin/psql/command.c | 133 ++++++++++++++++++++++++++++++++++++++--- src/bin/psql/common.c | 11 ++-- src/bin/psql/common.h | 2 +- src/bin/psql/help.c | 6 +- src/bin/psql/startup.c | 19 ++++++ 6 files changed, 184 insertions(+), 15 deletions(-)
Thomas Munro <tmunro@postgresql.org> writes:
> To make \watch react quickly when the user quits the pager or presses
> ^C, and also to increase the accuracy of its timing and decrease the
> rate of useless context switches, change the main loop of the \watch
> command to use sigwait() rather than a sleeping/polling loop, on Unix.
I think this is going to fall over on gaur, which doesn't have POSIX-style
sigwait. We've escaped dealing with that so far because our only existing
use of sigwait is hidden under
#if defined(ENABLE_THREAD_SAFETY) && !defined(WIN32)
Perhaps the easiest answer is to make the #if conditional for this
code look like that too.
regards, tom lane
On Tue, Jul 13, 2021 at 12:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Thomas Munro <tmunro@postgresql.org> writes: > > To make \watch react quickly when the user quits the pager or presses > > ^C, and also to increase the accuracy of its timing and decrease the > > rate of useless context switches, change the main loop of the \watch > > command to use sigwait() rather than a sleeping/polling loop, on Unix. > > I think this is going to fall over on gaur, which doesn't have POSIX-style > sigwait. We've escaped dealing with that so far because our only existing > use of sigwait is hidden under > > #if defined(ENABLE_THREAD_SAFETY) && !defined(WIN32) > > Perhaps the easiest answer is to make the #if conditional for this > code look like that too. Oh, thanks for the advance warning. Wouldn't HAVE_SIGWAIT be better? Like so.
Вложения
Thomas Munro <thomas.munro@gmail.com> writes:
> On Tue, Jul 13, 2021 at 12:30 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think this is going to fall over on gaur, which doesn't have POSIX-style
>> sigwait.
> Oh, thanks for the advance warning. Wouldn't HAVE_SIGWAIT be better? Like so.
That won't help as-is, because it *does* have sigwait, just not with
the POSIX API. thread_test.c does this:
/* Test for POSIX.1c 2-arg sigwait() and fail on single-arg version */
#include <signal.h>
int sigwait(const sigset_t *set, int *sig);
which perhaps should be pulled out of there and moved to the
configure script proper.
regards, tom lane
On Tue, Jul 13, 2021 at 1:09 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Thomas Munro <thomas.munro@gmail.com> writes: > > Oh, thanks for the advance warning. Wouldn't HAVE_SIGWAIT be better? Like so. > > That won't help as-is, because it *does* have sigwait, just not with > the POSIX API. thread_test.c does this: > > /* Test for POSIX.1c 2-arg sigwait() and fail on single-arg version */ > #include <signal.h> > int sigwait(const sigset_t *set, int *sig); > > which perhaps should be pulled out of there and moved to the > configure script proper. Ah, I see. I'll have a crack at that after lunch.
Thomas Munro <thomas.munro@gmail.com> writes:
> On Tue, Jul 13, 2021 at 1:09 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> That won't help as-is, because it *does* have sigwait, just not with
>> the POSIX API. thread_test.c does this:
>>
>> /* Test for POSIX.1c 2-arg sigwait() and fail on single-arg version */
>> #include <signal.h>
>> int sigwait(const sigset_t *set, int *sig);
>>
>> which perhaps should be pulled out of there and moved to the
>> configure script proper.
> Ah, I see. I'll have a crack at that after lunch.
Huh ... gaur did this:
/usr/ccs/bin/ld: Unsatisfied symbols:
sigwait (code)
which is not what I was expecting, because there is a definition for
sigwait in /usr/include (though not in a header file we use). It must
be in some add-on library instead of libc.
However, wrasse did this:
"/export/home/nm/farm/studio64v12_6/HEAD/pgsql.build/../pgsql/src/bin/psql/command.c", line 5062: prototype mismatch: 2
argspassed, 1 expected
cc: acomp failed for /export/home/nm/farm/studio64v12_6/HEAD/pgsql.build/../pgsql/src/bin/psql/command.c
which I was expecting even less. Evidently, prehistoric HPUX is not the
only platform still using the pre-POSIX API for this function. So we
really do need the full configure check.
regards, tom lane
On Tue, Jul 13, 2021 at 4:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Huh ... gaur did this: > > /usr/ccs/bin/ld: Unsatisfied symbols: > sigwait (code) > > which is not what I was expecting, because there is a definition for > sigwait in /usr/include (though not in a header file we use). It must > be in some add-on library instead of libc. Could be part of "DCE threads" (= "draft 4"), from last time I googled this topic when gaur blew up. > However, wrasse did this: > > "/export/home/nm/farm/studio64v12_6/HEAD/pgsql.build/../pgsql/src/bin/psql/command.c", line 5062: prototype mismatch: 2args passed, 1 expected > cc: acomp failed for /export/home/nm/farm/studio64v12_6/HEAD/pgsql.build/../pgsql/src/bin/psql/command.c > > which I was expecting even less. Evidently, prehistoric HPUX is not the > only platform still using the pre-POSIX API for this function. So we > really do need the full configure check. Huh, that is surprising. Apparently it has "draft 6" and also final, depending on a macro: https://illumos.org/man/2/sigwait https://docs.oracle.com/cd/E36784_01/html/E36872/sigwait-2.html Here's a first swing at a configure check. Could probably be improved a bit, as I haven't yet tried to share the check with thread_test.c (I was going to ask if it might be entirely removable since draft 4 systems like gaur surely can't compile the rest of thread_test.c due to other differences we know about, but now that I know that there are also draft 6 systems out there...). Note also the errno change, which I (ironically) did the pre-standard way by mistake.
Вложения
On Tue, Jul 13, 2021 at 5:18 PM Thomas Munro <thomas.munro@gmail.com> wrote: > https://illumos.org/man/2/sigwait > https://docs.oracle.com/cd/E36784_01/html/E36872/sigwait-2.html Oh, hmm, that's something we already have to do elsewhere to get standard conforming interfaces on that platform: # Some platforms use these, so just define them. They can't hurt if they # are not supported. For example, on Solaris -D_POSIX_PTHREAD_SEMANTICS # enables 5-arg getpwuid_r, among other things. PTHREAD_CFLAGS="$PTHREAD_CFLAGS -D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS"
Here's a better patch. I check if there is any declaration at all, which ancient HPUX should fail based on: command.c:5062:4: warning: implicit declaration of function 'sigwait' Then I also check that there isn't an incompatible declaration with the technique from thread_test.c, which Solaris should fail, based on: command.c:5062:8: error: too many arguments to function 'sigwait' A well placed -D_POSIX_PTHREAD_SEMANTICS might allow it work on Solaris, but I'm not sure if it's OK to do that without other threading options and I don't have access to test.
Вложения
Thomas Munro <thomas.munro@gmail.com> writes:
> Here's a better patch.
I didn't like the way you'd done this as two independent tests;
the second one falsely reports success if there isn't actually
any declaration of sigwait. I see you hacked around that by
checking both results in c.h, but the printed result is still
very misleading. Plus it's a waste of time to run the second
test at all, if the first one fails.
Here's a revised patch that I've tested (albeit lightly) on
both HPUX and Solaris. I also got rid of the independent
check in thread_test.c, since that's always been a hack
that delivers a misleading complaint when it fails.
Personally, I'd skip adding the stanza in c.h in favor of just
making configure set USE_SIGWAIT directly, but perhaps you
see it differently.
regards, tom lane
diff --git a/config/thread_test.c b/config/thread_test.c
index 784f4fe8ce..e2a9e62f49 100644
--- a/config/thread_test.c
+++ b/config/thread_test.c
@@ -43,10 +43,6 @@
#include <winsock2.h>
#endif
-/* Test for POSIX.1c 2-arg sigwait() and fail on single-arg version */
-#include <signal.h>
-int sigwait(const sigset_t *set, int *sig);
-
#define TEMP_FILENAME_1 "thread_test.1"
#define TEMP_FILENAME_2 "thread_test.2"
diff --git a/configure b/configure
index 1ea28a0d67..111751defa 100755
--- a/configure
+++ b/configure
@@ -15861,9 +15861,10 @@ $as_echo "#define HAVE_FSEEKO 1" >>confdefs.h
fi
-# posix_fadvise() is a no-op on Solaris, so don't incur function overhead
-# by calling it, 2009-04-02
-# http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/posix_fadvise.c
+# Make sure there's a declaration for sigwait(), then make sure
+# that it conforms to the POSIX standard (there seem to still be
+# some platforms out there with pre-POSIX sigwait()). On Solaris,
+# PTHREAD_CFLAGS affects the result.
# The Clang compiler raises a warning for an undeclared identifier that matches
# a compiler builtin function. All extant Clang versions are affected, as of
# Clang 3.6.0. Test a builtin known to every version. This problem affects the
@@ -15952,6 +15953,69 @@ case $ac_cv_c_decl_report in
*) ac_c_decl_warn_flag= ;;
esac
+ac_fn_c_check_decl "$LINENO" "sigwait" "ac_cv_have_decl_sigwait" "#include <signal.h>
+"
+if test "x$ac_cv_have_decl_sigwait" = xyes; then :
+ ac_have_decl=1
+else
+ ac_have_decl=0
+fi
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_DECL_SIGWAIT $ac_have_decl
+_ACEOF
+
+if test "x$ac_cv_have_decl_sigwait" = xyes; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for POSIX-conforming sigwait declaration" >&5
+$as_echo_n "checking for POSIX-conforming sigwait declaration... " >&6; }
+if ${pgac_cv_have_posix_decl_sigwait+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ _CFLAGS="$CFLAGS"
+ _LIBS="$LIBS"
+ CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
+ LIBS="$LIBS $PTHREAD_LIBS"
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+ #include <signal.h>
+ int sigwait(const sigset_t *set, int *sig);
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv_have_posix_decl_sigwait=yes
+else
+ pgac_cv_have_posix_decl_sigwait=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ CFLAGS="$_CFLAGS"
+ LIBS="$_LIBS"
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_have_posix_decl_sigwait" >&5
+$as_echo "$pgac_cv_have_posix_decl_sigwait" >&6; }
+fi
+if test "x$pgac_cv_have_posix_decl_sigwait" = xyes; then
+
+$as_echo "#define HAVE_POSIX_DECL_SIGWAIT 1" >>confdefs.h
+
+else
+ # On non-Windows, libpq requires POSIX sigwait() for thread safety.
+ if test "$enable_thread_safety" = yes -a "$PORTNAME" != "win32"; then
+ as_fn_error $? "POSIX-conforming sigwait is required to enable thread safety." "$LINENO" 5
+ fi
+fi
+
+# posix_fadvise() is a no-op on Solaris, so don't incur function overhead
+# by calling it, 2009-04-02
+# http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/posix_fadvise.c
if test "$PORTNAME" != "solaris"; then :
for ac_func in posix_fadvise
diff --git a/configure.ac b/configure.ac
index 57336e1fb6..1b0e32681a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1741,6 +1741,39 @@ PGAC_CHECK_BUILTIN_FUNC([__builtin_popcount], [unsigned int x])
# in case it finds that _LARGEFILE_SOURCE has to be #define'd for that.
AC_FUNC_FSEEKO
+# Make sure there's a declaration for sigwait(), then make sure
+# that it conforms to the POSIX standard (there seem to still be
+# some platforms out there with pre-POSIX sigwait()). On Solaris,
+# PTHREAD_CFLAGS affects the result.
+AC_CHECK_DECLS(sigwait, [], [], [#include <signal.h>])
+if test "x$ac_cv_have_decl_sigwait" = xyes; then
+ AC_CACHE_CHECK([for POSIX-conforming sigwait declaration],
+ [pgac_cv_have_posix_decl_sigwait],
+ [_CFLAGS="$CFLAGS"
+ _LIBS="$LIBS"
+ CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
+ LIBS="$LIBS $PTHREAD_LIBS"
+ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([
+ #include <signal.h>
+ int sigwait(const sigset_t *set, int *sig);
+ ],
+ [])],
+ [pgac_cv_have_posix_decl_sigwait=yes],
+ [pgac_cv_have_posix_decl_sigwait=no])
+ CFLAGS="$_CFLAGS"
+ LIBS="$_LIBS"
+ ])
+fi
+if test "x$pgac_cv_have_posix_decl_sigwait" = xyes; then
+ AC_DEFINE(HAVE_POSIX_DECL_SIGWAIT, 1,
+ [Define to 1 if you have a POSIX-conforming sigwait declaration.])
+else
+ # On non-Windows, libpq requires POSIX sigwait() for thread safety.
+ if test "$enable_thread_safety" = yes -a "$PORTNAME" != "win32"; then
+ AC_MSG_ERROR([POSIX-conforming sigwait is required to enable thread safety.])
+ fi
+fi
+
# posix_fadvise() is a no-op on Solaris, so don't incur function overhead
# by calling it, 2009-04-02
# http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/posix_fadvise.c
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index d704c4220c..376620f68f 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4899,7 +4899,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
FILE *pagerpipe = NULL;
int title_len;
int res = 0;
-#ifndef WIN32
+#ifdef USE_SIGWAIT
sigset_t sigalrm_sigchld_sigint;
sigset_t sigalrm_sigchld;
sigset_t sigint;
@@ -4913,7 +4913,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
return false;
}
-#ifndef WIN32
+#ifdef USE_SIGWAIT
sigemptyset(&sigalrm_sigchld_sigint);
sigaddset(&sigalrm_sigchld_sigint, SIGCHLD);
sigaddset(&sigalrm_sigchld_sigint, SIGALRM);
@@ -4952,7 +4952,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
* PAGER environment variables, because traditional pagers probably won't
* be very useful for showing a stream of results.
*/
-#ifndef WIN32
+#ifdef USE_SIGWAIT
pagerprog = getenv("PSQL_WATCH_PAGER");
#endif
if (pagerprog && myopt.topt.pager)
@@ -5023,7 +5023,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
if (pagerpipe && ferror(pagerpipe))
break;
-#ifdef WIN32
+#ifndef USE_SIGWAIT
/*
* Set up cancellation of 'watch' via SIGINT. We redo this each time
@@ -5059,7 +5059,8 @@ do_watch(PQExpBuffer query_buf, double sleep)
{
int signal_received;
- if (sigwait(&sigalrm_sigchld_sigint, &signal_received) < 0)
+ errno = sigwait(&sigalrm_sigchld_sigint, &signal_received);
+ if (errno != 0)
{
/* Some other signal arrived? */
if (errno == EINTR)
@@ -5091,7 +5092,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
restore_sigpipe_trap();
}
-#ifndef WIN32
+#ifdef USE_SIGWAIT
/* Disable the interval timer. */
memset(&interval, 0, sizeof(interval));
setitimer(ITIMER_REAL, &interval, NULL);
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 5f36f0d1c6..f0b46a5efc 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -110,7 +110,7 @@ log_locus_callback(const char **filename, uint64 *lineno)
}
}
-#ifndef WIN32
+#ifdef USE_SIGWAIT
static void
empty_signal_handler(SIGNAL_ARGS)
{
@@ -309,7 +309,7 @@ main(int argc, char *argv[])
psql_setup_cancel_handler();
-#ifndef WIN32
+#ifdef USE_SIGWAIT
/*
* do_watch() needs signal handlers installed (otherwise sigwait() will
diff --git a/src/include/c.h b/src/include/c.h
index c8ede08273..4feae5ef57 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -1312,6 +1312,10 @@ extern long long strtoll(const char *str, char **endptr, int base);
extern unsigned long long strtoull(const char *str, char **endptr, int base);
#endif
+#if defined(HAVE_POSIX_DECL_SIGWAIT)
+#define USE_SIGWAIT
+#endif
+
/* no special DLL markers on most ports */
#ifndef PGDLLIMPORT
#define PGDLLIMPORT
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index d69d461ff2..15ffdd895a 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -158,6 +158,10 @@
don't. */
#undef HAVE_DECL_RTLD_NOW
+/* Define to 1 if you have the declaration of `sigwait', and to 0 if you
+ don't. */
+#undef HAVE_DECL_SIGWAIT
+
/* Define to 1 if you have the declaration of `strlcat', and to 0 if you
don't. */
#undef HAVE_DECL_STRLCAT
@@ -414,6 +418,9 @@
/* Define to 1 if you have the <poll.h> header file. */
#undef HAVE_POLL_H
+/* Define to 1 if you have a POSIX-conforming sigwait declaration. */
+#undef HAVE_POSIX_DECL_SIGWAIT
+
/* Define to 1 if you have the `posix_fadvise' function. */
#undef HAVE_POSIX_FADVISE
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 294b968dcd..c967743467 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -249,6 +249,7 @@ sub GenerateFiles
HAVE_DECL_PWRITEV => 0,
HAVE_DECL_RTLD_GLOBAL => 0,
HAVE_DECL_RTLD_NOW => 0,
+ HAVE_DECL_SIGWAIT => 0,
HAVE_DECL_STRLCAT => undef,
HAVE_DECL_STRLCPY => undef,
HAVE_DECL_STRNLEN => 1,
@@ -332,6 +333,7 @@ sub GenerateFiles
HAVE_PAM_PAM_APPL_H => undef,
HAVE_POLL => undef,
HAVE_POLL_H => undef,
+ HAVE_POSIX_DECL_SIGWAIT => undef,
HAVE_POSIX_FADVISE => undef,
HAVE_POSIX_FALLOCATE => undef,
HAVE_PPC_LWARX_MUTEX_HINT => undef,
I wrote:
> Here's a revised patch that I've tested (albeit lightly) on
> both HPUX and Solaris.
Hm, I'd verified the configure results, but I didn't wait around
for the builds to finish, which was a mistake. On Solaris 11:
command.c: In function 'do_watch':
command.c:5062:8: error: too many arguments to function 'sigwait'
if (sigwait(&sigalrm_sigchld_sigint, &signal_received) < 0)
^
In file included from ../../../src/include/fe_utils/print.h:16:0,
from command.h:12,
from command.c:28:
/usr/include/signal.h:233:12: note: declared here
extern int sigwait(sigset_t *);
^
gmake[3]: *** [<builtin>: command.o] Error 1
gmake[2]: *** [Makefile:43: all-psql-recurse] Error 2
gmake[1]: *** [Makefile:42: all-bin-recurse] Error 2
gmake: *** [GNUmakefile:11: all-src-recurse] Error 2
What is happening is obvious in retrospect. Per my patch, configure
checks how sigwait is declared with -D_POSIX_PTHREAD_SEMANTICS, and
that's also how libpq's reference is compiled ... but psql's reference
is not compiled that way. It seems to work if we force command.c
to be compiled with PTHREAD_CFLAGS added, but that is really quite
grotty. I think it'd be safer to push the sigwait reference out
into a separate file to minimize the scope of those flags.
Just how badly did you want to use sigwait here? I'm having
considerable second thoughts about the value of that change
versus the hoops we're going to have to jump through to use it.
(I suppose a hacky solution might be to never define USE_SIGWAIT
on Solaris.)
regards, tom lane
On Wed, Jul 14, 2021 at 6:17 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Just how badly did you want to use sigwait here? I'm having > considerable second thoughts about the value of that change > versus the hoops we're going to have to jump through to use it. I'm sure I could find another way if it comes to it. The original proposal had a sleep(0.1) loop... I thought this was more elegant, but obviously I didn't know about this portability problem and there are surely other ways. (Searching for sigwait in the archives brings up some fun discussion of other long forgotten dinosaurs that also had this problem.) > (I suppose a hacky solution might be to never define USE_SIGWAIT > on Solaris.) Yeah I was thinking about that. I've just got a shell on an illumos VM and will try a couple of ideas out if I can remember how to drive this thing... more soon.
On Wed, Jul 14, 2021 at 4:08 PM Thomas Munro <thomas.munro@gmail.com> wrote: > On Wed, Jul 14, 2021 at 6:17 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > (I suppose a hacky solution might be to never define USE_SIGWAIT > > on Solaris.) > > Yeah I was thinking about that. I've just got a shell on an illumos > VM and will try a couple of ideas out if I can remember how to drive > this thing... more soon. I decided to try to make it work properly on that OS instead of the hacky solution now that I have access. I took your last patch, moved -D_POSIX_PTHREAD_SEMANTICS into CFLAGS in src/template/solaris, removed the special flags and libs from the new configure check where you had them, and then used HAVE_POSIX_SIGWAIT directly without the c.h change. I can't find any evidence on the 'net that any other OS cares about _POSIX_PTHREAD_SEMANTICS (every reference says it's a Solaris thing) so this should just remove some useless clutter, and it's not like we want POSIX draft 6 behaviour anywhere. (It's a bit weird to define a macro that has PTHREAD in the name when building things that aren't multi-threaded, but that was their choice). Do you see any problems with this approach? The main thing I can think of is that people who are using a custom CFLAGS on that OS will need to update the value they're using, but that seems OK (they'll likely see the new error and be alerted to the change). Another thing I noticed is that config/ax_pthread.m4 (something from autoconf that we don't want to modify) will add another copy of -D_POSIX_PTHREAD_SEMANTICS, but that's already the case.
Вложения
Thomas Munro <thomas.munro@gmail.com> writes:
> I decided to try to make it work properly on that OS instead of the
> hacky solution now that I have access. I took your last patch, moved
> -D_POSIX_PTHREAD_SEMANTICS into CFLAGS in src/template/solaris,
Aha! Yeah, just defining _POSIX_PTHREAD_SEMANTICS unconditionally
on Solaris seems like a good idea. However, messing with the template
file is not the best way to inject that, because if the user specifies
CFLAGS then the template file's setting is ignored. It's better to
handle it in the part of configure.ac that automatically adds flags
onto whatever the user or template file gave. Also, being a -D
switch, it really oughta go into CPPFLAGS not CFLAGS.
I've tested the attached on the GCC farm's Solaris machine (which
I believe is the same machine wrasse runs on), and it seems OK.
I think it's ready to go.
> I can't find any evidence on the 'net that any other OS
> cares about _POSIX_PTHREAD_SEMANTICS (every reference says it's a
> Solaris thing) so this should just remove some useless clutter,
Agreed; if we do find any other platforms that need it, we can extend
the PORTNAME check used here.
> Another thing
> I noticed is that config/ax_pthread.m4 (something from autoconf that
> we don't want to modify) will add another copy of
> -D_POSIX_PTHREAD_SEMANTICS, but that's already the case.
Ah, yeah, I see:
$ grep POSIX src/Makefile.global
PTHREAD_CFLAGS = -D_POSIX_PTHREAD_SEMANTICS -pthread -D_REENTRANT -D_THREAD_SAFE
CPPFLAGS = -D_POSIX_PTHREAD_SEMANTICS
Seems pretty harmless, and anyway we have to make sure this gets in there
whether or not --enable-thread-safety is given.
regards, tom lane
diff --git a/config/thread_test.c b/config/thread_test.c
index 784f4fe8ce..e2a9e62f49 100644
--- a/config/thread_test.c
+++ b/config/thread_test.c
@@ -43,10 +43,6 @@
#include <winsock2.h>
#endif
-/* Test for POSIX.1c 2-arg sigwait() and fail on single-arg version */
-#include <signal.h>
-int sigwait(const sigset_t *set, int *sig);
-
#define TEMP_FILENAME_1 "thread_test.1"
#define TEMP_FILENAME_2 "thread_test.2"
diff --git a/configure b/configure
index 1ea28a0d67..c85eb1bf55 100755
--- a/configure
+++ b/configure
@@ -7194,6 +7194,12 @@ $as_echo "#define PROFILE_PID_DIR 1" >>confdefs.h
fi
fi
+# On Solaris, we need this #define to get POSIX-conforming versions
+# of many interfaces (sigwait, getpwuid_r, ...).
+if test "$PORTNAME" = "solaris"; then
+ CPPFLAGS="$CPPFLAGS -D_POSIX_PTHREAD_SEMANTICS"
+fi
+
# We already have this in Makefile.win32, but configure needs it too
if test "$PORTNAME" = "win32"; then
CPPFLAGS="$CPPFLAGS -I$srcdir/src/include/port/win32"
@@ -11296,9 +11302,8 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
# set thread flags
# Some platforms use these, so just define them. They can't hurt if they
-# are not supported. For example, on Solaris -D_POSIX_PTHREAD_SEMANTICS
-# enables 5-arg getpwuid_r, among other things.
-PTHREAD_CFLAGS="$PTHREAD_CFLAGS -D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS"
+# are not supported.
+PTHREAD_CFLAGS="$PTHREAD_CFLAGS -D_REENTRANT -D_THREAD_SAFE"
# Check for *_r functions
_CFLAGS="$CFLAGS"
@@ -15861,9 +15866,11 @@ $as_echo "#define HAVE_FSEEKO 1" >>confdefs.h
fi
-# posix_fadvise() is a no-op on Solaris, so don't incur function overhead
-# by calling it, 2009-04-02
-# http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/posix_fadvise.c
+# Make sure there's a declaration for sigwait(), then make sure
+# that it conforms to the POSIX standard (there seem to still be
+# some platforms out there with pre-POSIX sigwait()). On Solaris,
+# _POSIX_PTHREAD_SEMANTICS affects the result, but we already
+# added that to CPPFLAGS.
# The Clang compiler raises a warning for an undeclared identifier that matches
# a compiler builtin function. All extant Clang versions are affected, as of
# Clang 3.6.0. Test a builtin known to every version. This problem affects the
@@ -15952,6 +15959,62 @@ case $ac_cv_c_decl_report in
*) ac_c_decl_warn_flag= ;;
esac
+ac_fn_c_check_decl "$LINENO" "sigwait" "ac_cv_have_decl_sigwait" "#include <signal.h>
+"
+if test "x$ac_cv_have_decl_sigwait" = xyes; then :
+ ac_have_decl=1
+else
+ ac_have_decl=0
+fi
+
+cat >>confdefs.h <<_ACEOF
+#define HAVE_DECL_SIGWAIT $ac_have_decl
+_ACEOF
+
+if test "x$ac_cv_have_decl_sigwait" = xyes; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for POSIX-conforming sigwait declaration" >&5
+$as_echo_n "checking for POSIX-conforming sigwait declaration... " >&6; }
+if ${pgac_cv_have_posix_decl_sigwait+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+ #include <signal.h>
+ int sigwait(const sigset_t *set, int *sig);
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv_have_posix_decl_sigwait=yes
+else
+ pgac_cv_have_posix_decl_sigwait=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_have_posix_decl_sigwait" >&5
+$as_echo "$pgac_cv_have_posix_decl_sigwait" >&6; }
+fi
+if test "x$pgac_cv_have_posix_decl_sigwait" = xyes; then
+
+$as_echo "#define HAVE_POSIX_DECL_SIGWAIT 1" >>confdefs.h
+
+else
+ # On non-Windows, libpq requires POSIX sigwait() for thread safety.
+ if test "$enable_thread_safety" = yes -a "$PORTNAME" != "win32"; then
+ as_fn_error $? "POSIX-conforming sigwait is required to enable thread safety." "$LINENO" 5
+ fi
+fi
+
+# posix_fadvise() is a no-op on Solaris, so don't incur function overhead
+# by calling it, 2009-04-02
+# http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/posix_fadvise.c
if test "$PORTNAME" != "solaris"; then :
for ac_func in posix_fadvise
diff --git a/configure.ac b/configure.ac
index 57336e1fb6..099c4a5a45 100644
--- a/configure.ac
+++ b/configure.ac
@@ -610,6 +610,12 @@ if test "$enable_profiling" = yes && test "$ac_cv_prog_cc_g" = yes; then
fi
fi
+# On Solaris, we need this #define to get POSIX-conforming versions
+# of many interfaces (sigwait, getpwuid_r, ...).
+if test "$PORTNAME" = "solaris"; then
+ CPPFLAGS="$CPPFLAGS -D_POSIX_PTHREAD_SEMANTICS"
+fi
+
# We already have this in Makefile.win32, but configure needs it too
if test "$PORTNAME" = "win32"; then
CPPFLAGS="$CPPFLAGS -I$srcdir/src/include/port/win32"
@@ -1122,9 +1128,8 @@ AS_IF([test "$enable_thread_safety" = yes -a "$PORTNAME" != "win32"],
AX_PTHREAD # set thread flags
# Some platforms use these, so just define them. They can't hurt if they
-# are not supported. For example, on Solaris -D_POSIX_PTHREAD_SEMANTICS
-# enables 5-arg getpwuid_r, among other things.
-PTHREAD_CFLAGS="$PTHREAD_CFLAGS -D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS"
+# are not supported.
+PTHREAD_CFLAGS="$PTHREAD_CFLAGS -D_REENTRANT -D_THREAD_SAFE"
# Check for *_r functions
_CFLAGS="$CFLAGS"
@@ -1741,6 +1746,33 @@ PGAC_CHECK_BUILTIN_FUNC([__builtin_popcount], [unsigned int x])
# in case it finds that _LARGEFILE_SOURCE has to be #define'd for that.
AC_FUNC_FSEEKO
+# Make sure there's a declaration for sigwait(), then make sure
+# that it conforms to the POSIX standard (there seem to still be
+# some platforms out there with pre-POSIX sigwait()). On Solaris,
+# _POSIX_PTHREAD_SEMANTICS affects the result, but we already
+# added that to CPPFLAGS.
+AC_CHECK_DECLS(sigwait, [], [], [#include <signal.h>])
+if test "x$ac_cv_have_decl_sigwait" = xyes; then
+ AC_CACHE_CHECK([for POSIX-conforming sigwait declaration],
+ [pgac_cv_have_posix_decl_sigwait],
+ [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([
+ #include <signal.h>
+ int sigwait(const sigset_t *set, int *sig);
+ ],
+ [])],
+ [pgac_cv_have_posix_decl_sigwait=yes],
+ [pgac_cv_have_posix_decl_sigwait=no])])
+fi
+if test "x$pgac_cv_have_posix_decl_sigwait" = xyes; then
+ AC_DEFINE(HAVE_POSIX_DECL_SIGWAIT, 1,
+ [Define to 1 if you have a POSIX-conforming sigwait declaration.])
+else
+ # On non-Windows, libpq requires POSIX sigwait() for thread safety.
+ if test "$enable_thread_safety" = yes -a "$PORTNAME" != "win32"; then
+ AC_MSG_ERROR([POSIX-conforming sigwait is required to enable thread safety.])
+ fi
+fi
+
# posix_fadvise() is a no-op on Solaris, so don't incur function overhead
# by calling it, 2009-04-02
# http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/posix_fadvise.c
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index d704c4220c..49d4c0e3ce 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -4899,7 +4899,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
FILE *pagerpipe = NULL;
int title_len;
int res = 0;
-#ifndef WIN32
+#ifdef HAVE_POSIX_DECL_SIGWAIT
sigset_t sigalrm_sigchld_sigint;
sigset_t sigalrm_sigchld;
sigset_t sigint;
@@ -4913,7 +4913,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
return false;
}
-#ifndef WIN32
+#ifdef HAVE_POSIX_DECL_SIGWAIT
sigemptyset(&sigalrm_sigchld_sigint);
sigaddset(&sigalrm_sigchld_sigint, SIGCHLD);
sigaddset(&sigalrm_sigchld_sigint, SIGALRM);
@@ -4952,7 +4952,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
* PAGER environment variables, because traditional pagers probably won't
* be very useful for showing a stream of results.
*/
-#ifndef WIN32
+#ifdef HAVE_POSIX_DECL_SIGWAIT
pagerprog = getenv("PSQL_WATCH_PAGER");
#endif
if (pagerprog && myopt.topt.pager)
@@ -5023,7 +5023,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
if (pagerpipe && ferror(pagerpipe))
break;
-#ifdef WIN32
+#ifndef HAVE_POSIX_DECL_SIGWAIT
/*
* Set up cancellation of 'watch' via SIGINT. We redo this each time
@@ -5059,7 +5059,8 @@ do_watch(PQExpBuffer query_buf, double sleep)
{
int signal_received;
- if (sigwait(&sigalrm_sigchld_sigint, &signal_received) < 0)
+ errno = sigwait(&sigalrm_sigchld_sigint, &signal_received);
+ if (errno != 0)
{
/* Some other signal arrived? */
if (errno == EINTR)
@@ -5091,7 +5092,7 @@ do_watch(PQExpBuffer query_buf, double sleep)
restore_sigpipe_trap();
}
-#ifndef WIN32
+#ifdef HAVE_POSIX_DECL_SIGWAIT
/* Disable the interval timer. */
memset(&interval, 0, sizeof(interval));
setitimer(ITIMER_REAL, &interval, NULL);
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 5f36f0d1c6..2931530f33 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -110,7 +110,7 @@ log_locus_callback(const char **filename, uint64 *lineno)
}
}
-#ifndef WIN32
+#ifdef HAVE_POSIX_DECL_SIGWAIT
static void
empty_signal_handler(SIGNAL_ARGS)
{
@@ -309,7 +309,7 @@ main(int argc, char *argv[])
psql_setup_cancel_handler();
-#ifndef WIN32
+#ifdef HAVE_POSIX_DECL_SIGWAIT
/*
* do_watch() needs signal handlers installed (otherwise sigwait() will
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index d69d461ff2..15ffdd895a 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -158,6 +158,10 @@
don't. */
#undef HAVE_DECL_RTLD_NOW
+/* Define to 1 if you have the declaration of `sigwait', and to 0 if you
+ don't. */
+#undef HAVE_DECL_SIGWAIT
+
/* Define to 1 if you have the declaration of `strlcat', and to 0 if you
don't. */
#undef HAVE_DECL_STRLCAT
@@ -414,6 +418,9 @@
/* Define to 1 if you have the <poll.h> header file. */
#undef HAVE_POLL_H
+/* Define to 1 if you have a POSIX-conforming sigwait declaration. */
+#undef HAVE_POSIX_DECL_SIGWAIT
+
/* Define to 1 if you have the `posix_fadvise' function. */
#undef HAVE_POSIX_FADVISE
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 294b968dcd..c967743467 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -249,6 +249,7 @@ sub GenerateFiles
HAVE_DECL_PWRITEV => 0,
HAVE_DECL_RTLD_GLOBAL => 0,
HAVE_DECL_RTLD_NOW => 0,
+ HAVE_DECL_SIGWAIT => 0,
HAVE_DECL_STRLCAT => undef,
HAVE_DECL_STRLCPY => undef,
HAVE_DECL_STRNLEN => 1,
@@ -332,6 +333,7 @@ sub GenerateFiles
HAVE_PAM_PAM_APPL_H => undef,
HAVE_POLL => undef,
HAVE_POLL_H => undef,
+ HAVE_POSIX_DECL_SIGWAIT => undef,
HAVE_POSIX_FADVISE => undef,
HAVE_POSIX_FALLOCATE => undef,
HAVE_PPC_LWARX_MUTEX_HINT => undef,
On Thu, Jul 15, 2021 at 4:07 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I've tested the attached on the GCC farm's Solaris machine (which > I believe is the same machine wrasse runs on), and it seems OK. > I think it's ready to go. Pushed. Thanks!