From 894459e59ffa5c7fee297b246c17e1f72564db1d Mon Sep 17 00:00:00 2001 From: Noah Misch Date: Wed, 7 Jan 2015 22:35:44 -0500 Subject: [PATCH] On Darwin, detect and report a multithreaded postmaster. Darwin --enable-nls builds use a substitute setlocale() that may start a thread. Buildfarm member orangutan experienced BackendList corruption on account of different postmaster threads executing signal handlers simultaneously. Furthermore, a multithreaded postmaster risks undefined behavior from sigprocmask() and fork(). Emit LOG messages about the problem and its workaround. Back-patch to 9.0 (all supported versions). --- configure | 2 +- configure.in | 2 +- src/backend/postmaster/postmaster.c | 43 +++++++++++++++++++++++++++++ src/common/exec.c | 12 ++++++++ src/include/pg_config.h.in | 3 ++ 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 2a8c4fd1f2..fce49088a8 100755 --- a/configure +++ b/configure @@ -11366,7 +11366,7 @@ fi LIBS_including_readline="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes wcstombs wcstombs_l +for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes wcstombs wcstombs_l do : as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" diff --git a/configure.in b/configure.in index 8ca03ab4ae..6aa69fb19f 100644 --- a/configure.in +++ b/configure.in @@ -1265,7 +1265,7 @@ PGAC_FUNC_GETTIMEOFDAY_1ARG LIBS_including_readline="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes wcstombs wcstombs_l]) +AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes wcstombs wcstombs_l]) AC_REPLACE_FUNCS(fseeko) case $host_os in diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index c3cff7ccc4..f33c4fed81 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -87,6 +87,10 @@ #include #endif +#ifdef HAVE_PTHREAD_IS_THREADED_NP +#include +#endif + #include "access/transam.h" #include "access/xlog.h" #include "bootstrap/bootstrap.h" @@ -1200,6 +1204,24 @@ PostmasterMain(int argc, char *argv[]) */ RemovePgTempFiles(); +#ifdef HAVE_PTHREAD_IS_THREADED_NP + + /* + * On Darwin, libintl replaces setlocale() with a version that calls + * CFLocaleCopyCurrent() when its second argument is "" and every relevant + * environment variable is unset or empty. CFLocaleCopyCurrent() makes + * the process multithreaded. The postmaster calls sigprocmask() and + * calls fork() without an immediate exec(), both of which have undefined + * behavior in a multithreaded program. A multithreaded postmaster is the + * normal case on Windows, which offers neither fork() nor sigprocmask(). + */ + if (pthread_is_threaded_np() != 0) + ereport(LOG, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("postmaster became multithreaded during startup"), + errhint("Set the LC_ALL environment variable to a valid locale."))); +#endif + /* * Remember postmaster startup time */ @@ -1657,6 +1679,15 @@ ServerLoop(void) last_touch_time = now; } +#ifdef HAVE_PTHREAD_IS_THREADED_NP + + /* + * With assertions enabled, check regularly for appearance of + * additional threads. All builds check at start and exit. + */ + Assert(pthread_is_threaded_np() == 0); +#endif + /* * If we already sent SIGQUIT to children and they are slow to shut * down, it's time to send them SIGKILL. This doesn't happen @@ -4745,6 +4776,18 @@ SubPostmasterMain(int argc, char *argv[]) static void ExitPostmaster(int status) { +#ifdef HAVE_PTHREAD_IS_THREADED_NP + + /* + * There is no known cause for a postmaster to become multithreaded after + * startup. Recheck to account for the possibility of unknown causes. + */ + if (pthread_is_threaded_np() != 0) + ereport(LOG, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("postmaster became multithreaded"))); +#endif + /* should cleanup shared memory and kill all backends */ /* diff --git a/src/common/exec.c b/src/common/exec.c index b31d621e37..e5575d5f50 100644 --- a/src/common/exec.c +++ b/src/common/exec.c @@ -556,8 +556,20 @@ set_pglocale_pgservice(const char *argv0, const char *app) /* don't set LC_ALL in the backend */ if (strcmp(app, PG_TEXTDOMAIN("postgres")) != 0) + { setlocale(LC_ALL, ""); + /* + * One could make a case for reproducing here PostmasterMain()'s test + * for whether the process is multithreaded. Unlike the postmaster, + * no frontend program calls sigprocmask() or otherwise provides for + * mutual exclusion between signal handlers. While frontends using + * fork(), if multithreaded, are formally exposed to undefined + * behavior, we have not witnessed a concrete bug. Therefore, + * complaining about multithreading here may be mere pedantry. + */ + } + if (find_my_exec(argv0, my_exec_path) < 0) return; diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 465281c60e..83f04e9382 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -394,6 +394,9 @@ /* Define to 1 if the PS_STRINGS thing exists. */ #undef HAVE_PS_STRINGS +/* Define to 1 if you have the `pthread_is_threaded_np' function. */ +#undef HAVE_PTHREAD_IS_THREADED_NP + /* Define to 1 if you have the header file. */ #undef HAVE_PWD_H -- 2.40.0