On Wed, Oct 03, 2018 at 12:09:54PM +1300, Thomas Munro wrote: > It looks like you missed another case that needs tolerance for late > signal delivery on Windows: > > +select pg_cancel_backend(pg_backend_pid()); > > https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.15263
Looking at 0001, why are the declarations needed in patch 0002 part of 0001 (see signalfuncs.h)? I think that something like instead the attached is enough for this part. Daniel, could you confirm? -- Michael
diff --git a/src/backend/storage/ipc/Makefile b/src/backend/storage/ipc/Makefile
index 9dbdc26c9b..bf4619d5fd 100644
--- a/src/backend/storage/ipc/Makefile
+++ b/src/backend/storage/ipc/Makefile
@@ -8,8 +8,8 @@ subdir = src/backend/storage/ipc
top_builddir = ../../../..
include $(top_builddir)/src/Makefile.global
-OBJS = barrier.o dsm_impl.o dsm.o ipc.o ipci.o latch.o pmsignal.o procarray.o \
- procsignal.o shmem.o shmqueue.o shm_mq.o shm_toc.o sinval.o \
- sinvaladt.o standby.o
+OBJS = signalfuncs.o barrier.o dsm_impl.o dsm.o ipc.o ipci.o latch.o \
+ pmsignal.o procarray.o procsignal.o shmem.o shmqueue.o shm_mq.o \
+ shm_toc.o sinval.o sinvaladt.o standby.o
include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/storage/ipc/signalfuncs.c b/src/backend/storage/ipc/signalfuncs.c
new file mode 100644
index 0000000000..c09a047127
--- /dev/null
+++ b/src/backend/storage/ipc/signalfuncs.c
@@ -0,0 +1,216 @@
+/*-------------------------------------------------------------------------
+ *
+ * signalfuncs.c
+ * Functions for signalling backends
+ *
+ * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ * src/backend/storage/ipc/signalfuncs.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include <signal.h>
+
+#include "catalog/pg_authid.h"
+#include "funcapi.h"
+#include "miscadmin.h"
+#include "postmaster/syslogger.h"
+#include "storage/ipc.h"
+#include "storage/pmsignal.h"
+#include "storage/proc.h"
+#include "storage/procarray.h"
+#include "utils/acl.h"
+#include "utils/builtins.h"
+
+/*
+ * Send a signal to another backend.
+ *
+ * The signal is delivered if the user is either a superuser or the same
+ * role as the backend being signaled. For "dangerous" signals, an explicit
+ * check for superuser needs to be done prior to calling this function.
+ *
+ * Returns 0 on success, 1 on general failure, 2 on normal permission error
+ * and 3 if the caller needs to be a superuser.
+ *
+ * In the event of a general failure (return code 1), a warning message will
+ * be emitted. For permission errors, doing that is the responsibility of
+ * the caller.
+ */
+#define SIGNAL_BACKEND_SUCCESS 0
+#define SIGNAL_BACKEND_ERROR 1
+#define SIGNAL_BACKEND_NOPERMISSION 2
+#define SIGNAL_BACKEND_NOSUPERUSER 3
+static int
+pg_signal_backend(int pid, int sig)
+{
+ PGPROC *proc = BackendPidGetProc(pid);
+
+ /*
+ * BackendPidGetProc returns NULL if the pid isn't valid; but by the time
+ * we reach kill(), a process for which we get a valid proc here might
+ * have terminated on its own. There's no way to acquire a lock on an
+ * arbitrary process to prevent that. But since so far all the callers of
+ * this mechanism involve some request for ending the process anyway, that
+ * it might end on its own first is not a problem.
+ */
+ if (proc == NULL)
+ {
+ /*
+ * This is just a warning so a loop-through-resultset will not abort
+ * if one backend terminated on its own during the run.
+ */
+ ereport(WARNING,
+ (errmsg("PID %d is not a PostgreSQL server process", pid)));
+ return SIGNAL_BACKEND_ERROR;
+ }
+
+ /* Only allow superusers to signal superuser-owned backends. */
+ if (superuser_arg(proc->roleId) && !superuser())
+ return SIGNAL_BACKEND_NOSUPERUSER;
+
+ /* Users can signal backends they have role membership in. */
+ if (!has_privs_of_role(GetUserId(), proc->roleId) &&
+ !has_privs_of_role(GetUserId(), DEFAULT_ROLE_SIGNAL_BACKENDID))
+ return SIGNAL_BACKEND_NOPERMISSION;
+
+ /*
+ * Can the process we just validated above end, followed by the pid being
+ * recycled for a new process, before reaching here? Then we'd be trying
+ * to kill the wrong thing. Seems near impossible when sequential pid
+ * assignment and wraparound is used. Perhaps it could happen on a system
+ * where pid re-use is randomized. That race condition possibility seems
+ * too unlikely to worry about.
+ */
+
+ /* If we have setsid(), signal the backend's whole process group */
+#ifdef HAVE_SETSID
+ if (kill(-pid, sig))
+#else
+ if (kill(pid, sig))
+#endif
+ {
+ /* Again, just a warning to allow loops */
+ ereport(WARNING,
+ (errmsg("could not send signal to process %d: %m", pid)));
+ return SIGNAL_BACKEND_ERROR;
+ }
+ return SIGNAL_BACKEND_SUCCESS;
+}
+
+/*
+ * Signal to cancel a backend process. This is allowed if you are a member of
+ * the role whose process is being canceled.
+ *
+ * Note that only superusers can signal superuser-owned processes.
+ */
+Datum
+pg_cancel_backend(PG_FUNCTION_ARGS)
+{
+ int r = pg_signal_backend(PG_GETARG_INT32(0), SIGINT);
+
+ if (r == SIGNAL_BACKEND_NOSUPERUSER)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("must be a superuser to cancel superuser query"))));
+
+ if (r == SIGNAL_BACKEND_NOPERMISSION)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("must be a member of the role whose query is being canceled or member of pg_signal_backend"))));
+
+ PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
+}
+
+/*
+ * Signal to terminate a backend process. This is allowed if you are a member
+ * of the role whose process is being terminated.
+ *
+ * Note that only superusers can signal superuser-owned processes.
+ */
+Datum
+pg_terminate_backend(PG_FUNCTION_ARGS)
+{
+ int r = pg_signal_backend(PG_GETARG_INT32(0), SIGTERM);
+
+ if (r == SIGNAL_BACKEND_NOSUPERUSER)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("must be a superuser to terminate superuser process"))));
+
+ if (r == SIGNAL_BACKEND_NOPERMISSION)
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("must be a member of the role whose process is being terminated or member of pg_signal_backend"))));
+
+ PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
+}
+
+/*
+ * Signal to reload the database configuration
+ *
+ * Permission checking for this function is managed through the normal
+ * GRANT system.
+ */
+Datum
+pg_reload_conf(PG_FUNCTION_ARGS)
+{
+ if (kill(PostmasterPid, SIGHUP))
+ {
+ ereport(WARNING,
+ (errmsg("failed to send signal to postmaster: %m")));
+ PG_RETURN_BOOL(false);
+ }
+
+ PG_RETURN_BOOL(true);
+}
+
+
+/*
+ * Rotate log file
+ *
+ * This function is kept to support adminpack 1.0.
+ */
+Datum
+pg_rotate_logfile(PG_FUNCTION_ARGS)
+{
+ if (!superuser())
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("must be superuser to rotate log files with adminpack 1.0"),
+ errhint("Consider using pg_logfile_rotate(), which is part of core, instead."))));
+
+ if (!Logging_collector)
+ {
+ ereport(WARNING,
+ (errmsg("rotation not possible because log collection not active")));
+ PG_RETURN_BOOL(false);
+ }
+
+ SendPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE);
+ PG_RETURN_BOOL(true);
+}
+
+/*
+ * Rotate log file
+ *
+ * Permission checking for this function is managed through the normal
+ * GRANT system.
+ */
+Datum
+pg_rotate_logfile_v2(PG_FUNCTION_ARGS)
+{
+ if (!Logging_collector)
+ {
+ ereport(WARNING,
+ (errmsg("rotation not possible because log collection not active")));
+ PG_RETURN_BOOL(false);
+ }
+
+ SendPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE);
+ PG_RETURN_BOOL(true);
+}
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index b24dece23f..6ea3679835 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -15,13 +15,11 @@
#include "postgres.h"
#include <sys/file.h>
-#include <signal.h>
#include <dirent.h>
#include <math.h>
#include <unistd.h>
#include "access/sysattr.h"
-#include "catalog/pg_authid.h"
#include "catalog/catalog.h"
#include "catalog/pg_tablespace.h"
#include "catalog/pg_type.h"
@@ -35,13 +33,9 @@
#include "postmaster/syslogger.h"
#include "rewrite/rewriteHandler.h"
#include "storage/fd.h"
-#include "storage/pmsignal.h"
-#include "storage/proc.h"
-#include "storage/procarray.h"
#include "utils/lsyscache.h"
#include "utils/ruleutils.h"
#include "tcop/tcopprot.h"
-#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/timestamp.h"
@@ -198,194 +192,6 @@ current_query(PG_FUNCTION_ARGS)
PG_RETURN_NULL();
}
-/*
- * Send a signal to another backend.
- *
- * The signal is delivered if the user is either a superuser or the same
- * role as the backend being signaled. For "dangerous" signals, an explicit
- * check for superuser needs to be done prior to calling this function.
- *
- * Returns 0 on success, 1 on general failure, 2 on normal permission error
- * and 3 if the caller needs to be a superuser.
- *
- * In the event of a general failure (return code 1), a warning message will
- * be emitted. For permission errors, doing that is the responsibility of
- * the caller.
- */
-#define SIGNAL_BACKEND_SUCCESS 0
-#define SIGNAL_BACKEND_ERROR 1
-#define SIGNAL_BACKEND_NOPERMISSION 2
-#define SIGNAL_BACKEND_NOSUPERUSER 3
-static int
-pg_signal_backend(int pid, int sig)
-{
- PGPROC *proc = BackendPidGetProc(pid);
-
- /*
- * BackendPidGetProc returns NULL if the pid isn't valid; but by the time
- * we reach kill(), a process for which we get a valid proc here might
- * have terminated on its own. There's no way to acquire a lock on an
- * arbitrary process to prevent that. But since so far all the callers of
- * this mechanism involve some request for ending the process anyway, that
- * it might end on its own first is not a problem.
- */
- if (proc == NULL)
- {
- /*
- * This is just a warning so a loop-through-resultset will not abort
- * if one backend terminated on its own during the run.
- */
- ereport(WARNING,
- (errmsg("PID %d is not a PostgreSQL server process", pid)));
- return SIGNAL_BACKEND_ERROR;
- }
-
- /* Only allow superusers to signal superuser-owned backends. */
- if (superuser_arg(proc->roleId) && !superuser())
- return SIGNAL_BACKEND_NOSUPERUSER;
-
- /* Users can signal backends they have role membership in. */
- if (!has_privs_of_role(GetUserId(), proc->roleId) &&
- !has_privs_of_role(GetUserId(), DEFAULT_ROLE_SIGNAL_BACKENDID))
- return SIGNAL_BACKEND_NOPERMISSION;
-
- /*
- * Can the process we just validated above end, followed by the pid being
- * recycled for a new process, before reaching here? Then we'd be trying
- * to kill the wrong thing. Seems near impossible when sequential pid
- * assignment and wraparound is used. Perhaps it could happen on a system
- * where pid re-use is randomized. That race condition possibility seems
- * too unlikely to worry about.
- */
-
- /* If we have setsid(), signal the backend's whole process group */
-#ifdef HAVE_SETSID
- if (kill(-pid, sig))
-#else
- if (kill(pid, sig))
-#endif
- {
- /* Again, just a warning to allow loops */
- ereport(WARNING,
- (errmsg("could not send signal to process %d: %m", pid)));
- return SIGNAL_BACKEND_ERROR;
- }
- return SIGNAL_BACKEND_SUCCESS;
-}
-
-/*
- * Signal to cancel a backend process. This is allowed if you are a member of
- * the role whose process is being canceled.
- *
- * Note that only superusers can signal superuser-owned processes.
- */
-Datum
-pg_cancel_backend(PG_FUNCTION_ARGS)
-{
- int r = pg_signal_backend(PG_GETARG_INT32(0), SIGINT);
-
- if (r == SIGNAL_BACKEND_NOSUPERUSER)
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be a superuser to cancel superuser query"))));
-
- if (r == SIGNAL_BACKEND_NOPERMISSION)
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be a member of the role whose query is being canceled or member of pg_signal_backend"))));
-
- PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
-}
-
-/*
- * Signal to terminate a backend process. This is allowed if you are a member
- * of the role whose process is being terminated.
- *
- * Note that only superusers can signal superuser-owned processes.
- */
-Datum
-pg_terminate_backend(PG_FUNCTION_ARGS)
-{
- int r = pg_signal_backend(PG_GETARG_INT32(0), SIGTERM);
-
- if (r == SIGNAL_BACKEND_NOSUPERUSER)
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be a superuser to terminate superuser process"))));
-
- if (r == SIGNAL_BACKEND_NOPERMISSION)
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be a member of the role whose process is being terminated or member of pg_signal_backend"))));
-
- PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
-}
-
-/*
- * Signal to reload the database configuration
- *
- * Permission checking for this function is managed through the normal
- * GRANT system.
- */
-Datum
-pg_reload_conf(PG_FUNCTION_ARGS)
-{
- if (kill(PostmasterPid, SIGHUP))
- {
- ereport(WARNING,
- (errmsg("failed to send signal to postmaster: %m")));
- PG_RETURN_BOOL(false);
- }
-
- PG_RETURN_BOOL(true);
-}
-
-
-/*
- * Rotate log file
- *
- * This function is kept to support adminpack 1.0.
- */
-Datum
-pg_rotate_logfile(PG_FUNCTION_ARGS)
-{
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser to rotate log files with adminpack 1.0"),
- errhint("Consider using pg_logfile_rotate(), which is part of core, instead."))));
-
- if (!Logging_collector)
- {
- ereport(WARNING,
- (errmsg("rotation not possible because log collection not active")));
- PG_RETURN_BOOL(false);
- }
-
- SendPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE);
- PG_RETURN_BOOL(true);
-}
-
-/*
- * Rotate log file
- *
- * Permission checking for this function is managed through the normal
- * GRANT system.
- */
-Datum
-pg_rotate_logfile_v2(PG_FUNCTION_ARGS)
-{
- if (!Logging_collector)
- {
- ereport(WARNING,
- (errmsg("rotation not possible because log collection not active")));
- PG_RETURN_BOOL(false);
- }
-
- SendPostmasterSignal(PMSIGNAL_ROTATE_LOGFILE);
- PG_RETURN_BOOL(true);
-}
-
/* Function to find out which databases make use of a tablespace */
typedef struct
signature.asc
Description: PGP signature
