From 26a53d29bd32809e07687ef6515d48ef7d07f0b8 Mon Sep 17 00:00:00 2001 From: Alan Antonuk Date: Tue, 20 Oct 2015 23:44:00 -0700 Subject: Lib: don't try hard when closing SSL sockets If a heartbeat timeout occurs skip calling SSL_shutdown as it involves doing a send() which will likely hang. Additionally don't wait for a response when doing an SSL_shutdown, as the underlying transport will not be reused. Fixes #313 --- librabbitmq/amqp_openssl.c | 27 ++++++--------------------- librabbitmq/amqp_socket.c | 6 +++--- librabbitmq/amqp_socket.h | 11 +++++++++-- librabbitmq/amqp_tcp_socket.c | 4 ++-- 4 files changed, 20 insertions(+), 28 deletions(-) diff --git a/librabbitmq/amqp_openssl.c b/librabbitmq/amqp_openssl.c index 727f48c..651353b 100644 --- a/librabbitmq/amqp_openssl.c +++ b/librabbitmq/amqp_openssl.c @@ -348,34 +348,19 @@ error_out1: } static int -amqp_ssl_socket_close(void *base) +amqp_ssl_socket_close(void *base, amqp_socket_close_enum force) { - int res; struct amqp_ssl_socket_t *self = (struct amqp_ssl_socket_t *)base; if (-1 == self->sockfd) { return AMQP_STATUS_SOCKET_CLOSED; } -start_shutdown: - res = SSL_shutdown(self->ssl); - if (0 == res) { - goto start_shutdown; - } else if (-1 == res) { - self->internal_error = SSL_get_error(self->ssl, res); - switch (self->internal_error) { - case SSL_ERROR_WANT_READ: - res = amqp_poll(self->sockfd, AMQP_SF_POLLIN, amqp_time_infinite()); - break; - case SSL_ERROR_WANT_WRITE: - res = amqp_poll(self->sockfd, AMQP_SF_POLLOUT, amqp_time_infinite()); - break; - } - if (AMQP_STATUS_OK == res) { - goto start_shutdown; - } - /* Swallow errors in poll, just consider the connection dead */ + if (AMQP_SC_NONE == force) { + /* don't try too hard to shutdown the connection */ + SSL_shutdown(self->ssl); } + SSL_free(self->ssl); self->ssl = NULL; @@ -400,7 +385,7 @@ amqp_ssl_socket_delete(void *base) struct amqp_ssl_socket_t *self = (struct amqp_ssl_socket_t *)base; if (self) { - amqp_ssl_socket_close(self); + amqp_ssl_socket_close(self, AMQP_SC_NONE); SSL_CTX_free(self->ctx); free(self); diff --git a/librabbitmq/amqp_socket.c b/librabbitmq/amqp_socket.c index f709a66..c7286cc 100644 --- a/librabbitmq/amqp_socket.c +++ b/librabbitmq/amqp_socket.c @@ -234,11 +234,11 @@ amqp_socket_open_noblock(amqp_socket_t *self, const char *host, int port, struct } int -amqp_socket_close(amqp_socket_t *self) +amqp_socket_close(amqp_socket_t *self, amqp_socket_close_enum force) { assert(self); assert(self->klass->close); - return self->klass->close(self); + return self->klass->close(self, force); } void @@ -852,7 +852,7 @@ beginrecv: if (AMQP_STATUS_TIMEOUT == res) { if (amqp_time_equal(deadline, state->next_recv_heartbeat)) { - amqp_socket_close(state->socket); + amqp_socket_close(state->socket, AMQP_SC_FORCE); return AMQP_STATUS_HEARTBEAT_TIMEOUT; } else if (amqp_time_equal(deadline, timeout_deadline)) { return AMQP_STATUS_TIMEOUT; diff --git a/librabbitmq/amqp_socket.h b/librabbitmq/amqp_socket.h index e7c6e24..dffeec7 100644 --- a/librabbitmq/amqp_socket.h +++ b/librabbitmq/amqp_socket.h @@ -45,6 +45,11 @@ typedef enum { AMQP_SF_POLLERR = 8 } amqp_socket_flag_enum; +typedef enum { + AMQP_SC_NONE = 0, + AMQP_SC_FORCE = 1 +} amqp_socket_close_enum; + int amqp_os_socket_error(void); @@ -55,7 +60,7 @@ amqp_os_socket_close(int sockfd); typedef ssize_t (*amqp_socket_send_fn)(void *, const void *, size_t, int); typedef ssize_t (*amqp_socket_recv_fn)(void *, void *, size_t, int); typedef int (*amqp_socket_open_fn)(void *, const char *, int, struct timeval *); -typedef int (*amqp_socket_close_fn)(void *); +typedef int (*amqp_socket_close_fn)(void *, amqp_socket_close_enum); typedef int (*amqp_socket_get_sockfd_fn)(void *); typedef void (*amqp_socket_delete_fn)(void *); @@ -132,11 +137,13 @@ amqp_socket_recv(amqp_socket_t *self, void *buf, size_t len, int flags); * longer be referenced. * * \param [in,out] self A socket object. + * \param [in] force, if set, just close the socket, don't attempt a TLS + * shutdown. * * \return Zero upon success, non-zero otherwise. */ int -amqp_socket_close(amqp_socket_t *self); +amqp_socket_close(amqp_socket_t *self, amqp_socket_close_enum force); /** * Destroy a socket object diff --git a/librabbitmq/amqp_tcp_socket.c b/librabbitmq/amqp_tcp_socket.c index c15e87c..71de2ef 100644 --- a/librabbitmq/amqp_tcp_socket.c +++ b/librabbitmq/amqp_tcp_socket.c @@ -180,7 +180,7 @@ amqp_tcp_socket_open(void *base, const char *host, int port, struct timeval *tim } static int -amqp_tcp_socket_close(void *base) +amqp_tcp_socket_close(void *base, AMQP_UNUSED amqp_socket_close_enum force) { struct amqp_tcp_socket_t *self = (struct amqp_tcp_socket_t *)base; if (-1 == self->sockfd) { @@ -208,7 +208,7 @@ amqp_tcp_socket_delete(void *base) struct amqp_tcp_socket_t *self = (struct amqp_tcp_socket_t *)base; if (self) { - amqp_tcp_socket_close(self); + amqp_tcp_socket_close(self, AMQP_SC_NONE); free(self); } } -- cgit v1.2.1