diff options
| -rw-r--r-- | src/backend/commands/async.c | 199 | ||||
| -rw-r--r-- | src/backend/libpq/be-secure-openssl.c | 28 | ||||
| -rw-r--r-- | src/backend/libpq/be-secure.c | 43 | ||||
| -rw-r--r-- | src/backend/postmaster/autovacuum.c | 6 | ||||
| -rw-r--r-- | src/backend/storage/ipc/sinval.c | 223 | ||||
| -rw-r--r-- | src/backend/tcop/postgres.c | 106 | ||||
| -rw-r--r-- | src/include/commands/async.h | 12 | ||||
| -rw-r--r-- | src/include/storage/sinval.h | 7 | ||||
| -rw-r--r-- | src/include/tcop/tcopprot.h | 4 |
9 files changed, 194 insertions, 434 deletions
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 7ece5279995..d73248ca504 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -79,10 +79,12 @@ * either, but just process the queue directly. * * 5. Upon receipt of a PROCSIG_NOTIFY_INTERRUPT signal, the signal handler - * can call inbound-notify processing immediately if this backend is idle - * (ie, it is waiting for a frontend command and is not within a transaction - * block). Otherwise the handler may only set a flag, which will cause the - * processing to occur just before we next go idle. + * sets the process's latch, which triggers the event to be processed + * immediately if this backend is idle (i.e., it is waiting for a frontend + * command and is not within a transaction block. C.f. + * ProcessClientReadInterrupt()). Otherwise the handler may only set a + * flag, which will cause the processing to occur just before we next go + * idle. * * Inbound-notify processing consists of reading all of the notifications * that have arrived since scanning last time. We read every notification @@ -126,6 +128,7 @@ #include "miscadmin.h" #include "storage/ipc.h" #include "storage/lmgr.h" +#include "storage/proc.h" #include "storage/procarray.h" #include "storage/procsignal.h" #include "storage/sinval.h" @@ -334,17 +337,13 @@ static List *pendingNotifies = NIL; /* list of Notifications */ static List *upperPendingNotifies = NIL; /* list of upper-xact lists */ /* - * State for inbound notifications consists of two flags: one saying whether - * the signal handler is currently allowed to call ProcessIncomingNotify - * directly, and one saying whether the signal has occurred but the handler - * was not allowed to call ProcessIncomingNotify at the time. - * - * NB: the "volatile" on these declarations is critical! If your compiler - * does not grok "volatile", you'd be best advised to compile this file - * with all optimization turned off. + * Inbound notifications are initially processed by HandleNotifyInterrupt(), + * called from inside a signal handler. That just sets the + * notifyInterruptPending flag and sets the process + * latch. ProcessNotifyInterrupt() will then be called whenever it's safe to + * actually deal with the interrupt. */ -static volatile sig_atomic_t notifyInterruptEnabled = 0; -static volatile sig_atomic_t notifyInterruptOccurred = 0; +volatile sig_atomic_t notifyInterruptPending = false; /* True if we've registered an on_shmem_exit cleanup */ static bool unlistenExitRegistered = false; @@ -1625,164 +1624,45 @@ AtSubAbort_Notify(void) /* * HandleNotifyInterrupt * - * This is called when PROCSIG_NOTIFY_INTERRUPT is received. - * - * If we are idle (notifyInterruptEnabled is set), we can safely invoke - * ProcessIncomingNotify directly. Otherwise, just set a flag - * to do it later. + * Signal handler portion of interrupt handling. Let the backend know + * that there's a pending notify interrupt. If we're currently reading + * from the client, this will interrupt the read and + * ProcessClientReadInterrupt() will call ProcessNotifyInterrupt(). */ void HandleNotifyInterrupt(void) { /* * Note: this is called by a SIGNAL HANDLER. You must be very wary what - * you do here. Some helpful soul had this routine sprinkled with - * TPRINTFs, which would likely lead to corruption of stdio buffers if - * they were ever turned on. + * you do here. */ - /* Don't joggle the elbow of proc_exit */ - if (proc_exit_inprogress) - return; - - if (notifyInterruptEnabled) - { - bool save_ImmediateInterruptOK = ImmediateInterruptOK; - - /* - * We may be called while ImmediateInterruptOK is true; turn it off - * while messing with the NOTIFY state. This prevents problems if - * SIGINT or similar arrives while we're working. Just to be real - * sure, bump the interrupt holdoff counter as well. That way, even - * if something inside ProcessIncomingNotify() transiently sets - * ImmediateInterruptOK (eg while waiting on a lock), we won't get - * interrupted until we're done with the notify interrupt. - */ - ImmediateInterruptOK = false; - HOLD_INTERRUPTS(); - - /* - * I'm not sure whether some flavors of Unix might allow another - * SIGUSR1 occurrence to recursively interrupt this routine. To cope - * with the possibility, we do the same sort of dance that - * EnableNotifyInterrupt must do --- see that routine for comments. - */ - notifyInterruptEnabled = 0; /* disable any recursive signal */ - notifyInterruptOccurred = 1; /* do at least one iteration */ - for (;;) - { - notifyInterruptEnabled = 1; - if (!notifyInterruptOccurred) - break; - notifyInterruptEnabled = 0; - if (notifyInterruptOccurred) - { - /* Here, it is finally safe to do stuff. */ - if (Trace_notify) - elog(DEBUG1, "HandleNotifyInterrupt: perform async notify"); - - ProcessIncomingNotify(); - - if (Trace_notify) - elog(DEBUG1, "HandleNotifyInterrupt: done"); - } - } + /* signal that work needs to be done */ + notifyInterruptPending = true; - /* - * Restore the holdoff level and ImmediateInterruptOK, and check for - * interrupts if needed. - */ - RESUME_INTERRUPTS(); - ImmediateInterruptOK = save_ImmediateInterruptOK; - if (save_ImmediateInterruptOK) - CHECK_FOR_INTERRUPTS(); - } - else - { - /* - * In this path it is NOT SAFE to do much of anything, except this: - */ - notifyInterruptOccurred = 1; - } + /* make sure the event is processed in due course */ + SetLatch(MyLatch); } /* - * EnableNotifyInterrupt - * - * This is called by the PostgresMain main loop just before waiting - * for a frontend command. If we are truly idle (ie, *not* inside - * a transaction block), then process any pending inbound notifies, - * and enable the signal handler to process future notifies directly. + * ProcessNotifyInterrupt * - * NOTE: the signal handler starts out disabled, and stays so until - * PostgresMain calls this the first time. + * This is called just after waiting for a frontend command. If a + * interrupt arrives (via HandleNotifyInterrupt()) while reading, the + * read will be interrupted via the process's latch, and this routine + * will get called. If we are truly idle (ie, *not* inside a transaction + * block), process the incoming notifies. */ void -EnableNotifyInterrupt(void) +ProcessNotifyInterrupt(void) { if (IsTransactionOrTransactionBlock()) return; /* not really idle */ - /* - * This code is tricky because we are communicating with a signal handler - * that could interrupt us at any point. If we just checked - * notifyInterruptOccurred and then set notifyInterruptEnabled, we could - * fail to respond promptly to a signal that happens in between those two - * steps. (A very small time window, perhaps, but Murphy's Law says you - * can hit it...) Instead, we first set the enable flag, then test the - * occurred flag. If we see an unserviced interrupt has occurred, we - * re-clear the enable flag before going off to do the service work. (That - * prevents re-entrant invocation of ProcessIncomingNotify() if another - * interrupt occurs.) If an interrupt comes in between the setting and - * clearing of notifyInterruptEnabled, then it will have done the service - * work and left notifyInterruptOccurred zero, so we have to check again - * after clearing enable. The whole thing has to be in a loop in case - * another interrupt occurs while we're servicing the first. Once we get - * out of the loop, enable is set and we know there is no unserviced - * interrupt. - * - * NB: an overenthusiastic optimizing compiler could easily break this - * code. Hopefully, they all understand what "volatile" means these days. - */ - for (;;) - { - notifyInterruptEnabled = 1; - if (!notifyInterruptOccurred) - break; - notifyInterruptEnabled = 0; - if (notifyInterruptOccurred) - { - if (Trace_notify) - elog(DEBUG1, "EnableNotifyInterrupt: perform async notify"); - - ProcessIncomingNotify(); - - if (Trace_notify) - elog(DEBUG1, "EnableNotifyInterrupt: done"); - } - } + while (notifyInterruptPending) + ProcessIncomingNotify(); } -/* - * DisableNotifyInterrupt - * - * This is called by the PostgresMain main loop just after receiving - * a frontend command. Signal handler execution of inbound notifies - * is disabled until the next EnableNotifyInterrupt call. - * - * The PROCSIG_CATCHUP_INTERRUPT signal handler also needs to call this, - * so as to prevent conflicts if one signal interrupts the other. So we - * must return the previous state of the flag. - */ -bool -DisableNotifyInterrupt(void) -{ - bool result = (notifyInterruptEnabled != 0); - - notifyInterruptEnabled = 0; - - return result; -} /* * Read all pending notifications from the queue, and deliver appropriate @@ -2076,9 +1956,10 @@ asyncQueueAdvanceTail(void) /* * ProcessIncomingNotify * - * Deal with arriving NOTIFYs from other backends. - * This is called either directly from the PROCSIG_NOTIFY_INTERRUPT - * signal handler, or the next time control reaches the outer idle loop. + * Deal with arriving NOTIFYs from other backends as soon as it's safe to + * do so. This used to be called from the PROCSIG_NOTIFY_INTERRUPT + * signal handler, but isn't anymore. + * * Scan the queue for arriving notifications and report them to my front * end. * @@ -2087,18 +1968,13 @@ asyncQueueAdvanceTail(void) static void ProcessIncomingNotify(void) { - bool catchup_enabled; - /* We *must* reset the flag */ - notifyInterruptOccurred = 0; + notifyInterruptPending = false; /* Do nothing else if we aren't actively listening */ if (listenChannels == NIL) return; - /* Must prevent catchup interrupt while I am running */ - catchup_enabled = DisableCatchupInterrupt(); - if (Trace_notify) elog(DEBUG1, "ProcessIncomingNotify"); @@ -2123,9 +1999,6 @@ ProcessIncomingNotify(void) if (Trace_notify) elog(DEBUG1, "ProcessIncomingNotify: done"); - - if (catchup_enabled) - EnableCatchupInterrupt(); } /* diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 20b4742d12e..3db358e402f 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -511,6 +511,7 @@ be_tls_read(Port *port, void *ptr, size_t len) ssize_t n; int err; int waitfor; + int latchret; rloop: errno = 0; @@ -531,12 +532,29 @@ rloop: break; } + waitfor = WL_LATCH_SET; + if (err == SSL_ERROR_WANT_READ) - waitfor = WL_SOCKET_READABLE; + waitfor |= WL_SOCKET_READABLE; else - waitfor = WL_SOCKET_WRITEABLE; + waitfor |= WL_SOCKET_WRITEABLE; - WaitLatchOrSocket(MyLatch, waitfor, port->sock, 0); + latchret = WaitLatchOrSocket(MyLatch, waitfor, port->sock, 0); + + /* + * We'll, among other situations, get here if the low level + * routine doing the actual recv() via the socket got interrupted + * by a signal. That's so we can handle interrupts once outside + * openssl, so we don't jump out from underneath its covers. We + * can check this both, when reading and writing, because even + * when writing that's just openssl's doing, not a 'proper' write + * initiated by postgres. + */ + if (latchret & WL_LATCH_SET) + { + ResetLatch(MyLatch); + ProcessClientReadInterrupt(); /* preserves errno */ + } goto rloop; case SSL_ERROR_SYSCALL: /* leave it to caller to ereport the value of errno */ @@ -647,6 +665,10 @@ wloop: waitfor = WL_SOCKET_WRITEABLE; WaitLatchOrSocket(MyLatch, waitfor, port->sock, 0); + /* + * XXX: We'll, at some later point, likely want to add interrupt + * processing here. + */ goto wloop; case SSL_ERROR_SYSCALL: /* leave it to caller to ereport the value of errno */ diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index c9a8f6df4fe..b90ab0ea86f 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -128,6 +128,7 @@ secure_read(Port *port, void *ptr, size_t len) { ssize_t n; +retry: #ifdef USE_SSL if (port->ssl_in_use) { @@ -139,6 +140,14 @@ secure_read(Port *port, void *ptr, size_t len) n = secure_raw_read(port, ptr, len); } + /* Process interrupts that happened while (or before) receiving. */ + ProcessClientReadInterrupt(); /* preserves errno */ + + /* retry after processing interrupts */ + if (n < 0 && errno == EINTR) + { + goto retry; + } return n; } @@ -147,8 +156,6 @@ secure_raw_read(Port *port, void *ptr, size_t len) { ssize_t n; - prepare_for_client_read(); - /* * Try to read from the socket without blocking. If it succeeds we're * done, otherwise we'll wait for the socket using the latch mechanism. @@ -168,10 +175,20 @@ rloop: int save_errno = errno; w = WaitLatchOrSocket(MyLatch, - WL_SOCKET_READABLE, + WL_LATCH_SET | WL_SOCKET_READABLE, port->sock, 0); - if (w & WL_SOCKET_READABLE) + if (w & WL_LATCH_SET) + { + ResetLatch(MyLatch); + /* + * Force a return, so interrupts can be processed when not + * (possibly) underneath a ssl library. + */ + errno = EINTR; + return -1; + } + else if (w & WL_SOCKET_READABLE) { goto rloop; } @@ -183,8 +200,6 @@ rloop: errno = save_errno; } - client_read_ended(); - return n; } @@ -197,6 +212,7 @@ secure_write(Port *port, void *ptr, size_t len) { ssize_t n; +retry: #ifdef USE_SSL if (port->ssl_in_use) { @@ -208,6 +224,21 @@ secure_write(Port *port, void *ptr, size_t len) n = secure_raw_write(port, ptr, len); } + /* + * XXX: We'll, at some later point, likely want to add interrupt + * processing here. + */ + + /* + * Retry after processing interrupts. This can be triggered even though we + * don't check for latch set's during writing yet, because SSL + * renegotiations might have required reading from the socket. + */ + if (n < 0 && errno == EINTR) + { + goto retry; + } + return n; } diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 6492067d077..77158c1e8f0 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -582,9 +582,6 @@ AutoVacLauncherMain(int argc, char *argv[]) launcher_determine_sleep(!dlist_is_empty(&AutoVacuumShmem->av_freeWorkers), false, &nap); - /* Allow sinval catchup interrupts while sleeping */ - EnableCatchupInterrupt(); - /* * Wait until naptime expires or we get some type of signal (all the * signal handlers will wake us by calling SetLatch). @@ -595,7 +592,8 @@ AutoVacLauncherMain(int argc, char *argv[]) ResetLatch(MyLatch); - DisableCatchupInterrupt(); + /* Process sinval catchup interrupts that happened while sleeping */ + ProcessCatchupInterrupt(); /* * Emergency bailout if postmaster has died. This is to avoid the diff --git a/src/backend/storage/ipc/sinval.c b/src/backend/storage/ipc/sinval.c index e6b0d498c14..67ec5152a84 100644 --- a/src/backend/storage/ipc/sinval.c +++ b/src/backend/storage/ipc/sinval.c @@ -18,6 +18,7 @@ #include "commands/async.h" #include "miscadmin.h" #include "storage/ipc.h" +#include "storage/proc.h" #include "storage/sinvaladt.h" #include "utils/inval.h" @@ -32,19 +33,12 @@ uint64 SharedInvalidMessageCounter; * through a cache reset exercise. This is done by sending * PROCSIG_CATCHUP_INTERRUPT to any backend that gets too far behind. * - * State for catchup events consists of two flags: one saying whether - * the signal handler is currently allowed to call ProcessCatchupEvent - * directly, and one saying whether the signal has occurred but the handler - * was not allowed to call ProcessCatchupEvent at the time. - * - * NB: the "volatile" on these declarations is critical! If your compiler - * does not grok "volatile", you'd be best advised to compile this file - * with all optimization turned off. + * The signal handler will set a interrupt pending flag and will set the + * processes latch. Whenever starting to read from the client, or when + * interrupted while doing so, ProcessClientReadInterrupt() will call + * ProcessCatchupEvent(). */ -static volatile int catchupInterruptEnabled = 0; -static volatile int catchupInterruptOccurred = 0; - -static void ProcessCatchupEvent(void); +volatile sig_atomic_t catchupInterruptPending = false; /* @@ -141,9 +135,9 @@ ReceiveSharedInvalidMessages( * catchup signal this way avoids creating spikes in system load for what * should be just a background maintenance activity. */ - if (catchupInterruptOccurred) + if (catchupInterruptPending) { - catchupInterruptOccurred = 0; + catchupInterruptPending = false; elog(DEBUG4, "sinval catchup complete, cleaning queue"); SICleanupQueue(false, 0); } @@ -155,12 +149,9 @@ ReceiveSharedInvalidMessages( * * This is called when PROCSIG_CATCHUP_INTERRUPT is received. * - * If we are idle (catchupInterruptEnabled is set), we can safely - * invoke ProcessCatchupEvent directly. Otherwise, just set a flag - * to do it later. (Note that it's quite possible for normal processing - * of the current transaction to cause ReceiveSharedInvalidMessages() - * to be run later on; in that case the flag will get cleared again, - * since there's no longer any reason to do anything.) + * We used to directly call ProcessCatchupEvent directly when idle. These days + * we just set a flag to do it later and notify the process of that fact by + * setting the process's latch. */ void HandleCatchupInterrupt(void) @@ -170,174 +161,46 @@ HandleCatchupInterrupt(void) * you do here. */ - /* Don't joggle the elbow of proc_exit */ - if (proc_exit_inprogress) - return; - - if (catchupInterruptEnabled) - { - bool save_ImmediateInterruptOK = ImmediateInterruptOK; - - /* - * We may be called while ImmediateInterruptOK is true; turn it off - * while messing with the catchup state. This prevents problems if - * SIGINT or similar arrives while we're working. Just to be real - * sure, bump the interrupt holdoff counter as well. That way, even - * if something inside ProcessCatchupEvent() transiently sets - * ImmediateInterruptOK (eg while waiting on a lock), we won't get - * interrupted until we're done with the catchup interrupt. - */ - ImmediateInterruptOK = false; - HOLD_INTERRUPTS(); - - /* - * I'm not sure whether some flavors of Unix might allow another - * SIGUSR1 occurrence to recursively interrupt this routine. To cope - * with the possibility, we do the same sort of dance that - * EnableCatchupInterrupt must do --- see that routine for comments. - */ - catchupInterruptEnabled = 0; /* disable any recursive signal */ - catchupInterruptOccurred = 1; /* do at least one iteration */ - for (;;) - { - catchupInterruptEnabled = 1; - if (!catchupInterruptOccurred) - break; - catchupInterruptEnabled = 0; - if (catchupInterruptOccurred) - { - /* Here, it is finally safe to do stuff. */ - ProcessCatchupEvent(); - } - } + catchupInterruptPending = true; - /* - * Restore the holdoff level and ImmediateInterruptOK, and check for - * interrupts if needed. - */ - RESUME_INTERRUPTS(); - ImmediateInterruptOK = save_ImmediateInterruptOK; - if (save_ImmediateInterruptOK) - CHECK_FOR_INTERRUPTS(); - } - else - { - /* - * In this path it is NOT SAFE to do much of anything, except this: - */ - catchupInterruptOccurred = 1; - } + /* make sure the event is processed in due course */ + SetLatch(MyLatch); } /* - * EnableCatchupInterrupt - * - * This is called by the PostgresMain main loop just before waiting - * for a frontend command. We process any pending catchup events, - * and enable the signal handler to process future events directly. + * ProcessCatchupInterrupt * - * NOTE: the signal handler starts out disabled, and stays so until - * PostgresMain calls this the first time. + * The portion of catchup interrupt handling that runs outside of the signal + * handler, which allows it to actually process pending invalidations. */ void -EnableCatchupInterrupt(void) +ProcessCatchupInterrupt(void) { - /* - * This code is tricky because we are communicating with a signal handler - * that could interrupt us at any point. If we just checked - * catchupInterruptOccurred and then set catchupInterruptEnabled, we could - * fail to respond promptly to a signal that happens in between those two - * steps. (A very small time window, perhaps, but Murphy's Law says you - * can hit it...) Instead, we first set the enable flag, then test the - * occurred flag. If we see an unserviced interrupt has occurred, we - * re-clear the enable flag before going off to do the service work. (That - * prevents re-entrant invocation of ProcessCatchupEvent() if another - * interrupt occurs.) If an interrupt comes in between the setting and - * clearing of catchupInterruptEnabled, then it will have done the service - * work and left catchupInterruptOccurred zero, so we have to check again - * after clearing enable. The whole thing has to be in a loop in case - * another interrupt occurs while we're servicing the first. Once we get - * out of the loop, enable is set and we know there is no unserviced - * interrupt. - * - * NB: an overenthusiastic optimizing compiler could easily break this - * code. Hopefully, they all understand what "volatile" means these days. - */ - for (;;) + while (catchupInterruptPending) { - catchupInterruptEnabled = 1; - if (!catchupInterruptOccurred) - break; - catchupInterruptEnabled = 0; - if (catchupInterruptOccurred) - ProcessCatchupEvent(); - } -} - -/* - * DisableCatchupInterrupt - * - * This is called by the PostgresMain main loop just after receiving - * a frontend command. Signal handler execution of catchup events - * is disabled until the next EnableCatchupInterrupt call. - * - * The PROCSIG_NOTIFY_INTERRUPT signal handler also needs to call this, - * so as to prevent conflicts if one signal interrupts the other. So we - * must return the previous state of the flag. - */ -bool -DisableCatchupInterrupt(void) -{ - bool result = (catchupInterruptEnabled != 0); - - catchupInterruptEnabled = 0; - - return result; -} - -/* - * ProcessCatchupEvent - * - * Respond to a catchup event (PROCSIG_CATCHUP_INTERRUPT) from another - * backend. - * - * This is called either directly from the PROCSIG_CATCHUP_INTERRUPT - * signal handler, or the next time control reaches the outer idle loop - * (assuming there's still anything to do by then). - */ -static void -ProcessCatchupEvent(void) -{ - bool notify_enabled; - - /* Must prevent notify interrupt while I am running */ - notify_enabled = DisableNotifyInterrupt(); - - /* - * What we need to do here is cause ReceiveSharedInvalidMessages() to run, - * which will do the necessary work and also reset the - * catchupInterruptOccurred flag. If we are inside a transaction we can - * just call AcceptInvalidationMessages() to do this. If we aren't, we - * start and immediately end a transaction; the call to - * AcceptInvalidationMessages() happens down inside transaction start. - * - * It is awfully tempting to just call AcceptInvalidationMessages() - * without the rest of the xact start/stop overhead, and I think that - * would actually work in the normal case; but I am not sure that things - * would clean up nicely if we got an error partway through. - */ - if (IsTransactionOrTransactionBlock()) - { - elog(DEBUG4, "ProcessCatchupEvent inside transaction"); - AcceptInvalidationMessages(); - } - else - { - elog(DEBUG4, "ProcessCatchupEvent outside transaction"); - StartTransactionCommand(); - CommitTransactionCommand(); + /* + * What we need to do here is cause ReceiveSharedInvalidMessages() to + * run, which will do the necessary work and also reset the + * catchupInterruptPending flag. If we are inside a transaction we + * can just call AcceptInvalidationMessages() to do this. If we + * aren't, we start and immediately end a transaction; the call to + * AcceptInvalidationMessages() happens down inside transaction start. + * + * It is awfully tempting to just call AcceptInvalidationMessages() + * without the rest of the xact start/stop overhead, and I think that + * would actually work in the normal case; but I am not sure that things + * would clean up nicely if we got an error partway through. + */ + if (IsTransactionOrTransactionBlock()) + { + elog(DEBUG4, "ProcessCatchupEvent inside transaction"); + AcceptInvalidationMessages(); + } + else + { + elog(DEBUG4, "ProcessCatchupEvent outside transaction"); + StartTransactionCommand(); + CommitTransactionCommand(); + } } - - if (notify_enabled) - EnableNotifyInterrupt(); } diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 556e5633284..bcc4f243134 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -301,17 +301,25 @@ InteractiveBackend(StringInfo inBuf) * interactive_getc -- collect one character from stdin * * Even though we are not reading from a "client" process, we still want to - * respond to signals, particularly SIGTERM/SIGQUIT. Hence we must use - * prepare_for_client_read and client_read_ended. + * respond to signals, particularly SIGTERM/SIGQUIT. */ static int interactive_getc(void) { int c; - prepare_for_client_read(); + /* + * This will not process catchup interrupts or notifications while + * reading. But those can't really be relevant for a standalone backend + * anyway. To properly handle SIGTERM there's a hack in die() that + * directly processes interrupts at this stage... + */ + CHECK_FOR_INTERRUPTS(); + c = getc(stdin); - client_read_ended(); + + ProcessClientReadInterrupt(); + return c; } @@ -513,53 +521,33 @@ ReadCommand(StringInfo inBuf) } /* - * prepare_for_client_read -- set up to possibly block on client input + * ProcessClientReadInterrupt() - Process interrupts specific to client reads * - * This must be called immediately before any low-level read from the - * client connection. It is necessary to do it at a sufficiently low level - * that there won't be any other operations except the read kernel call - * itself between this call and the subsequent client_read_ended() call. - * In particular there mustn't be use of malloc() or other potentially - * non-reentrant libc functions. This restriction makes it safe for us - * to allow interrupt service routines to execute nontrivial code while - * we are waiting for input. - */ -void -prepare_for_client_read(void) -{ - if (DoingCommandRead) - { - /* Enable immediate processing of asynchronous signals */ - EnableNotifyInterrupt(); - EnableCatchupInterrupt(); - - /* Allow die interrupts to be processed while waiting */ - ImmediateInterruptOK = true; - - /* And don't forget to detect one that already arrived */ - CHECK_FOR_INTERRUPTS(); - } -} - -/* - * client_read_ended -- get out of the client-input state + * This is called just after low-level reads. That might be after the read + * finished successfully, or it was interrupted via interrupt. * - * This is called just after low-level reads. It must preserve errno! + * Must preserve errno! */ void -client_read_ended(void) +ProcessClientReadInterrupt(void) { + int save_errno = errno; + if (DoingCommandRead) { - int save_errno = errno; - - ImmediateInterruptOK = false; + /* Check for general interrupts that arrived while reading */ + CHECK_FOR_INTERRUPTS(); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); + /* Process sinval catchup interrupts that happened while reading */ + if (catchupInterruptPending) + ProcessCatchupInterrupt(); - errno = save_errno; + /* Process sinval catchup interrupts that happened while reading */ + if (notifyInterruptPending) + ProcessNotifyInterrupt(); } + + errno = save_errno; } @@ -2626,6 +2614,15 @@ die(SIGNAL_ARGS) /* If we're still here, waken anything waiting on the process latch */ SetLatch(MyLatch); + /* + * If we're in single user mode, we want to quit immediately - we can't + * rely on latches as they wouldn't work when stdin/stdout is a + * file. Rather ugly, but it's unlikely to be worthwhile to invest much + * more effort just for the benefit of single user mode. + */ + if (DoingCommandRead && whereToSendOutput != DestRemote) + ProcessInterrupts(); + errno = save_errno; } @@ -2834,8 +2831,6 @@ ProcessInterrupts(void) QueryCancelPending = false; /* ProcDie trumps QueryCancel */ ImmediateInterruptOK = false; /* not idle anymore */ LockErrorCleanup(); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); /* As in quickdie, don't risk sending to client during auth */ if (ClientAuthInProgress && whereToSendOutput == DestRemote) whereToSendOutput = DestNone; @@ -2871,8 +2866,6 @@ ProcessInterrupts(void) QueryCancelPending = false; /* lost connection trumps QueryCancel */ ImmediateInterruptOK = false; /* not idle anymore */ LockErrorCleanup(); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); /* don't send to client, we already know the connection to be dead. */ whereToSendOutput = DestNone; ereport(FATAL, @@ -2892,8 +2885,6 @@ ProcessInterrupts(void) ImmediateInterruptOK = false; /* not idle anymore */ RecoveryConflictPending = false; LockErrorCleanup(); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); pgstat_report_recovery_conflict(RecoveryConflictReason); ereport(FATAL, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), @@ -2926,8 +2917,6 @@ ProcessInterrupts(void) { ImmediateInterruptOK = false; /* not idle anymore */ LockErrorCleanup(); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); /* As in quickdie, don't risk sending to client during auth */ if (whereToSendOutput == DestRemote) whereToSendOutput = DestNone; @@ -2945,8 +2934,6 @@ ProcessInterrupts(void) ImmediateInterruptOK = false; /* not idle anymore */ (void) get_timeout_indicator(STATEMENT_TIMEOUT, true); LockErrorCleanup(); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); ereport(ERROR, (errcode(ERRCODE_LOCK_NOT_AVAILABLE), errmsg("canceling statement due to lock timeout"))); @@ -2955,8 +2942,6 @@ ProcessInterrupts(void) { ImmediateInterruptOK = false; /* not idle anymore */ LockErrorCleanup(); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); ereport(ERROR, (errcode(ERRCODE_QUERY_CANCELED), errmsg("canceling statement due to statement timeout"))); @@ -2965,8 +2950,6 @@ ProcessInterrupts(void) { ImmediateInterruptOK = false; /* not idle anymore */ LockErrorCleanup(); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); ereport(ERROR, (errcode(ERRCODE_QUERY_CANCELED), errmsg("canceling autovacuum task"))); @@ -2976,8 +2959,6 @@ ProcessInterrupts(void) ImmediateInterruptOK = false; /* not idle anymore */ RecoveryConflictPending = false; LockErrorCleanup(); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); pgstat_report_recovery_conflict(RecoveryConflictReason); ereport(ERROR, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), @@ -2994,13 +2975,12 @@ ProcessInterrupts(void) { ImmediateInterruptOK = false; /* not idle anymore */ LockErrorCleanup(); - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); ereport(ERROR, (errcode(ERRCODE_QUERY_CANCELED), errmsg("canceling statement due to user request"))); } } + /* If we get here, do nothing (probably, QueryCancelPending was reset) */ } @@ -3843,14 +3823,8 @@ PostgresMain(int argc, char *argv[], disable_all_timeouts(false); QueryCancelPending = false; /* second to avoid race condition */ - /* - * Turn off these interrupts too. This is only needed here and not in - * other exception-catching places since these interrupts are only - * enabled while we wait for client input. - */ + /* Not reading from the client anymore. */ DoingCommandRead = false; - DisableNotifyInterrupt(); - DisableCatchupInterrupt(); /* Make sure libpq is in a good state */ pq_comm_reset(); diff --git a/src/include/commands/async.h b/src/include/commands/async.h index 87c3abbfee7..8491f4736f7 100644 --- a/src/include/commands/async.h +++ b/src/include/commands/async.h @@ -13,6 +13,8 @@ #ifndef ASYNC_H #define ASYNC_H +#include <signal.h> + #include "fmgr.h" /* @@ -21,6 +23,7 @@ #define NUM_ASYNC_BUFFERS 8 extern bool Trace_notify; +extern volatile sig_atomic_t notifyInterruptPending; extern Size AsyncShmemSize(void); extern void AsyncShmemInit(void); @@ -48,12 +51,7 @@ extern void ProcessCompletedNotifies(void); /* signal handler for inbound notifies (PROCSIG_NOTIFY_INTERRUPT) */ extern void HandleNotifyInterrupt(void); -/* - * enable/disable processing of inbound notifies directly from signal handler. - * The enable routine first performs processing of any inbound notifies that - * have occurred since the last disable. - */ -extern void EnableNotifyInterrupt(void); -extern bool DisableNotifyInterrupt(void); +/* process interrupts */ +extern void ProcessNotifyInterrupt(void); #endif /* ASYNC_H */ diff --git a/src/include/storage/sinval.h b/src/include/storage/sinval.h index 1a6f2dfd38b..d9ffd7228fe 100644 --- a/src/include/storage/sinval.h +++ b/src/include/storage/sinval.h @@ -14,8 +14,9 @@ #ifndef SINVAL_H #define SINVAL_H -#include "storage/relfilenode.h" +#include <signal.h> +#include "storage/relfilenode.h" /* * We support several types of shared-invalidation messages: @@ -123,6 +124,7 @@ typedef union /* Counter of messages processed; don't worry about overflow. */ extern uint64 SharedInvalidMessageCounter; +extern volatile sig_atomic_t catchupInterruptPending; extern void SendSharedInvalidMessages(const SharedInvalidationMessage *msgs, int n); @@ -138,8 +140,7 @@ extern void HandleCatchupInterrupt(void); * The enable routine first performs processing of any catchup events that * have occurred since the last disable. */ -extern void EnableCatchupInterrupt(void); -extern bool DisableCatchupInterrupt(void); +extern void ProcessCatchupInterrupt(void); extern int xactGetCommittedInvalidationMessages(SharedInvalidationMessage **msgs, bool *RelcacheInitFileInval); diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h index 0a350fda2a9..fe8c725c7fe 100644 --- a/src/include/tcop/tcopprot.h +++ b/src/include/tcop/tcopprot.h @@ -67,8 +67,8 @@ extern void StatementCancelHandler(SIGNAL_ARGS); extern void FloatExceptionHandler(SIGNAL_ARGS) __attribute__((noreturn)); extern void RecoveryConflictInterrupt(ProcSignalReason reason); /* called from SIGUSR1 * handler */ -extern void prepare_for_client_read(void); -extern void client_read_ended(void); +extern void ProcessClientReadInterrupt(void); + extern void process_postgres_switches(int argc, char *argv[], GucContext ctx, const char **dbname); extern void PostgresMain(int argc, char *argv[], |