diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index f7f6618bc21143c1474eafe4a6c0ba44de29ad17..2646555f141bf63a242d13651b344695d0a3e71c 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -353,7 +353,6 @@ be_tls_open_server(Port *port) (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("could not initialize SSL connection: %s", SSLerrmessage()))); - be_tls_close(port); return -1; } if (!my_SSL_set_fd(port, port->sock)) @@ -362,7 +361,6 @@ be_tls_open_server(Port *port) (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("could not set SSL socket: %s", SSLerrmessage()))); - be_tls_close(port); return -1; } port->ssl_in_use = true; @@ -419,7 +417,6 @@ aloop: err))); break; } - be_tls_close(port); return -1; } @@ -449,7 +446,6 @@ aloop: { /* shouldn't happen */ pfree(peer_cn); - be_tls_close(port); return -1; } @@ -463,7 +459,6 @@ aloop: (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("SSL certificate's common name contains embedded null"))); pfree(peer_cn); - be_tls_close(port); return -1; } diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c index fcdbfcea07b25ec9920ac14ada581c17e18d055f..6667cf94c64a5517528118e85548bdc29a95ff8a 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -220,32 +220,45 @@ socket_comm_reset(void) /* -------------------------------- * socket_close - shutdown libpq at backend exit * - * Note: in a standalone backend MyProcPort will be null, - * don't crash during exit... + * This is the one pg_on_exit_callback in place during BackendInitialize(). + * That function's unusual signal handling constrains that this callback be + * safe to run at any instant. * -------------------------------- */ static void socket_close(int code, Datum arg) { + /* Nothing to do in a standalone backend, where MyProcPort is NULL. */ if (MyProcPort != NULL) { #if defined(ENABLE_GSS) || defined(ENABLE_SSPI) #ifdef ENABLE_GSS OM_uint32 min_s; - /* Shutdown GSSAPI layer */ + /* + * Shutdown GSSAPI layer. This section does nothing when interrupting + * BackendInitialize(), because pg_GSS_recvauth() makes first use of + * "ctx" and "cred". + */ if (MyProcPort->gss->ctx != GSS_C_NO_CONTEXT) gss_delete_sec_context(&min_s, &MyProcPort->gss->ctx, NULL); if (MyProcPort->gss->cred != GSS_C_NO_CREDENTIAL) gss_release_cred(&min_s, &MyProcPort->gss->cred); #endif /* ENABLE_GSS */ - /* GSS and SSPI share the port->gss struct */ + /* + * GSS and SSPI share the port->gss struct. Since nowhere else does a + * postmaster child free this, doing so is safe when interrupting + * BackendInitialize(). + */ free(MyProcPort->gss); #endif /* ENABLE_GSS || ENABLE_SSPI */ - /* Cleanly shut down SSL layer */ + /* + * Cleanly shut down SSL layer. Nowhere else does a postmaster child + * call this, so this is safe when interrupting BackendInitialize(). + */ secure_close(MyProcPort); /* diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 6e2ba08a93dad9f828a4714fb7055e59f1b72d58..87f543031ac05e34f33970b296ae6de74a6b450e 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -3960,7 +3960,16 @@ BackendInitialize(Port *port) * We arrange for a simple exit(1) if we receive SIGTERM or SIGQUIT or * timeout while trying to collect the startup packet. Otherwise the * postmaster cannot shutdown the database FAST or IMMED cleanly if a - * buggy client fails to send the packet promptly. + * buggy client fails to send the packet promptly. XXX it follows that + * the remainder of this function must tolerate losing control at any + * instant. Likewise, any pg_on_exit_callback registered before or during + * this function must be prepared to execute at any instant between here + * and the end of this function. Furthermore, affected callbacks execute + * partially or not at all when a second exit-inducing signal arrives + * after proc_exit_prepare() decrements on_proc_exit_index. (Thanks to + * that mechanic, callbacks need not anticipate more than one call.) This + * is fragile; it ought to instead follow the norm of handling interrupts + * at selected, safe opportunities. */ pqsignal(SIGTERM, startup_die); pqsignal(SIGQUIT, startup_die);