From a72f462e2d0ca7ea8475da8a08e8aa953c8a7945 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 08:48:27 +0100 Subject: [PATCH 01/12] Fixes for MXS-196 and other related problems. --- server/core/dcb.c | 19 +++++- server/core/poll.c | 95 ++++++++++++++++++-------- server/include/dcb.h | 3 +- server/modules/protocol/mysql_client.c | 7 +- 4 files changed, 92 insertions(+), 32 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 8f2f159d4..99a01a640 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -724,7 +724,7 @@ int rc; */ rc = poll_add_dcb(dcb); - if (rc == DCBFD_CLOSED) { + if (rc) { dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); dcb_final_free(dcb); return NULL; @@ -2362,6 +2362,23 @@ bool dcb_set_state( return succp; } +void dcb_revert_state( + DCB* dcb, + const dcb_state_t new_state, + dcb_state_t old_state) +{ + CHK_DCB(dcb); + spinlock_acquire(&dcb->dcb_initlock); + + if ((DCB_STATE_POLLING == new_state || DCB_STATE_LISTENING == new_state) && (DCB_STATE_ALLOC == old_state || DCB_STATE_NOPOLLING == old_state)) + { + dcb->state = old_state; + spinlock_release(&dcb->dcb_initlock); + return; + } + else assert(false); +} + static bool dcb_set_state_nomutex( DCB* dcb, const dcb_state_t new_state, diff --git a/server/core/poll.c b/server/core/poll.c index 9a1a5565d..24b27006f 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -186,6 +186,11 @@ static struct { */ static void poll_loadav(void *); +/** + * Function to analyse error return from epoll_ctl + */ +static int poll_resolve_error(int, bool); + /** * Initialise the polling system we are using for the gateway. * @@ -275,20 +280,9 @@ poll_add_dcb(DCB *dcb) */ if (dcb_set_state(dcb, new_state, &old_state)) { rc = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, dcb->fd, &ev); - - if (rc != 0) { - int eno = errno; - errno = 0; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Adding dcb %p in state %s " - "to poll set failed. epoll_ctl failed due " - "%d, %s.", - dcb, - STRDCBSTATE(dcb->state), - eno, - strerror(eno)))); - } else { + if (rc) rc = poll_resolve_error(errno, true); + if (0 == rc) + { LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, "%lu [poll_add_dcb] Added dcb %p in state %s to " @@ -297,7 +291,7 @@ poll_add_dcb(DCB *dcb) dcb, STRDCBSTATE(dcb->state)))); } - ss_info_dassert(rc == 0, "Unable to add poll"); /*< trap in debug */ + else dcb_revert_state(dcb, new_state, old_state); } else { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, @@ -351,17 +345,7 @@ poll_remove_dcb(DCB *dcb) if (dcb->fd > 0) { rc = epoll_ctl(epoll_fd, EPOLL_CTL_DEL, dcb->fd, &ev); - - if (rc != 0) { - int eno = errno; - errno = 0; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : epoll_ctl failed due %d, %s.", - eno, - strerror(eno)))); - } - ss_dassert(rc == 0); /*< trap in debug */ + if (rc) rc = poll_resolve_error(errno, false); } } /*< @@ -380,6 +364,63 @@ return_rc: return rc; } +/** + * Check error returns from epoll_ctl. Most result in a crash since they + * are "impossible". Adding when already present is assumed non-fatal. + * Likewise, removing when not present is assumed non-fatal. + * It is assumed that callers to poll routines can handle the failure + * that results from hitting system limit, although an error is written + * here to record the problem. + * + * @param errornum The errno set by epoll_ctl + * @param adding True for adding to poll list, false for removing + * @return -1 on error or 0 for possibly revised return code + */ +static int +poll_resolve_error(int errornum, bool adding) +{ + if (adding) + { + if (EEXIST == errornum) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : epoll_ctl could not add, already exists."))); + // Assume another thread added and no serious harm done + return 0; + } + if (ENOSPC == errornum) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "The limit imposed by /proc/sys/fs/epoll/max_user_watches was " + "encountered while trying to register (EPOLL_CTL_ADD) a new " + "file descriptor on an epoll instance."))); + /* Failure - assume handled by callers */ + return -1; + } + } + else + { + /* Must be removing */ + if (ENOENT == errornum) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : epoll_ctl could not remove, not found."))); + // Assume another thread removed and no serious harm done + return 0; + } + } + /* Common checks for add or remove - crash MaxScale */ + if (EBADF == errornum) assert (!(EBADF == errornum)); + if (EINVAL == errornum) assert (!(EINVAL == errornum)); + if (ENOMEM == errornum) assert (!(ENOMEM == errornum)); + if (EPERM == errornum) assert (!(EPERM == errornum)); + /* Undocumented error number */ + assert(false); +} + #define BLOCKINGPOLL 0 /*< Set BLOCKING POLL to 1 if using a single thread and to make * debugging easier. */ @@ -1605,7 +1646,7 @@ RESULT_ROW *row; } /** - * Return a resultset that has the current set of services in it + * Return a result set that has the current set of services in it * * @return A Result set */ diff --git a/server/include/dcb.h b/server/include/dcb.h index 19f1e72ea..7f0997a9a 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -338,7 +338,8 @@ int dcb_remove_callback(DCB *, DCB_REASON, int (*)(struct dcb *, DCB_REASON, vo int dcb_isvalid(DCB *); /* Check the DCB is in the linked list */ int dcb_count_by_usage(DCB_USAGE); /* Return counts of DCBs */ -bool dcb_set_state(DCB* dcb, dcb_state_t new_state, dcb_state_t* old_state); +bool dcb_set_state(DCB *dcb, dcb_state_t new_state, dcb_state_t *old_state); +void dcb_revert_state(DCB *dcb, const dcb_state_t new_state, dcb_state_t old_state); void dcb_call_foreach (struct server* server, DCB_REASON reason); size_t dcb_get_session_id(DCB* dcb); bool dcb_get_ses_log_info(DCB* dcb, size_t* sesid, int* enabled_logs); diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index c3e463139..f4d3663a9 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -1451,8 +1451,8 @@ int gw_MySQLListener( // add listening socket to poll structure if (poll_add_dcb(listen_dcb) == -1) { fprintf(stderr, - "\n* Failed to start polling the socket due error " - "%i, %s.\n\n", + "\n* MaxScale encountered system limit while " + "attempting to register on an epoll instance.\n\n", errno, strerror(errno)); return 0; @@ -1687,7 +1687,8 @@ int gw_MySQLAccept(DCB *listener) client_dcb, 1, 0, - "MaxScale internal error."); + "MaxScale encountered system limit while " + "attempting to register on an epoll instance."); /** close client_dcb */ dcb_close(client_dcb); From 18a95eeb71892a50c0ddc721138388748aa6ed3f Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 14:50:01 +0100 Subject: [PATCH 02/12] Simplify adding and removing DCBs from polling, improve error handling. Remove dcb_set_state functions as not adding value. --- server/core/dcb.c | 200 ++----------------------------------------- server/core/poll.c | 164 +++++++++++++++++++++-------------- server/include/dcb.h | 2 - 3 files changed, 105 insertions(+), 261 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 99a01a640..abdc4dde6 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -84,10 +84,6 @@ static SPINLOCK dcbspin = SPINLOCK_INIT; static SPINLOCK zombiespin = SPINLOCK_INIT; static void dcb_final_free(DCB *dcb); -static bool dcb_set_state_nomutex( - DCB* dcb, - const dcb_state_t new_state, - dcb_state_t* old_state); static void dcb_call_callback(DCB *dcb, DCB_REASON reason); static DCB* dcb_get_next (DCB* dcb); static int dcb_null_write(DCB *dcb, GWBUF *buf); @@ -291,8 +287,7 @@ dcb_add_to_zombieslist(DCB *dcb) * Set state which indicates that it has been added to zombies * list. */ - succp = dcb_set_state(dcb, DCB_STATE_ZOMBIE, &prev_state); - ss_info_dassert(succp, "Failed to set DCB_STATE_ZOMBIE"); + dcb->state = DCB_STATE_ZOMBIE; spinlock_release(&zombiespin); } @@ -604,7 +599,7 @@ bool succp = false; &tls_log_info.li_sesid, &tls_log_info.li_enabled_logs))); - succp = dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); + dcb->state = DCB_STATE_DISCONNECTED; ss_dassert(succp); dcb_next = dcb->memdata.next; dcb_final_free(dcb); @@ -644,7 +639,7 @@ int rc; if ((funcs = (GWPROTOCOL *)load_module(protocol, MODULE_PROTOCOL)) == NULL) { - dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); + dcb->state = DCB_STATE_DISCONNECTED; dcb_final_free(dcb); LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, @@ -682,7 +677,7 @@ int rc; dcb, session->client, session->client->fd))); - dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); + dcb->state = DCB_STATE_DISCONNECTED; dcb_final_free(dcb); return NULL; } else { @@ -725,7 +720,7 @@ int rc; rc = poll_add_dcb(dcb); if (rc) { - dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); + dcb->state = DCB_STATE_DISCONNECTED; dcb_final_free(dcb); return NULL; } @@ -1920,7 +1915,7 @@ dcb_drain_writeq_SSL(DCB *dcb) } /** - * Removes dcb from poll set, and adds it to zombies list. As a consequense, + * Removes dcb from poll set, and adds it to zombies list. As a consequence, * dcb first moves to DCB_STATE_NOPOLLING, and then to DCB_STATE_ZOMBIE state. * At the end of the function state may not be DCB_STATE_ZOMBIE because once * dcb_initlock is released parallel threads may change the state. @@ -1947,7 +1942,6 @@ dcb_close(DCB *dcb) */ if (dcb->state == DCB_STATE_ALLOC) { - dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); dcb_final_free(dcb); return; } @@ -2341,188 +2335,6 @@ void dcb_hashtable_stats( dcb_printf(dcb, "\tLongest chain length: %d\n", longest); } - -bool dcb_set_state( - DCB* dcb, - const dcb_state_t new_state, - dcb_state_t* old_state) -{ - bool succp; - dcb_state_t state ; - - CHK_DCB(dcb); - spinlock_acquire(&dcb->dcb_initlock); - succp = dcb_set_state_nomutex(dcb, new_state, &state); - - spinlock_release(&dcb->dcb_initlock); - - if (old_state != NULL) { - *old_state = state; - } - return succp; -} - -void dcb_revert_state( - DCB* dcb, - const dcb_state_t new_state, - dcb_state_t old_state) -{ - CHK_DCB(dcb); - spinlock_acquire(&dcb->dcb_initlock); - - if ((DCB_STATE_POLLING == new_state || DCB_STATE_LISTENING == new_state) && (DCB_STATE_ALLOC == old_state || DCB_STATE_NOPOLLING == old_state)) - { - dcb->state = old_state; - spinlock_release(&dcb->dcb_initlock); - return; - } - else assert(false); -} - -static bool dcb_set_state_nomutex( - DCB* dcb, - const dcb_state_t new_state, - dcb_state_t* old_state) -{ - bool succp = false; - dcb_state_t state = DCB_STATE_UNDEFINED; - - CHK_DCB(dcb); - - state = dcb->state; - - if (old_state != NULL) { - *old_state = state; - } - - switch (state) { - case DCB_STATE_UNDEFINED: - dcb->state = new_state; - succp = true; - break; - - case DCB_STATE_ALLOC: - switch (new_state) { - /** fall through, for client requests */ - case DCB_STATE_POLLING: - /** fall through, for connect listeners */ - case DCB_STATE_LISTENING: - /** for failed connections */ - case DCB_STATE_DISCONNECTED: - dcb->state = new_state; - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_POLLING: - switch(new_state) { - case DCB_STATE_NOPOLLING: - dcb->state = new_state; - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_LISTENING: - switch(new_state) { - case DCB_STATE_NOPOLLING: - dcb->state = new_state; - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_NOPOLLING: - switch (new_state) { - /** Stopped services which are restarting will go from - * DCB_STATE_NOPOLLING to DCB_STATE_LISTENING.*/ - case DCB_STATE_LISTENING: - case DCB_STATE_ZOMBIE: /*< fall through */ - dcb->state = new_state; - case DCB_STATE_POLLING: /*< ok to try but state can't change */ - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_ZOMBIE: - switch (new_state) { - case DCB_STATE_DISCONNECTED: /*< fall through */ - dcb->state = new_state; - case DCB_STATE_POLLING: /*< ok to try but state can't change */ - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_DISCONNECTED: - switch (new_state) { - case DCB_STATE_FREED: - dcb->state = new_state; - succp = true; - break; - default: - ss_dassert(old_state != NULL); - break; - } - break; - - case DCB_STATE_FREED: - ss_dassert(old_state != NULL); - break; - - default: - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Unknown dcb state %s for " - "dcb %p", - STRDCBSTATE(dcb->state), - dcb))); - ss_dassert(false); - break; - } /*< switch (dcb->state) */ - - if (succp) { - LOGIF(LD, (skygw_log_write_flush( - LOGFILE_DEBUG, - "%lu [dcb_set_state_nomutex] dcb %p fd %d %s -> %s", - pthread_self(), - dcb, - dcb->fd, - STRDCBSTATE(state), - STRDCBSTATE(dcb->state)))); - } - else - { - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [dcb_set_state_nomutex] Failed " - "to change state of DCB %p. " - "Old state %s > new state %s.", - pthread_self(), - dcb, - (old_state == NULL ? "NULL" : STRDCBSTATE(*old_state)), - STRDCBSTATE(new_state)))); - } - return succp; -} - /** * Write data to a socket through an SSL structure. The SSL structure is linked to a DCB's socket * and all communication is encrypted and done via the SSL structure. diff --git a/server/core/poll.c b/server/core/poll.c index 24b27006f..9eed91bba 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -189,7 +189,7 @@ static void poll_loadav(void *); /** * Function to analyse error return from epoll_ctl */ -static int poll_resolve_error(int, bool); +static int poll_resolve_error(DCB *, int, bool); /** * Initialise the polling system we are using for the gateway. @@ -252,7 +252,7 @@ int poll_add_dcb(DCB *dcb) { int rc = -1; - dcb_state_t old_state = DCB_STATE_UNDEFINED; + dcb_state_t old_state = dcb->state; dcb_state_t new_state; struct epoll_event ev; @@ -268,47 +268,71 @@ poll_add_dcb(DCB *dcb) /*< * Choose new state according to the role of dcb. */ + spinlock_acquire(&dcb->dcb_initlock); if (dcb->dcb_role == DCB_ROLE_REQUEST_HANDLER) { new_state = DCB_STATE_POLLING; } else { ss_dassert(dcb->dcb_role == DCB_ROLE_SERVICE_LISTENER); new_state = DCB_STATE_LISTENING; } + /* + * Check DCB current state seems sensible + */ + if (DCB_STATE_DISCONNECTED == dcb->state + || DCB_STATE_ZOMBIE == dcb->state + || DCB_STATE_UNDEFINED == dcb->state) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "%lu [poll_add_dcb] Error : existing state of dcb %p " + "is %s, but this should be impossible, crashing.", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + assert(false); + } + if (DCB_STATE_POLLING == dcb->state + || DCB_STATE_LISTENING == dcb->state) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "%lu [poll_add_dcb] Error : existing state of dcb %p " + "is %s, but this is probably an error, not crashing.", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + } /*< * If dcb is in unexpected state, state change fails indicating that dcb * is not polling anymore. */ - if (dcb_set_state(dcb, new_state, &old_state)) { - rc = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, dcb->fd, &ev); - if (rc) rc = poll_resolve_error(errno, true); - if (0 == rc) - { - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [poll_add_dcb] Added dcb %p in state %s to " - "poll set.", - pthread_self(), - dcb, - STRDCBSTATE(dcb->state)))); - } - else dcb_revert_state(dcb, new_state, old_state); - } else { - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Unable to set new state for dcb %p " - "in state %s. Adding to poll set failed.", - dcb, - STRDCBSTATE(dcb->state)))); + dcb->state = new_state; + spinlock_release(&dcb->dcb_initlock); + /* + * The only possible failure that will not cause a crash is + * running out of system resources. + */ + rc = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, dcb->fd, &ev); + if (rc) + { + rc = poll_resolve_error(dcb, errno, true); } - + if (0 == rc) + { + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [poll_add_dcb] Added dcb %p in state %s to poll set.", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + } + else dcb->state = old_state; return rc; } /** * Remove a descriptor from the set of descriptors within the * polling environment. - * The state change command may fail because concurrent threads may call - * dcb_set_state simultaneously and the conflict is prevented in dcb_set_state. * * @param dcb The descriptor to remove * @return -1 on error or 0 on success @@ -316,52 +340,53 @@ poll_add_dcb(DCB *dcb) int poll_remove_dcb(DCB *dcb) { - struct epoll_event ev; int rc = -1; - dcb_state_t old_state = DCB_STATE_UNDEFINED; - dcb_state_t new_state = DCB_STATE_NOPOLLING; CHK_DCB(dcb); + spinlock_acquire(&dcb->dcb_initlock); /*< It is possible that dcb has already been removed from the set */ - if (dcb->state != DCB_STATE_POLLING && - dcb->state != DCB_STATE_LISTENING) - { - if (dcb->state == DCB_STATE_NOPOLLING || - dcb->state == DCB_STATE_ZOMBIE) - { - rc = 0; - } - goto return_rc; + if (dcb->state == DCB_STATE_NOPOLLING || + dcb->state == DCB_STATE_ZOMBIE) + { + spinlock_release(&dcb->dcb_initlock); + return 0; + } + if (DCB_STATE_POLLING != dcb->state + && DCB_STATE_LISTENING != dcb->state) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "%lu [poll_remove_dcb] Error : existing state of dcb %p " + "is %s, but this is probably an error, not crashing.", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); } /*< * Set state to NOPOLLING and remove dcb from poll set. */ - if (dcb_set_state(dcb, new_state, &old_state)) - { - /** - * Only positive fds can be removed from epoll set. - */ - if (dcb->fd > 0) - { - rc = epoll_ctl(epoll_fd, EPOLL_CTL_DEL, dcb->fd, &ev); - if (rc) rc = poll_resolve_error(errno, false); - } - } - /*< - * This call was redundant, but the end result is correct. + dcb->state = DCB_STATE_NOPOLLING; + /** + * Only positive fds can be removed from epoll set. + * But this test was removed by Martin as it is hard to + * see that there should ever be a situation where + * fd isn't positive and the DCB is also in the poll list. + */ + /* if (dcb->fd > 0) { */ + spinlock_release(&dcb->dcb_initlock); + rc = epoll_ctl(epoll_fd, EPOLL_CTL_DEL, dcb->fd, &ev); + /** + * The poll_resolve_error function will always + * return 0 or crash. So if it returns non-zero result, + * things have gone wrong and we crash. */ - else if (old_state == new_state) - { - rc = 0; - goto return_rc; - } - + if (rc) rc = poll_resolve_error(dcb, errno, false); + if (rc) assert(false); /*< Set bit for each maxscale thread */ - bitmask_copy(&dcb->memdata.bitmask, poll_bitmask()); - rc = 0; -return_rc: + bitmask_copy(&dcb->memdata.bitmask, poll_bitmask()); return rc; + /* } */ } /** @@ -377,7 +402,7 @@ return_rc: * @return -1 on error or 0 for possibly revised return code */ static int -poll_resolve_error(int errornum, bool adding) +poll_resolve_error(DCB *dcb, int errornum, bool adding) { if (adding) { @@ -385,7 +410,10 @@ poll_resolve_error(int errornum, bool adding) { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "Error : epoll_ctl could not add, already exists."))); + "%lu [poll_resolve_error] Error : epoll_ctl could not add, " + "already exists for DCB %p.", + pthread_self(), + dcb))); // Assume another thread added and no serious harm done return 0; } @@ -393,9 +421,12 @@ poll_resolve_error(int errornum, bool adding) { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "The limit imposed by /proc/sys/fs/epoll/max_user_watches was " + "%lu [poll_resolve_error] The limit imposed by " + "/proc/sys/fs/epoll/max_user_watches was " "encountered while trying to register (EPOLL_CTL_ADD) a new " - "file descriptor on an epoll instance."))); + "file descriptor on an epoll instance for dcb %p.", + pthread_self(), + dcb))); /* Failure - assume handled by callers */ return -1; } @@ -407,7 +438,10 @@ poll_resolve_error(int errornum, bool adding) { LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "Error : epoll_ctl could not remove, not found."))); + "%lu [poll_resolve_error] Error : epoll_ctl could not remove, " + "not found, for dcb %p.", + pthread_self(), + dcb))); // Assume another thread removed and no serious harm done return 0; } diff --git a/server/include/dcb.h b/server/include/dcb.h index 7f0997a9a..e33669b72 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -338,8 +338,6 @@ int dcb_remove_callback(DCB *, DCB_REASON, int (*)(struct dcb *, DCB_REASON, vo int dcb_isvalid(DCB *); /* Check the DCB is in the linked list */ int dcb_count_by_usage(DCB_USAGE); /* Return counts of DCBs */ -bool dcb_set_state(DCB *dcb, dcb_state_t new_state, dcb_state_t *old_state); -void dcb_revert_state(DCB *dcb, const dcb_state_t new_state, dcb_state_t old_state); void dcb_call_foreach (struct server* server, DCB_REASON reason); size_t dcb_get_session_id(DCB* dcb); bool dcb_get_ses_log_info(DCB* dcb, size_t* sesid, int* enabled_logs); From 77d374cd524559daaf7450e82e258d7ad8fdf37e Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 15:06:45 +0100 Subject: [PATCH 03/12] Fix mistake. --- server/core/poll.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/poll.c b/server/core/poll.c index 9eed91bba..21ad914ae 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -341,7 +341,7 @@ int poll_remove_dcb(DCB *dcb) { int rc = -1; - + struct epoll_event ev; CHK_DCB(dcb); spinlock_acquire(&dcb->dcb_initlock); From 44fcb9bc310e5111becea4e2ccf4818e5be724aa Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 15:29:58 +0100 Subject: [PATCH 04/12] Fix for incorrect password handling. --- server/modules/protocol/mysql_common.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 9b138360a..fcf254d64 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -46,6 +46,9 @@ #include #include +/* The following can be compared using memcmp to detect a null password */ +uint8_t null_client_sha1[MYSQL_SCRAMBLE_LEN]=""; + /** Defined in log_manager.cc */ extern int lm_enabled_logfiles_bitmask; extern size_t log_ses_count[]; @@ -577,7 +580,7 @@ int gw_send_authentication_to_backend( if (strlen(dbname)) curr_db = dbname; - if (strlen((char *)passwd)) + if (memcmp(passwd, null_client_sha1, MYSQL_SCRAMBLE_LEN)) curr_passwd = passwd; dcb = conn->owner_dcb; @@ -1122,7 +1125,7 @@ GWBUF* gw_create_change_user_packet( curr_db = db; } - if (strlen((char *)pwd) > 0) + if (memcmp(pwd, null_client_sha1, MYSQL_SCRAMBLE_LEN)) { curr_passwd = pwd; } @@ -1358,12 +1361,7 @@ int gw_check_mysql_scramble_data(DCB *dcb, uint8_t *token, unsigned int token_le gw_bin2hex(hex_double_sha1, password, SHA_DIGEST_LENGTH); } else { /* check if the password is not set in the user table */ - if (!strlen((char *)password)) { - /* Username without password */ - return 0; - } else { - return 1; - } + return memcmp(password, null_client_sha1, MYSQL_SCRAMBLE_LEN) ? 1 : 0; } /*< From fffd8fb73a99e24edc3b8b935f878110965ef38a Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 16:48:53 +0100 Subject: [PATCH 05/12] Unify DCB close processing to single function dcb_close. Remove dcb_add_to_zombieslist (incorporating logic into dcb_close). Alter logic so that DCB that is just allocated will still go to zombie list if dcb->fd is not closed. --- server/core/dcb.c | 186 ++++++++++++---------------- server/core/poll.c | 4 - server/core/test/testdcb.c | 2 +- server/include/dcb.h | 1 - server/modules/protocol/maxscaled.c | 9 +- server/modules/protocol/telnetd.c | 6 +- 6 files changed, 88 insertions(+), 120 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index abdc4dde6..ef259ca4b 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -248,50 +248,6 @@ dcb_free(DCB *dcb) } } -/** - * Add the DCB to the end of zombies list. - * - * Adding to list occurs once per DCB. This is ensured by changing the - * state of DCB to DCB_STATE_ZOMBIE after addition. Prior insertion, DCB state - * is checked and operation proceeds only if state differs from DCB_STATE_ZOMBIE. - * @param dcb The DCB to add to the zombie list - * @return none - */ -void -dcb_add_to_zombieslist(DCB *dcb) -{ - bool succp = false; - dcb_state_t prev_state = DCB_STATE_UNDEFINED; - - CHK_DCB(dcb); - - /*< - * Protect zombies list access. - */ - spinlock_acquire(&zombiespin); - /*< - * If dcb is already added to zombies list, return. - */ - if (dcb->state != DCB_STATE_NOPOLLING) { - ss_dassert(dcb->state != DCB_STATE_POLLING && - dcb->state != DCB_STATE_LISTENING); - spinlock_release(&zombiespin); - return; - } - /*< - * Add closing dcb to the top of the list. - */ - dcb->memdata.next = zombies; - zombies = dcb; - /*< - * Set state which indicates that it has been added to zombies - * list. - */ - dcb->state = DCB_STATE_ZOMBIE; - - spinlock_release(&zombiespin); -} - /* * Clone a DCB for internal use, mostly used for specialist filters * to create dummy clients based on real clients. @@ -600,7 +556,6 @@ bool succp = false; &tls_log_info.li_enabled_logs))); dcb->state = DCB_STATE_DISCONNECTED; - ss_dassert(succp); dcb_next = dcb->memdata.next; dcb_final_free(dcb); dcb = dcb_next; @@ -1928,72 +1883,91 @@ dcb_drain_writeq_SSL(DCB *dcb) void dcb_close(DCB *dcb) { - int rc = 0; + int rc = 0; - CHK_DCB(dcb); + CHK_DCB(dcb); - LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, - "%lu [dcb_close]", - pthread_self()))); - - /** - * dcb_close may be called for freshly created dcb, in which case - * it only needs to be freed. - */ - if (dcb->state == DCB_STATE_ALLOC) + LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, + "%lu [dcb_close]", + pthread_self()))); + + if (DCB_STATE_UNDEFINED == dcb->state + || DCB_STATE_DISCONNECTED == dcb->state + || DCB_STATE_ZOMBIE == dcb->state) + { + LOGIF(LE, (skygw_log_write( + LOGFILE_ERROR, + "%lu [dcb_close] Error : Removing DCB %p but was in state %s " + "which is not legal for a call to dcb_close. ", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + assert(false); + } + + /** + * dcb_close may be called for freshly created dcb, in which case + * it only needs to be freed. + */ + if (dcb->state == DCB_STATE_ALLOC && dcb->fd != DCBFD_CLOSED) + { + dcb_final_free(dcb); + return; + } + + /*< + * Stop dcb's listening and modify state accordingly. + */ + if (dcb->state == DCB_STATE_POLLING || dcb->state == DCB_STATE_LISTENING) + { + if (dcb->state == DCB_STATE_LISTENING) { - dcb_final_free(dcb); - return; + LOGIF(LE, (skygw_log_write( + LOGFILE_ERROR, + "%lu [dcb_close] Error : Removing DCB %p but was in state %s " + "which is not expected for a call to dcb_close, although it" + "should be processed correctly. ", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); } + rc = poll_remove_dcb(dcb); + /* + * Return will always be 0 or function will have crashed + */ + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_close] Removed dcb %p in state %s from " + "poll set.", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + /** + * close protocol and router session + */ + if (dcb->func.close != NULL) + { + dcb->func.close(dcb); + } + /** Call possible callback for this DCB in case of close */ + dcb_call_callback(dcb, DCB_REASON_CLOSE); + } + assert (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC); + + spinlock_acquire(&zombiespin); + + /*< + * Add closing dcb to the top of the list. + */ + dcb->memdata.next = zombies; + zombies = dcb; + /*< + * Set state which indicates that it has been added to zombies + * list. + */ + dcb->state = DCB_STATE_ZOMBIE; - ss_dassert(dcb->state == DCB_STATE_POLLING || - dcb->state == DCB_STATE_NOPOLLING || - dcb->state == DCB_STATE_ZOMBIE); - - /*< - * Stop dcb's listening and modify state accordingly. - */ - if (dcb->state == DCB_STATE_POLLING) - { - rc = poll_remove_dcb(dcb); - - if (rc == 0) { - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [dcb_close] Removed dcb %p in state %s from " - "poll set.", - pthread_self(), - dcb, - STRDCBSTATE(dcb->state)))); - } else { - LOGIF(LE, (skygw_log_write( - LOGFILE_ERROR, - "Error : Removing DCB fd == %d in state %s from " - "poll set failed.", - dcb->fd, - STRDCBSTATE(dcb->state)))); - } - - if (rc == 0) - { - /** - * close protocol and router session - */ - if (dcb->func.close != NULL) - { - dcb->func.close(dcb); - } - /** Call possible callback for this DCB in case of close */ - dcb_call_callback(dcb, DCB_REASON_CLOSE); - - if (dcb->state == DCB_STATE_NOPOLLING) - { - dcb_add_to_zombieslist(dcb); - } - } - ss_dassert(dcb->state == DCB_STATE_NOPOLLING || - dcb->state == DCB_STATE_ZOMBIE); - } + spinlock_release(&zombiespin); } /** diff --git a/server/core/poll.c b/server/core/poll.c index 21ad914ae..0406140a4 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -302,10 +302,6 @@ poll_add_dcb(DCB *dcb) dcb, STRDCBSTATE(dcb->state)))); } - /*< - * If dcb is in unexpected state, state change fails indicating that dcb - * is not polling anymore. - */ dcb->state = new_state; spinlock_release(&dcb->dcb_initlock); /* diff --git a/server/core/test/testdcb.c b/server/core/test/testdcb.c index 2703763f0..13462fc87 100644 --- a/server/core/test/testdcb.c +++ b/server/core/test/testdcb.c @@ -64,7 +64,7 @@ int buflen; ss_info_dassert(!dcb_isvalid(dcb), "Freed DCB must not be valid"); ss_dfprintf(stderr, "\t..done\nMake clone DCB a zombie"); clone->state = DCB_STATE_NOPOLLING; - dcb_add_to_zombieslist(clone); + dcb_close(clone); ss_info_dassert(dcb_get_zombies() == clone, "Clone DCB must be start of zombie list now"); ss_dfprintf(stderr, "\t..done\nProcess the zombies list"); dcb_process_zombies(0); diff --git a/server/include/dcb.h b/server/include/dcb.h index e33669b72..0e5cddb48 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -330,7 +330,6 @@ const char *gw_dcb_state2string(int); /* DCB state to string */ void dcb_printf(DCB *, const char *, ...); /* DCB version of printf */ int dcb_isclient(DCB *); /* the DCB is the client of the session */ void dcb_hashtable_stats(DCB *, void *); /**< Print statisitics */ -void dcb_add_to_zombieslist(DCB* dcb); int dcb_add_callback(DCB *, DCB_REASON, int (*)(struct dcb *, DCB_REASON, void *), void *); int dcb_remove_callback(DCB *, DCB_REASON, int (*)(struct dcb *, DCB_REASON, void *), diff --git a/server/modules/protocol/maxscaled.c b/server/modules/protocol/maxscaled.c index 67d51f9bd..0812ea964 100644 --- a/server/modules/protocol/maxscaled.c +++ b/server/modules/protocol/maxscaled.c @@ -270,9 +270,7 @@ int n_connect = 0; { atomic_add(&dcb->stats.n_accepts, 1); client_dcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER); - if (client_dcb == NULL) - { close(so); return n_connect; @@ -283,7 +281,8 @@ int n_connect = 0; if ((maxscaled_pr = (MAXSCALED *)malloc(sizeof(MAXSCALED))) == NULL) { client_dcb->protocol = NULL; - dcb_add_to_zombieslist(client_dcb); + close(so); + dcb_close(client_dcb); return n_connect; } maxscaled_pr->username = NULL; @@ -293,9 +292,9 @@ int n_connect = 0; client_dcb->session = session_alloc(dcb->session->service, client_dcb); - if (poll_add_dcb(client_dcb) == -1) + if (poll_add_dcb(client_dcb)) { - dcb_add_to_zombieslist(dcb); + dcb_close(dcb); return n_connect; } n_connect++; diff --git a/server/modules/protocol/telnetd.c b/server/modules/protocol/telnetd.c index ab5c95b47..9a88307d2 100644 --- a/server/modules/protocol/telnetd.c +++ b/server/modules/protocol/telnetd.c @@ -315,13 +315,13 @@ int n_connect = 0; if (telnetd_pr == NULL) { - dcb_add_to_zombieslist(client_dcb); + dcb_close(client_dcb); return n_connect; } - if (poll_add_dcb(client_dcb) == -1) + if (poll_add_dcb(client_dcb)) { - dcb_add_to_zombieslist(dcb); + dcb_close(dcb); return n_connect; } n_connect++; From 96619e2f8f03994cabf88b5241944a49b9d29b82 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 17:39:32 +0100 Subject: [PATCH 06/12] Allow zombies to be submitted to dcb_close - but why does this happen? --- server/core/dcb.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/server/core/dcb.c b/server/core/dcb.c index ef259ca4b..fc4c412ea 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1891,6 +1891,18 @@ dcb_close(DCB *dcb) "%lu [dcb_close]", pthread_self()))); + if (DCB_STATE_ZOMBIE == dcb->state) + { + LOGIF(LE, (skygw_log_write( + LOGFILE_ERROR, + "%lu [dcb_close] Error : Removing DCB %p but was in state %s " + "which is unexpected for a call to dcb_close, but not crashing. ", + pthread_self(), + dcb, + STRDCBSTATE(dcb->state)))); + return 0; + } + if (DCB_STATE_UNDEFINED == dcb->state || DCB_STATE_DISCONNECTED == dcb->state || DCB_STATE_ZOMBIE == dcb->state) From 9ee8d11808c6349adb14f7799035f9398f5e1b61 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Mon, 6 Jul 2015 17:57:03 +0100 Subject: [PATCH 07/12] Allow for DCB becoming a zombie during processing. --- server/core/dcb.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index fc4c412ea..7a5314580 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1883,8 +1883,6 @@ dcb_drain_writeq_SSL(DCB *dcb) void dcb_close(DCB *dcb) { - int rc = 0; - CHK_DCB(dcb); LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, @@ -1900,7 +1898,7 @@ dcb_close(DCB *dcb) pthread_self(), dcb, STRDCBSTATE(dcb->state)))); - return 0; + return; } if (DCB_STATE_UNDEFINED == dcb->state @@ -1943,9 +1941,10 @@ dcb_close(DCB *dcb) dcb, STRDCBSTATE(dcb->state)))); } - rc = poll_remove_dcb(dcb); + poll_remove_dcb(dcb); /* - * Return will always be 0 or function will have crashed + * Return will always be 0 or function will have crashed, so we + * threw away return value. */ LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, @@ -1964,22 +1963,23 @@ dcb_close(DCB *dcb) /** Call possible callback for this DCB in case of close */ dcb_call_callback(dcb, DCB_REASON_CLOSE); } - assert (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC); + if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC); + { + spinlock_acquire(&zombiespin); - spinlock_acquire(&zombiespin); - - /*< - * Add closing dcb to the top of the list. - */ - dcb->memdata.next = zombies; - zombies = dcb; - /*< - * Set state which indicates that it has been added to zombies - * list. - */ - dcb->state = DCB_STATE_ZOMBIE; + /*< + * Add closing dcb to the top of the list. + */ + dcb->memdata.next = zombies; + zombies = dcb; + /*< + * Set state which indicates that it has been added to zombies + * list. + */ + dcb->state = DCB_STATE_ZOMBIE; - spinlock_release(&zombiespin); + spinlock_release(&zombiespin); + } } /** From 5577ef94e9a32fa813291b8780bebcbfe678ba5d Mon Sep 17 00:00:00 2001 From: counterpoint Date: Tue, 7 Jul 2015 08:37:29 +0100 Subject: [PATCH 08/12] Wrap spinlock around more logic; simplify process zombies list logic. --- server/core/dcb.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 7a5314580..f350a8ad3 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -484,14 +484,13 @@ bool succp = false; /*< * Move dcb to linked list of victim dcbs. */ + ptr->memdata.next = NULL; if (dcb_list == NULL) { dcb_list = ptr; - dcb = dcb_list; } else { dcb->memdata.next = ptr; - dcb = dcb->memdata.next; } - dcb->memdata.next = NULL; + dcb = ptr; ptr = tptr; } else @@ -1963,10 +1962,10 @@ dcb_close(DCB *dcb) /** Call possible callback for this DCB in case of close */ dcb_call_callback(dcb, DCB_REASON_CLOSE); } + + spinlock_acquire(&zombiespin); if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC); { - spinlock_acquire(&zombiespin); - /*< * Add closing dcb to the top of the list. */ @@ -1977,9 +1976,8 @@ dcb_close(DCB *dcb) * list. */ dcb->state = DCB_STATE_ZOMBIE; - - spinlock_release(&zombiespin); } + spinlock_release(&zombiespin); } /** From d4eff72d8a659f33825bc1163092d58267b5d932 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Tue, 7 Jul 2015 08:59:52 +0100 Subject: [PATCH 09/12] Fix stupid bug. --- server/core/dcb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index f350a8ad3..210e6d82d 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1964,7 +1964,7 @@ dcb_close(DCB *dcb) } spinlock_acquire(&zombiespin); - if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC); + if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC) { /*< * Add closing dcb to the top of the list. From 462c8e42ef787499eb37ea671fc21a24eaba4437 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Tue, 7 Jul 2015 09:12:38 +0100 Subject: [PATCH 10/12] Fix more subtle bug and expand debug message for dcb_close entry. --- server/core/dcb.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 210e6d82d..87542b5c1 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -1885,24 +1885,18 @@ dcb_close(DCB *dcb) CHK_DCB(dcb); LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, - "%lu [dcb_close]", - pthread_self()))); + "%lu [dcb_close] DCB %p in state %s", + pthread_self(), + dcb, + dcb ? STRDCBSTATE(dcb->state) : "Invalid DCB"))); if (DCB_STATE_ZOMBIE == dcb->state) { - LOGIF(LE, (skygw_log_write( - LOGFILE_ERROR, - "%lu [dcb_close] Error : Removing DCB %p but was in state %s " - "which is unexpected for a call to dcb_close, but not crashing. ", - pthread_self(), - dcb, - STRDCBSTATE(dcb->state)))); return; } - if (DCB_STATE_UNDEFINED == dcb->state - || DCB_STATE_DISCONNECTED == dcb->state - || DCB_STATE_ZOMBIE == dcb->state) + if (DCB_STATE_UNDEFINED == dcb->state + || DCB_STATE_DISCONNECTED == dcb->state) { LOGIF(LE, (skygw_log_write( LOGFILE_ERROR, From be789855eeb294d2c0b1c8a92505212931c5d935 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Tue, 7 Jul 2015 09:43:04 +0100 Subject: [PATCH 11/12] Add lines to revision history. --- server/core/dcb.c | 3 +++ server/core/poll.c | 1 + server/modules/protocol/httpd.c | 1 + 3 files changed, 5 insertions(+) diff --git a/server/core/dcb.c b/server/core/dcb.c index 87542b5c1..d1eb69a5a 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -50,6 +50,9 @@ * 07/05/2014 Mark Riddoch Addition of callback mechanism * 20/06/2014 Mark Riddoch Addition of dcb_clone * 29/05/2015 Markus Makela Addition of dcb_write_SSL + * 07/07/2015 Martin Brampton Merged add to zombieslist into dcb_close, + * fixes for various error situations, + * remove dcb_set_state etc, simplifications. * * @endverbatim */ diff --git a/server/core/poll.c b/server/core/poll.c index 0406140a4..95ae7dc73 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -71,6 +71,7 @@ int max_poll_sleep; * in the loop after the epoll_wait. This allows for better * thread utilisaiton and fairer scheduling of the event * processing. + * 07/07/15 Martin Brampton Simplified add and remove DCB, improve error handling. * * @endverbatim */ diff --git a/server/modules/protocol/httpd.c b/server/modules/protocol/httpd.c index 0de934dec..edd6c75b8 100644 --- a/server/modules/protocol/httpd.c +++ b/server/modules/protocol/httpd.c @@ -33,6 +33,7 @@ * Date Who Description * 08/07/2013 Massimiliano Pinto Initial version * 09/07/2013 Massimiliano Pinto Added /show?dcb|session for all dcbs|sessions + * 07/07/2015 Martin Brampton Fixed problems with DCBs on errors * * @endverbatim */ From 4c8aa02c31b92259fd3f0eba4bf8063f07a0263a Mon Sep 17 00:00:00 2001 From: counterpoint Date: Tue, 7 Jul 2015 16:51:22 +0100 Subject: [PATCH 12/12] Finalise comments; change abort from assert(false) to raise(SIGABRT). --- server/core/dcb.c | 3 ++- server/core/poll.c | 8 +++++--- server/modules/protocol/httpd.c | 1 - server/modules/protocol/maxscaled.c | 1 + server/modules/protocol/mysql_common.c | 1 + server/modules/protocol/telnetd.c | 1 + 6 files changed, 10 insertions(+), 5 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index d1eb69a5a..78f4a821f 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -60,6 +60,7 @@ #include #include #include +#include #include #include #include @@ -1908,7 +1909,7 @@ dcb_close(DCB *dcb) pthread_self(), dcb, STRDCBSTATE(dcb->state)))); - assert(false); + raise(SIGABRT); } /** diff --git a/server/core/poll.c b/server/core/poll.c index 95ae7dc73..a7afa9345 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -15,10 +15,12 @@ * * Copyright MariaDB Corporation Ab 2013-2014 */ + #include #include #include #include +#include #include #include #include @@ -290,7 +292,7 @@ poll_add_dcb(DCB *dcb) pthread_self(), dcb, STRDCBSTATE(dcb->state)))); - assert(false); + raise(SIGABRT); } if (DCB_STATE_POLLING == dcb->state || DCB_STATE_LISTENING == dcb->state) @@ -379,7 +381,7 @@ poll_remove_dcb(DCB *dcb) * things have gone wrong and we crash. */ if (rc) rc = poll_resolve_error(dcb, errno, false); - if (rc) assert(false); + if (rc) raise(SIGABRT); /*< Set bit for each maxscale thread */ bitmask_copy(&dcb->memdata.bitmask, poll_bitmask()); return rc; @@ -449,7 +451,7 @@ poll_resolve_error(DCB *dcb, int errornum, bool adding) if (ENOMEM == errornum) assert (!(ENOMEM == errornum)); if (EPERM == errornum) assert (!(EPERM == errornum)); /* Undocumented error number */ - assert(false); + raise(SIGABRT); } #define BLOCKINGPOLL 0 /*< Set BLOCKING POLL to 1 if using a single thread and to make diff --git a/server/modules/protocol/httpd.c b/server/modules/protocol/httpd.c index edd6c75b8..0de934dec 100644 --- a/server/modules/protocol/httpd.c +++ b/server/modules/protocol/httpd.c @@ -33,7 +33,6 @@ * Date Who Description * 08/07/2013 Massimiliano Pinto Initial version * 09/07/2013 Massimiliano Pinto Added /show?dcb|session for all dcbs|sessions - * 07/07/2015 Martin Brampton Fixed problems with DCBs on errors * * @endverbatim */ diff --git a/server/modules/protocol/maxscaled.c b/server/modules/protocol/maxscaled.c index 0812ea964..159718df0 100644 --- a/server/modules/protocol/maxscaled.c +++ b/server/modules/protocol/maxscaled.c @@ -58,6 +58,7 @@ extern __thread log_info_t tls_log_info; * Revision History * Date Who Description * 13/06/2014 Mark Riddoch Initial implementation + * 07/07/15 Martin Brampton Correct failure handling * * @endverbatim */ diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index fcf254d64..ea3227699 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -36,6 +36,7 @@ * 03/10/2014 Massimiliano Pinto Added netmask for wildcard in IPv4 hosts. * 24/10/2014 Massimiliano Pinto Added Mysql user@host @db authentication support * 10/11/2014 Massimiliano Pinto Charset at connect is passed to backend during authentication + * 07/07/15 Martin Brampton Fix problem recognising null password * */ diff --git a/server/modules/protocol/telnetd.c b/server/modules/protocol/telnetd.c index 9a88307d2..006fabe99 100644 --- a/server/modules/protocol/telnetd.c +++ b/server/modules/protocol/telnetd.c @@ -67,6 +67,7 @@ extern __thread log_info_t tls_log_info; * Date Who Description * 17/06/2013 Mark Riddoch Initial version * 17/07/2013 Mark Riddoch Addition of login phase + * 07/07/2015 Martin Brampton Call unified dcb_close on error * * @endverbatim */