diff --git a/server/core/dcb.c b/server/core/dcb.c index 8f2f159d4..78f4a821f 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 */ @@ -57,6 +60,7 @@ #include #include #include +#include #include #include #include @@ -84,10 +88,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); @@ -252,51 +252,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. - */ - succp = dcb_set_state(dcb, DCB_STATE_ZOMBIE, &prev_state); - ss_info_dassert(succp, "Failed to set 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. @@ -533,14 +488,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 @@ -604,8 +558,7 @@ bool succp = false; &tls_log_info.li_sesid, &tls_log_info.li_enabled_logs))); - succp = dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); - ss_dassert(succp); + dcb->state = DCB_STATE_DISCONNECTED; dcb_next = dcb->memdata.next; dcb_final_free(dcb); dcb = dcb_next; @@ -644,7 +597,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 +635,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 { @@ -724,8 +677,8 @@ int rc; */ rc = poll_add_dcb(dcb); - if (rc == DCBFD_CLOSED) { - dcb_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); + if (rc) { + dcb->state = DCB_STATE_DISCONNECTED; dcb_final_free(dcb); return NULL; } @@ -1920,7 +1873,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. @@ -1933,73 +1886,96 @@ dcb_drain_writeq_SSL(DCB *dcb) void dcb_close(DCB *dcb) { - int rc = 0; + CHK_DCB(dcb); - CHK_DCB(dcb); + LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, + "%lu [dcb_close] DCB %p in state %s", + pthread_self(), + dcb, + dcb ? STRDCBSTATE(dcb->state) : "Invalid 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) + if (DCB_STATE_ZOMBIE == dcb->state) + { + return; + } + + if (DCB_STATE_UNDEFINED == dcb->state + || DCB_STATE_DISCONNECTED == 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)))); + raise(SIGABRT); + } + + /** + * 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_set_state(dcb, DCB_STATE_DISCONNECTED, NULL); - 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)))); } - - ss_dassert(dcb->state == DCB_STATE_POLLING || - dcb->state == DCB_STATE_NOPOLLING || - dcb->state == DCB_STATE_ZOMBIE); - + poll_remove_dcb(dcb); + /* + * Return will always be 0 or function will have crashed, so we + * threw away return value. + */ + 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); + } + + spinlock_acquire(&zombiespin); + if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC) + { /*< - * 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); - } + * 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); } /** @@ -2341,171 +2317,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; -} - -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 9a1a5565d..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 @@ -71,6 +73,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 */ @@ -186,6 +189,11 @@ static struct { */ static void poll_loadav(void *); +/** + * Function to analyse error return from epoll_ctl + */ +static int poll_resolve_error(DCB *, int, bool); + /** * Initialise the polling system we are using for the gateway. * @@ -247,7 +255,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; @@ -263,58 +271,67 @@ 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; } - /*< - * If dcb is in unexpected state, state change fails indicating that dcb - * is not polling anymore. + /* + * Check DCB current state seems sensible */ - 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 { - 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)))); - } - ss_info_dassert(rc == 0, "Unable to add poll"); /*< trap in debug */ - } 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)))); + 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)))); + raise(SIGABRT); } - + 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)))); + } + 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 @@ -322,62 +339,119 @@ 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; - + struct epoll_event ev; 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 != 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 */ - } - } - /*< - * 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) raise(SIGABRT); /*< 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; + /* } */ +} + +/** + * 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(DCB *dcb, int errornum, bool adding) +{ + if (adding) + { + if (EEXIST == errornum) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "%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; + } + if (ENOSPC == errornum) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "%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 for dcb %p.", + pthread_self(), + dcb))); + /* Failure - assume handled by callers */ + return -1; + } + } + else + { + /* Must be removing */ + if (ENOENT == errornum) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "%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; + } + } + /* 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 */ + raise(SIGABRT); } #define BLOCKINGPOLL 0 /*< Set BLOCKING POLL to 1 if using a single thread and to make @@ -1605,7 +1679,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/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 19f1e72ea..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 *), @@ -338,7 +337,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_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/maxscaled.c b/server/modules/protocol/maxscaled.c index 67d51f9bd..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 */ @@ -270,9 +271,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 +282,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 +293,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/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); diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 9b138360a..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 * */ @@ -46,6 +47,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 +581,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 +1126,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 +1362,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; } /*< diff --git a/server/modules/protocol/telnetd.c b/server/modules/protocol/telnetd.c index ab5c95b47..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 */ @@ -315,13 +316,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++;