diff --git a/client/CMakeLists.txt b/client/CMakeLists.txt index 32ab702ea..eb13ae659 100644 --- a/client/CMakeLists.txt +++ b/client/CMakeLists.txt @@ -8,3 +8,7 @@ else() message(STATUS "Could not find editline library. MaxAdmin will be built without it.") endif() install(TARGETS maxadmin DESTINATION ${MAXSCALE_BINDIR}) + +if(BUILD_TESTS) + add_subdirectory(test) +endif() diff --git a/client/maxadmin.c b/client/maxadmin.c index dec941d2f..6f8899163 100644 --- a/client/maxadmin.c +++ b/client/maxadmin.c @@ -466,7 +466,8 @@ int i, j, newline = 1; { if (newline == 1 && buf[j] == 'O') newline = 2; - else if (newline == 2 && buf[j] == 'K' && j == i - 1) + else if ((newline == 2 && buf[j] == 'K' && j == i - 1) || + (j == i - 2 && buf[j] == 'O' && buf[j + 1] == 'K')) { return 1; } diff --git a/client/test/CMakeLists.txt b/client/test/CMakeLists.txt new file mode 100644 index 000000000..e59eaf7f0 --- /dev/null +++ b/client/test/CMakeLists.txt @@ -0,0 +1,4 @@ +install(PROGRAMS maxadmin_test.sh DESTINATION ${CMAKE_BINARY_DIR}) +install(PROGRAMS maxadmin_stress.sh DESTINATION ${CMAKE_BINARY_DIR}) +add_test(TestMaxAdmin ${CMAKE_BINARY_DIR}/maxadmin_test.sh) +add_test(TestMaxAdminStress ${CMAKE_BINARY_DIR}/maxadmin_stress.sh) diff --git a/client/test/maxadmin_stress.sh b/client/test/maxadmin_stress.sh old mode 100644 new mode 100755 diff --git a/client/test/maxadmin_test.sh b/client/test/maxadmin_test.sh index c58c1512e..03976bc90 100755 --- a/client/test/maxadmin_test.sh +++ b/client/test/maxadmin_test.sh @@ -96,61 +96,45 @@ fi # # Test enable|disable log debug|trace|message|error # -maxadmin -pmariadb enable log debug >& /dev/null -if [ $? -eq "1" ]; then - echo "Enable debug log: Failed" - failure=`expr $failure + 1` -else - passed=`expr $passed + 1` - echo "Enable debug log: Passed" -fi -maxadmin -pmariadb enable log trace >& /dev/null -if [ $? -eq "1" ]; then - echo "Enable trace log: Failed" - failure=`expr $failure + 1` -else - passed=`expr $passed + 1` - echo "Enable trace log: Passed" -fi +for action in enable disable +do + maxadmin -pmariadb $action log debug >& /dev/null + if [ $? -eq "1" ]; then + echo "$action debug log: Failed" + failure=`expr $failure + 1` + else + passed=`expr $passed + 1` + echo "$action debug log: Passed" + fi -maxadmin -pmariadb enable log message >& /dev/null -if [ $? -eq "1" ]; then - echo "Enable message log: Failed" + maxadmin -pmariadb $action log trace >& /dev/null + if [ $? -eq "1" ]; then + echo "$action trace log: Failed" + failure=`expr $failure + 1` + else + passed=`expr $passed + 1` + echo "$action trace log: Passed" + fi + + maxadmin -pmariadb $action log message >& /dev/null + if [ $? -eq "1" ]; then + echo "$action message log: Failed" failure=`expr $failure + 1` -else + else passed=`expr $passed + 1` - echo "Enable message log: Passed" -fi + echo "$action message log: Passed" + fi -maxadmin -pmariadb enable log error >& /dev/null -if [ $? -eq "1" ]; then - echo "Enable error log: Failed" + maxadmin -pmariadb $action log error >& /dev/null + if [ $? -eq "1" ]; then + echo "$action error log: Failed" failure=`expr $failure + 1` -else + else passed=`expr $passed + 1` - echo "Enable error log: Passed" -fi - - - -maxadmin -pmariadb disable log debug >& /dev/null -if [ $? -eq "1" ]; then - echo "Disable debug log: Failed" - failure=`expr $failure + 1` -else - passed=`expr $passed + 1` - echo "Disable debug log: Passed" -fi - -maxadmin -pmariadb disable log trace >& /dev/null -if [ $? -eq "1" ]; then - echo "Disable trace log: Failed" - failure=`expr $failure + 1` -else - passed=`expr $passed + 1` - echo "Disable trace log: Passed" -fi + echo "$action error log: Passed" + fi +done # # Test restart monitor|service without, with invalid and with long invalid argument @@ -186,7 +170,7 @@ do done # -# Test set server qwerty master withaout, with invalid and with long invalid arg +# Test set server qwerty master without, with invalid and with long invalid arg # maxadmin -pmariadb set server qwerty >& /dev/null if [ $? -eq "1" ]; then diff --git a/log_manager/test/CMakeLists.txt b/log_manager/test/CMakeLists.txt index e510cf79a..e485e747e 100644 --- a/log_manager/test/CMakeLists.txt +++ b/log_manager/test/CMakeLists.txt @@ -1,6 +1,6 @@ add_executable(testlog testlog.c) -add_executable(testorder testorder.c) -target_link_libraries(testlog pthread log_manager utils) -target_link_libraries(testorder pthread log_manager utils) +add_executable(testorder testorder.c ../../server/core/random_jkiss.c) +target_link_libraries(testlog pthread log_manager utils fullcore) +target_link_libraries(testorder pthread log_manager utils fullcore) add_test(NAME Internal-TestLogOrder COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/logorder.sh 200 0 1000 ${CMAKE_CURRENT_BINARY_DIR}/logorder.log) add_test(Internal-TestLog testlog) diff --git a/query_classifier/test/CMakeLists.txt b/query_classifier/test/CMakeLists.txt index 44b66e461..66c56eca7 100644 --- a/query_classifier/test/CMakeLists.txt +++ b/query_classifier/test/CMakeLists.txt @@ -10,5 +10,5 @@ endif() add_subdirectory(canonical_tests) add_executable(classify classify.c) -target_link_libraries(classify query_classifier fullcore) +target_link_libraries(classify query_classifier ${CURL_LIBRARIES} utils log_manager pthread ${EMBEDDED_LIB} ${PCRE_LINK_FLAGS} ssl aio rt crypt dl crypto inih z m stdc++ fullcore) add_test(Internal-TestQueryClassifier classify ${CMAKE_CURRENT_SOURCE_DIR}/input.sql ${CMAKE_CURRENT_SOURCE_DIR}/expected.sql) diff --git a/query_classifier/test/canonical_tests/CMakeLists.txt b/query_classifier/test/canonical_tests/CMakeLists.txt index 1dafc428e..925a3dd4f 100644 --- a/query_classifier/test/canonical_tests/CMakeLists.txt +++ b/query_classifier/test/canonical_tests/CMakeLists.txt @@ -7,7 +7,7 @@ else() file(COPY ${ERRMSG} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}) endif() endif() -add_executable(canonizer canonizer.c) +add_executable(canonizer canonizer.c ${CMAKE_SOURCE_DIR}/server/core/random_jkiss.c) target_link_libraries(canonizer pthread query_classifier z dl ssl aio crypt crypto rt m ${EMBEDDED_LIB} fullcore stdc++) add_test(NAME Internal-TestCanonicalQuery COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/canontest.sh ${CMAKE_CURRENT_BINARY_DIR}/test.log diff --git a/server/core/CMakeLists.txt b/server/core/CMakeLists.txt index 28dd5a873..22e407268 100644 --- a/server/core/CMakeLists.txt +++ b/server/core/CMakeLists.txt @@ -1,5 +1,5 @@ if(BUILD_TESTS OR BUILD_TOOLS) - add_library(fullcore STATIC adminusers.c atomic.c config.c buffer.c dbusers.c dcb.c filter.c gwbitmask.c gw_utils.c hashtable.c hint.c housekeeper.c load_utils.c memlog.c modutil.c monitor.c poll.c resultset.c secrets.c server.c service.c session.c spinlock.c thread.c users.c utils.c gwdirs.c externcmd.c maxscale_pcre2.c) + add_library(fullcore STATIC random_jkiss.c adminusers.c atomic.c config.c buffer.c dbusers.c dcb.c filter.c gwbitmask.c gw_utils.c hashtable.c hint.c housekeeper.c load_utils.c memlog.c modutil.c monitor.c poll.c resultset.c secrets.c server.c service.c session.c spinlock.c thread.c users.c utils.c gwdirs.c externcmd.c maxscale_pcre2.c) if(WITH_JEMALLOC) target_link_libraries(fullcore ${JEMALLOC_LIBRARIES}) elseif(WITH_TCMALLOC) @@ -12,7 +12,7 @@ add_executable(maxscale atomic.c buffer.c spinlock.c gateway.c gw_utils.c utils.c dcb.c load_utils.c session.c service.c server.c poll.c config.c users.c hashtable.c dbusers.c thread.c gwbitmask.c monitor.c adminusers.c secrets.c filter.c modutil.c hint.c - housekeeper.c memlog.c resultset.c gwdirs.c externcmd.c maxscale_pcre2.c) + housekeeper.c memlog.c resultset.c gwdirs.c externcmd.c random_jkiss.c maxscale_pcre2.c) if(WITH_JEMALLOC) target_link_libraries(maxscale ${JEMALLOC_LIBRARIES}) @@ -23,11 +23,11 @@ endif() target_link_libraries(maxscale ${EMBEDDED_LIB} ${PCRE_LINK_FLAGS} ${CURL_LIBRARIES} log_manager utils ssl aio pthread crypt dl crypto inih z rt m stdc++) install(TARGETS maxscale DESTINATION ${MAXSCALE_BINDIR}) -add_executable(maxkeys maxkeys.c secrets.c utils.c gwdirs.c ${CMAKE_SOURCE_DIR}/log_manager/log_manager.cc) +add_executable(maxkeys maxkeys.c spinlock.c secrets.c utils.c gwdirs.c random_jkiss.c ${CMAKE_SOURCE_DIR}/log_manager/log_manager.cc) target_link_libraries(maxkeys utils pthread crypt crypto) install(TARGETS maxkeys DESTINATION ${MAXSCALE_BINDIR}) -add_executable(maxpasswd maxpasswd.c secrets.c utils.c gwdirs.c ${CMAKE_SOURCE_DIR}/log_manager/log_manager.cc) +add_executable(maxpasswd maxpasswd.c spinlock.c secrets.c utils.c gwdirs.c random_jkiss.c ${CMAKE_SOURCE_DIR}/log_manager/log_manager.cc) target_link_libraries(maxpasswd utils pthread crypt crypto) install(TARGETS maxpasswd DESTINATION ${MAXSCALE_BINDIR}) diff --git a/server/core/buffer.c b/server/core/buffer.c index dca4ffb69..e8a1d9100 100644 --- a/server/core/buffer.c +++ b/server/core/buffer.c @@ -35,7 +35,9 @@ * 15/07/2014 Mark Riddoch Addition of properties * 28/08/2014 Mark Riddoch Adition of tail pointer to speed * the gwbuf_append process - * + * 09/11/2015 Martin Brampton Add buffer tracing (conditional compilation), + * accessed by "show buffers" maxadmin command + * * @endverbatim */ #include @@ -47,10 +49,23 @@ #include #include +#if defined(BUFFER_TRACE) +#include +#include + +static HASHTABLE *buffer_hashtable = NULL; +#endif + static buffer_object_t* gwbuf_remove_buffer_object( GWBUF* buf, buffer_object_t* bufobj); +#if defined(BUFFER_TRACE) +static void gwbuf_add_to_hashtable(GWBUF *buf); +static int bhashfn (void *key); +static int bcmpfn (void *key1, void *key2); +static void gwbuf_remove_from_hashtable(GWBUF *buf); +#endif /** * Allocate a new gateway buffer structure of size bytes. @@ -114,9 +129,111 @@ retblock: "Error : Memory allocation failed due to %s.", strerror_r(errno, errbuf, sizeof(errbuf))))); } +#if defined(BUFFER_TRACE) + else + { + gwbuf_add_to_hashtable(rval); + } +#endif return rval; } +#if defined(BUFFER_TRACE) +/** + * Store a trace of buffer creation + * + * @param buf The buffer to record + */ +static void +gwbuf_add_to_hashtable(GWBUF *buf) +{ + void *array[16]; + size_t size, i, total; + char **strings; + char *tracetext; + + size = backtrace (array, 16); + strings = backtrace_symbols (array, size); + total = (2 * size) + 1; + for (i = 0; i < size; i++) + { + total += strlen(strings[i]); + } + tracetext = (char *)malloc(total); + if (tracetext) + { + char *ptr = tracetext; + for (i = 0; i < size; i++) + { + sprintf(ptr, "\t%s\n", strings[i]); + ptr += (strlen(strings[i]) + 2); + } + free (strings); + + if (NULL == buffer_hashtable) + { + buffer_hashtable = hashtable_alloc(10000, bhashfn, bcmpfn); + hashtable_memory_fns(buffer_hashtable,NULL,NULL,NULL,(HASHMEMORYFN)free); + } + hashtable_add(buffer_hashtable, buf, (void *)tracetext); + } +} + +/** + * Hash a buffer (address) to an integer + * + * @param key The pointer to the buffer + */ +static int +bhashfn(void *key) +{ + return (int)((uintptr_t) key % INT_MAX); +} + +/** + * Compare two buffer keys (pointers) + * + * @param key1 The pointer to the first buffer + * @param key2 The pointer to the second buffer + */ +static int +bcmpfn(void *key1, void *key2) +{ + return key1 == key2 ? 0 : 1; +} + +/** + * Remove a buffer from the store of buffer traces + * + * @param buf The buffer to be removed + */ +static void +gwbuf_remove_from_hashtable(GWBUF *buf) +{ + hashtable_delete(buffer_hashtable, buf); +} + +/** + * Print all buffer traces via a given print DCB + * + * @param pdcb Print DCB for output + */ +void +dprintAllBuffers(void *pdcb) +{ + void *buf; + char *backtrace; + HASHITERATOR *buffers = hashtable_iterator(buffer_hashtable); + while (NULL != (buf = hashtable_next(buffers))) + { + dcb_printf((DCB *)pdcb, "Buffer: %p\n", (void *)buf); + backtrace = hashtable_fetch(buffer_hashtable, buf); + dcb_printf((DCB *)pdcb, "%s", backtrace); + } + hashtable_iterator_free(buffers); +} +#endif + /** * Free a gateway buffer * @@ -157,6 +274,9 @@ BUF_PROPERTY *prop; buf->hint = buf->hint->next; hint_free(h); } +#if defined(BUFFER_TRACE) + gwbuf_remove_from_hashtable(buf); +#endif free(buf); } @@ -196,6 +316,9 @@ GWBUF *rval; rval->tail = rval; rval->next = NULL; CHK_GWBUF(rval); +#if defined(BUFFER_TRACE) + gwbuf_add_to_hashtable(rval); +#endif return rval; } @@ -263,6 +386,9 @@ GWBUF *gwbuf_clone_portion( clonebuf->next = NULL; clonebuf->tail = clonebuf; CHK_GWBUF(clonebuf); +#if defined(BUFFER_TRACE) + gwbuf_add_to_hashtable(clonebuf); +#endif return clonebuf; } diff --git a/server/core/dbusers.c b/server/core/dbusers.c index a4e48afa0..593894158 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -1961,6 +1961,7 @@ static void *uh_keydup(void* key) { if (current_key->resource) rval->resource = strdup(current_key->resource); + else rval->resource = NULL; return (void *) rval; } diff --git a/server/core/dcb.c b/server/core/dcb.c index ac7646ade..7dafccfd9 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -55,6 +55,10 @@ * fixes for various error situations, * remove dcb_set_state etc, simplifications. * 10/07/2015 Martin Brampton Simplify, merge dcb_read and dcb_read_n + * 04/09/2015 Martin Brampton Changes to ensure DCB always has session pointer + * 28/09/2015 Martin Brampton Add counters, maxima for DCBs and zombies + * 29/05/2015 Martin Brampton Impose locking in dcb_call_foreach callbacks + * 17/10/2015 Martin Brampton Add hangup for each and bitmask display MaxAdmin * * @endverbatim */ @@ -83,7 +87,11 @@ #define SSL_ERRBUF_LEN 140 static DCB *allDCBs = NULL; /* Diagnostics need a list of DCBs */ +static int nDCBs = 0; +static int maxDCBs = 0; static DCB *zombies = NULL; +static int nzombies = 0; +static int maxzombies = 0; static SPINLOCK dcbspin = SPINLOCK_INIT; static SPINLOCK zombiespin = SPINLOCK_INIT; @@ -96,7 +104,7 @@ static int dcb_null_auth(DCB *dcb, SERVER *server, SESSION *session, GWBUF *buf static inline int dcb_isvalid_nolock(DCB *dcb); static inline DCB * dcb_find_in_list(DCB *dcb); static inline void dcb_process_victim_queue(DCB *listofdcb); -static void dcb_close_finish(DCB *); +static void dcb_stop_polling_and_shutdown (DCB *dcb); static bool dcb_maybe_add_persistent(DCB *); static inline bool dcb_write_parameter_check(DCB *dcb, GWBUF *queue); #if defined(FAKE_CODE) @@ -223,29 +231,23 @@ DCB *newdcb; ptr = ptr->next; ptr->next = newdcb; } + nDCBs++; + if (nDCBs > maxDCBs) maxDCBs = nDCBs; spinlock_release(&dcbspin); return newdcb; } /** - * Free a DCB that has not been associated with a descriptor. + * Provided only for consistency, simply calls dcb_close to guarantee + * safe disposal of a DCB * * @param dcb The DCB to free */ void dcb_free(DCB *dcb) { - if (dcb->fd != DCBFD_CLOSED) - { - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Attempt to free a DCB via dcb_free " - "that has been associated with a descriptor."))); - } - raise(SIGABRT); - /* Another statement to avoid a compiler warning */ - dcb_final_free(dcb); + dcb_close(dcb); } /* @@ -295,10 +297,10 @@ dcb_final_free(DCB *dcb) { DCB_CALLBACK *cb; - CHK_DCB(dcb); - ss_info_dassert(dcb->state == DCB_STATE_DISCONNECTED || - dcb->state == DCB_STATE_ALLOC, - "dcb not in DCB_STATE_DISCONNECTED not in DCB_STATE_ALLOC state."); + CHK_DCB(dcb); + ss_info_dassert(dcb->state == DCB_STATE_DISCONNECTED || + dcb->state == DCB_STATE_ALLOC, + "dcb not in DCB_STATE_DISCONNECTED not in DCB_STATE_ALLOC state."); if (DCB_POLL_BUSY(dcb)) { @@ -332,55 +334,59 @@ dcb_final_free(DCB *dcb) if (ptr) ptr->next = dcb->next; } - spinlock_release(&dcbspin); + nDCBs--; + spinlock_release(&dcbspin); - if (dcb->session) { - /*< - * Terminate client session. - */ - { - SESSION *local_session = dcb->session; - dcb->session = NULL; - CHK_SESSION(local_session); - /** - * Set session's client pointer NULL so that other threads - * won't try to call dcb_close for client DCB - * after this call. - */ - if (local_session->client == dcb) - { - spinlock_acquire(&local_session->ses_lock); - local_session->client = NULL; - spinlock_release(&local_session->ses_lock); - } - session_free(local_session); - } + if (dcb->session) { + /*< + * Terminate client session. + */ + SESSION *local_session = dcb->session; + dcb->session = NULL; + CHK_SESSION(local_session); + /** + * Set session's client pointer NULL so that other threads + * won't try to call dcb_close for client DCB + * after this call. + */ + if (local_session->client == dcb) + { + spinlock_acquire(&local_session->ses_lock); + local_session->client = NULL; + spinlock_release(&local_session->ses_lock); + } + if (SESSION_STATE_DUMMY != local_session->state) + { + session_free(local_session); + } } if (dcb->protocol && (!DCB_IS_CLONE(dcb))) free(dcb->protocol); - if (dcb->protoname) - free(dcb->protoname); + if (dcb->protoname) + free(dcb->protoname); if (dcb->remote) free(dcb->remote); if (dcb->user) free(dcb->user); - /* Clear write and read buffers */ - if (dcb->delayq) { - GWBUF *queue = dcb->delayq; - while ((queue = gwbuf_consume(queue, GWBUF_LENGTH(queue))) != NULL); - } + /* Clear write and read buffers */ + if (dcb->delayq) { + GWBUF *queue = dcb->delayq; + while ((queue = gwbuf_consume(queue, GWBUF_LENGTH(queue))) != NULL); + dcb->delayq = NULL; + } if (dcb->writeq) { GWBUF *queue = dcb->writeq; while ((queue = gwbuf_consume(queue, GWBUF_LENGTH(queue))) != NULL); dcb->writeq = NULL; } - if (dcb->dcb_readqueue) - { - GWBUF* queue = dcb->dcb_readqueue; - while ((queue = gwbuf_consume(queue, GWBUF_LENGTH(queue))) != NULL); - } + if (dcb->dcb_readqueue) + { + GWBUF* queue = dcb->dcb_readqueue; + while ((queue = gwbuf_consume(queue, GWBUF_LENGTH(queue))) != NULL); + dcb->dcb_readqueue = NULL; + } spinlock_acquire(&dcb->cb_lock); while ((cb = dcb->callbacks) != NULL) @@ -410,10 +416,9 @@ dcb_final_free(DCB *dcb) DCB * dcb_process_zombies(int threadid) { -DCB *zombiedcb, *previousdcb; +DCB *zombiedcb; +DCB *previousdcb = NULL, *nextdcb; DCB *listofdcb = NULL; -DCB *dcb = NULL; -bool succp = false; /** * Perform a dirty read to see if there is anything in the queue. @@ -423,7 +428,9 @@ bool succp = false; * dcb_final_free. */ if (!zombies) + { return NULL; + } /* * Process the zombie queue and create a list of DCB's that can be @@ -436,11 +443,10 @@ bool succp = false; */ spinlock_acquire(&zombiespin); zombiedcb = zombies; - previousdcb = NULL; while (zombiedcb) { CHK_DCB(zombiedcb); - + nextdcb = zombiedcb->memdata.next; /* * Skip processing of DCB's that are * in the event queue waiting to be processed. @@ -448,7 +454,6 @@ bool succp = false; if (zombiedcb->evq.next || zombiedcb->evq.prev) { previousdcb = zombiedcb; - zombiedcb = zombiedcb->memdata.next; } else { @@ -467,22 +472,25 @@ bool succp = false; * queue or NULL if the DCB is at the head of the * queue. Remove zombiedcb from the zombies list. */ - if (previousdcb == NULL) + if (NULL == previousdcb) + { zombies = zombiedcb->memdata.next; - else + } + else + { previousdcb->memdata.next = zombiedcb->memdata.next; + } LOGIF(LD, (skygw_log_write_flush( LOGFILE_DEBUG, - "%lu [dcb_process_zombies] Remove dcb " + "%lu [%s] Remove dcb " "%p fd %d in state %s from the " "list of zombies.", pthread_self(), + __func__, zombiedcb, zombiedcb->fd, - STRDCBSTATE(zombiedcb->state)))); - ss_info_dassert(zombiedcb->state == DCB_STATE_ZOMBIE, - "dcb not in DCB_STATE_ZOMBIE state."); + STRDCBSTATE(zombiedcb->state)))); /*< * Move zombie dcb to linked list of victim dcbs. * The variable dcb is used to hold the last DCB @@ -491,33 +499,25 @@ bool succp = false; * (listofdcb) is not NULL, then it follows that * dcb will also not be null. */ - if (listofdcb == NULL) - { - listofdcb = zombiedcb; - } - else - { - dcb->memdata.next = zombiedcb; - } - /* Set dcb for next iteration of loop */ - dcb = zombiedcb; - zombiedcb = zombiedcb->memdata.next; - /* After we've moved zombiedcb forward, set - link to null as dcb is last of the new list */ - dcb->memdata.next = NULL; + nzombies--; + zombiedcb->memdata.next = listofdcb; + listofdcb = zombiedcb; } else { - /* Since we didn't remove this dcb from the zombies - list, we need to advance the previous pointer */ + /* Since we didn't remove this dcb from the zombies + list, we need to advance the previous pointer */ previousdcb = zombiedcb; - zombiedcb = zombiedcb->memdata.next; } } + zombiedcb = nextdcb; } spinlock_release(&zombiespin); - dcb_process_victim_queue(listofdcb); + if (listofdcb) + { + dcb_process_victim_queue(listofdcb); + } return zombies; } @@ -534,12 +534,64 @@ bool succp = false; static inline void dcb_process_victim_queue(DCB *listofdcb) { - DCB *dcb; + DCB *dcb = listofdcb; - dcb = listofdcb; while (dcb != NULL) { - DCB *nextdcb = NULL; + DCB *nextdcb; + /*< + * Stop dcb's listening and modify state accordingly. + */ + spinlock_acquire(&dcb->dcb_initlock); + if (dcb->state == DCB_STATE_POLLING || dcb->state == DCB_STATE_LISTENING) + { + if (dcb->state == DCB_STATE_LISTENING) + { + LOGIF(LE, (skygw_log_write( + LOGFILE_ERROR, + "%lu [%s] 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(), + __func__, + dcb, + STRDCBSTATE(dcb->state)))); + } + else { + /* Must be DCB_STATE_POLLING */ + spinlock_release(&dcb->dcb_initlock); + if (0 == dcb->persistentstart && dcb_maybe_add_persistent(dcb)) + { + /* Have taken DCB into persistent pool, no further killing */ + dcb = dcb->memdata.next; + continue; + } + else + { + DCB *nextdcb; + dcb_stop_polling_and_shutdown(dcb); + spinlock_acquire(&zombiespin); + bitmask_copy(&dcb->memdata.bitmask, poll_bitmask()); + nextdcb = dcb->memdata.next; + dcb->memdata.next = zombies; + zombies = dcb; + nzombies++; + if (nzombies > maxzombies) maxzombies = nzombies; + spinlock_release(&zombiespin); + dcb = nextdcb; + continue; + } + } + } + /* + * Into the final close logic, so if DCB is for backend server, we + * must decrement the number of current connections. + */ + if (dcb->server && 0 == dcb->persistentstart) + { + atomic_add(&dcb->server->stats.n_current, -1); + } + if (dcb->fd > 0) { /*< @@ -581,15 +633,36 @@ dcb_process_victim_queue(DCB *listofdcb) &tls_log_info.li_sesid, &tls_log_info.li_enabled_logs))); - dcb->state = DCB_STATE_DISCONNECTED; - nextdcb = dcb->memdata.next; - dcb_final_free(dcb); - dcb = nextdcb; + dcb->state = DCB_STATE_DISCONNECTED; + nextdcb = dcb->memdata.next; + spinlock_release(&dcb->dcb_initlock); + dcb_final_free(dcb); + dcb = nextdcb; } /** Reset threads session data */ LOGIF(LT, tls_log_info.li_sesid = 0); } +/** + * Remove a DCB from the poll list and trigger shutdown mechanisms. + * + * @param dcb The DCB to be processed + */ +static void +dcb_stop_polling_and_shutdown (DCB *dcb) +{ + poll_remove_dcb(dcb); + /** + * 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); +} + /** * Connect to a server * @@ -710,7 +783,6 @@ dcb_connect(SERVER *server, SESSION *session, const char *protocol) session->client, session->client->fd))); } - ss_dassert(dcb->fd == DCBFD_CLOSED); /*< must be uninitialized at this point */ /** * Successfully connected to backend. Assign file descriptor to dcb */ @@ -1355,29 +1427,28 @@ dcb_write_SSL(DCB *dcb, GWBUF *queue) #endif /* FAKE_CODE */ do { - w = gw_write_SSL (dcb->ssl, GWBUF_DATA (queue), GWBUF_LENGTH (queue)); + w = gw_write_SSL(dcb->ssl, GWBUF_DATA(queue), GWBUF_LENGTH(queue)); dcb->stats.n_writes++; if (w <= 0) { - int ssl_errno = SSL_get_error (dcb->ssl, w); - dcb_write_SSL_error_report (dcb, w, ssl_errno); + int ssl_errno = SSL_get_error(dcb->ssl, w); + dcb_write_SSL_error_report(dcb, w, ssl_errno); if (ssl_errno != SSL_ERROR_WANT_WRITE) { - atomic_add (&dcb->writeqlen, gwbuf_length (queue)); + atomic_add(&dcb->writeqlen, gwbuf_length(queue)); dcb->stats.n_buffered++; - dcb_write_tidy_up (dcb, below_water); + dcb_write_tidy_up(dcb, below_water); return 1; } #ifdef SS_DEBUG else { - skygw_log_write (LD, "SSL error: SSL_ERROR_WANT_WRITE, retrying SSL_write..."); + skygw_log_write(LD, "SSL error: SSL_ERROR_WANT_WRITE, retrying SSL_write..."); } #endif } - } - while(w <= 0); + } while(w <= 0); /** Remove written bytes from the queue */ queue = gwbuf_consume(queue, w); @@ -1705,17 +1776,6 @@ dcb_close(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"))); - - if (DCB_STATE_ZOMBIE == dcb->state) - { - return; - } - if (DCB_STATE_UNDEFINED == dcb->state || DCB_STATE_DISCONNECTED == dcb->state) { @@ -1726,7 +1786,7 @@ dcb_close(DCB *dcb) pthread_self(), dcb, STRDCBSTATE(dcb->state)))); - raise(SIGABRT); + raise(SIGABRT); } /** @@ -1738,43 +1798,44 @@ dcb_close(DCB *dcb) 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) - { - 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)))); - } - if ((dcb->state == DCB_STATE_POLLING && !dcb_maybe_add_persistent(dcb)) - || (dcb->state == DCB_STATE_LISTENING)) - { - dcb_close_finish(dcb); - } - } - spinlock_acquire(&zombiespin); - if (dcb->state == DCB_STATE_NOPOLLING || dcb->state == DCB_STATE_ALLOC) + /* + * If DCB is in persistent pool, mark it as an error and exit + */ + if (dcb->persistentstart > 0) { + dcb->dcb_errhandle_called = true; + return; + } + + spinlock_acquire(&zombiespin); + if (!dcb->dcb_is_zombie) + { + if (0 == dcb->persistentstart && dcb->server && DCB_STATE_POLLING == dcb->state) + { + /* May be a candidate for persistence, so save user name */ + char *user; + user = session_getUser(dcb->session); + if (user && strlen(user) && !dcb->user) + { + dcb->user = strdup(user); + } + } /*< - * Add closing dcb to the top of the list. + * Add closing dcb to the top of the list, setting zombie marker */ + dcb->dcb_is_zombie = true; dcb->memdata.next = zombies; zombies = dcb; - /*< - * Set state which indicates that it has been added to zombies - * list. - */ - dcb->state = DCB_STATE_ZOMBIE; + nzombies++; + if (nzombies > maxzombies) maxzombies = nzombies; + /*< Set bit for each maxscale thread. This should be done before + * the state is changed, so as to protect the DCB from premature + * destruction. */ + if (dcb->server) + { + bitmask_copy(&dcb->memdata.bitmask, poll_bitmask()); + } } spinlock_release(&zombiespin); } @@ -1789,25 +1850,44 @@ dcb_close(DCB *dcb) static bool dcb_maybe_add_persistent(DCB *dcb) { - char *user; int poolcount = -1; - user = session_getUser(dcb->session); - if (user - && strlen(user) + if (dcb->user != NULL + && strlen(dcb->user) && dcb->server && dcb->server->persistpoolmax + && (dcb->server->status & SERVER_RUNNING) && !dcb->dcb_errhandle_called && !(dcb->flags & DCBF_HUNG) && (poolcount = dcb_persistent_clean_count(dcb, false)) < dcb->server->persistpoolmax) { + DCB_CALLBACK *loopcallback; LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, "%lu [dcb_maybe_add_persistent] Adding DCB to persistent pool, user %s.\n", pthread_self(), - user))); - dcb->user = strdup(user); + dcb->user))); + dcb->dcb_is_zombie = false; dcb->persistentstart = time(NULL); - session_unlink_dcb(dcb->session, dcb); + if (dcb->session) + /*< + * Terminate client session. + */ + { + SESSION *local_session = dcb->session; + session_set_dummy(dcb); + CHK_SESSION(local_session); + if (SESSION_STATE_DUMMY != local_session->state) + { + session_free(local_session); + } + } + spinlock_acquire(&dcb->cb_lock); + while ((loopcallback = dcb->callbacks) != NULL) + { + dcb->callbacks = loopcallback->next; + free(loopcallback); + } + spinlock_release(&dcb->cb_lock); spinlock_acquire(&dcb->server->persistlock); dcb->nextpersistent = dcb->server->persistent; dcb->server->persistent = dcb; @@ -1820,58 +1900,21 @@ dcb_maybe_add_persistent(DCB *dcb) { LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, - "%lu [dcb_maybe_add_persistent] Not adding DCB %p to persistent pool, user %s, " - "max for pool %d, error handle called %s, hung flag %s, pool count %d.\n", + "%lu [dcb_maybe_add_persistent] Not adding DCB %p to persistent pool, " + "user %s, max for pool %d, error handle called %s, hung flag %s, " + "server status %d, pool count %d.\n", pthread_self(), dcb, - user ? user : "", + dcb->user ? dcb->user : "", (dcb->server && dcb->server->persistpoolmax) ? dcb->server->persistpoolmax : 0, dcb->dcb_errhandle_called ? "true" : "false", (dcb->flags & DCBF_HUNG) ? "true" : "false", + dcb->server ? dcb->server->status : 0, poolcount))); } return false; } -/** - * Final calls for DCB close - * - * @param dcb The DCB to print - * - */ -static void -dcb_close_finish(DCB *dcb) -{ - 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)))); - /** - * Do a consistency check, then adjust counter if not from persistent pool - */ - if (dcb->server) - { - if (dcb->server->persistent) CHK_DCB(dcb->server->persistent); - if (0 == dcb->persistentstart) atomic_add(&dcb->server->stats.n_current, -1); - } - /** - * 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); - } - /** * Diagnostic to print a DCB * @@ -1967,6 +2010,15 @@ dprintOneDCB(DCB *pdcb, DCB *dcb) if (dcb->remote) dcb_printf(pdcb, "\tConnected to: %s\n", dcb->remote); + if (dcb->server) + { + if (dcb->server->name) + dcb_printf(pdcb, "\tServer name/IP: %s\n", + dcb->server->name); + if (dcb->server->port) + dcb_printf(pdcb, "\tPort number: %d\n", + dcb->server->port); + } if (dcb->user) dcb_printf(pdcb, "\tUsername: %s\n", dcb->user); @@ -1988,6 +2040,15 @@ dprintOneDCB(DCB *pdcb, DCB *dcb) dcb_printf(pdcb, "\tRole: %s\n", rolename); free(rolename); } + if (!bitmask_isallclear(&dcb->memdata.bitmask)) + { + char *bitmasktext = bitmask_render_readable(&dcb->memdata.bitmask); + if (bitmasktext) + { + dcb_printf(pdcb, "\tBitMask: %s\n", bitmasktext); + free(bitmasktext); + } + } dcb_printf(pdcb, "\tStatistics:\n"); dcb_printf(pdcb, "\t\tNo. of Reads: %d\n", dcb->stats.n_reads); dcb_printf(pdcb, "\t\tNo. of Writes: %d\n", dcb->stats.n_writes); @@ -2197,13 +2258,11 @@ gw_dcb_state2string (int state) case DCB_STATE_POLLING: return "DCB in the polling loop"; case DCB_STATE_NOPOLLING: - return "DCB not in the polling loop"; + return "DCB not in polling loop"; case DCB_STATE_LISTENING: return "DCB for listening socket"; case DCB_STATE_DISCONNECTED: return "DCB socket closed"; - case DCB_STATE_FREED: - return "DCB memory could be freed"; case DCB_STATE_ZOMBIE: return "DCB Zombie"; case DCB_STATE_UNDEFINED: @@ -2638,17 +2697,21 @@ dcb_call_foreach(struct server* server, DCB_REASON reason) case DCB_REASON_NOT_RESPONDING: { DCB *dcb; - dcb = dcb_get_next(NULL); - + spinlock_acquire(&dcbspin); + dcb = allDCBs; + while (dcb != NULL) { + spinlock_acquire(&dcb->dcb_initlock); if (dcb->state == DCB_STATE_POLLING && dcb->server && - strcmp(dcb->server->unique_name,server->unique_name) == 0) + strcmp(dcb->server->unique_name,server->unique_name) == 0) { dcb_call_callback(dcb, DCB_REASON_NOT_RESPONDING); } - dcb = dcb_get_next(dcb); + spinlock_release(&dcb->dcb_initlock); + dcb = dcb->next; } + spinlock_release(&dcbspin); break; } @@ -2658,6 +2721,36 @@ dcb_call_foreach(struct server* server, DCB_REASON reason) return; } +/** + * Call all the callbacks on all DCB's that match the server and the reason given + * + * @param reason The DCB_REASON that triggers the callback + */ +void +dcb_hangup_foreach(struct server* server) +{ + LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG, + "%lu [dcb_hangup_foreach]", + pthread_self()))); + + DCB *dcb; + spinlock_acquire(&dcbspin); + dcb = allDCBs; + + while (dcb != NULL) + { + spinlock_acquire(&dcb->dcb_initlock); + if (dcb->state == DCB_STATE_POLLING && dcb->server && + strcmp(dcb->server->unique_name,server->unique_name) == 0) + { + poll_fake_hangup_event(dcb); + } + spinlock_release(&dcb->dcb_initlock); + dcb = dcb->next; + } + spinlock_release(&dcbspin); +} + /** * Null protocol write routine used for cloned dcb's. It merely consumes @@ -2731,9 +2824,11 @@ dcb_persistent_clean_count(DCB *dcb, bool cleanall) CHK_DCB(persistentdcb); nextdcb = persistentdcb->nextpersistent; if (cleanall - || persistentdcb-> dcb_errhandle_called - || count >= server->persistpoolmax - || (time(NULL) - persistentdcb->persistentstart) > server->persistmaxtime) + || persistentdcb-> dcb_errhandle_called + || count >= server->persistpoolmax + || persistentdcb->server == NULL + || !(persistentdcb->server->status & SERVER_RUNNING) + || (time(NULL) - persistentdcb->persistentstart) > server->persistmaxtime) { /* Remove from persistent pool */ if (previousdcb) { @@ -2761,7 +2856,11 @@ dcb_persistent_clean_count(DCB *dcb, bool cleanall) while (disposals) { nextdcb = disposals->nextpersistent; - dcb_close_finish(disposals); + disposals->persistentstart = -1; + if (DCB_STATE_POLLING == disposals->state) + { + dcb_stop_polling_and_shutdown(disposals); + } dcb_close(disposals); disposals = nextdcb; } diff --git a/server/core/gwbitmask.c b/server/core/gwbitmask.c index 765864d72..0068c846f 100644 --- a/server/core/gwbitmask.c +++ b/server/core/gwbitmask.c @@ -17,6 +17,7 @@ */ #include #include +#include #include /** @@ -47,10 +48,14 @@ * Date Who Description * 28/06/13 Mark Riddoch Initial implementation * 20/08/15 Martin Brampton Added caveats about limitations (above) + * 17/10/15 Martin Brampton Added display of bitmask * * @endverbatim */ +static int bitmask_isset_without_spinlock(GWBITMASK *bitmask, int bit); +static int bitmask_count_bits_set(GWBITMASK *bitmask); + /** * Initialise a bitmask * @@ -151,19 +156,42 @@ unsigned char mask; * Return a non-zero value if the bit at the specified bit * position in the bitmask is set. * The bitmask will automatically be extended if the bit is - * beyond the current bitmask length. This could be optimised - * by assuming that a bit beyond the length is unset. + * beyond the current bitmask length. The work is done in the function + * bitmask_isset_without_spinlock, which can be called when a spinlock + * has already been acquired. * * @param bitmask Pointer the bitmask - * @param bit Bit to clear + * @param bit Bit to test */ int bitmask_isset(GWBITMASK *bitmask, int bit) { +int result; + + spinlock_acquire(&bitmask->lock); + result = bitmask_isset_without_spinlock(bitmask, bit); + spinlock_release(&bitmask->lock); + return result; +} + +/** + * Return a non-zero value if the bit at the specified bit + * position in the bitmask is set. Should be called while holding a + * lock on the bitmask. + * + * The bitmask will automatically be extended if the bit is + * beyond the current bitmask length. This could be optimised + * by assuming that a bit beyond the length is unset. + * + * @param bitmask Pointer the bitmask + * @param bit Bit to test + */ +static int +bitmask_isset_without_spinlock(GWBITMASK *bitmask, int bit) +{ unsigned char *ptr; unsigned char mask; - spinlock_acquire(&bitmask->lock); if (bit >= bitmask->length) { bitmask->bits = realloc(bitmask->bits, @@ -174,7 +202,6 @@ unsigned char mask; } ptr = bitmask->bits + (bit / 8); mask = 1 << (bit % 8); - spinlock_release(&bitmask->lock); return *ptr & mask; } @@ -239,3 +266,90 @@ bitmask_copy(GWBITMASK *dest, GWBITMASK *src) spinlock_release(&src->lock); } +/** + * Return a comma separated list of the numbers of the bits that are set in + * a bitmask, numbering starting at zero. Constrained to reject requests that + * could require more than three digit numbers. The returned string must be + * freed by the caller (unless it is null on account of memory allocation + * failure). + * + * @param bitmask Bitmap to make readable + * @return pointer to the newly allocated string, or null if no memory + */ +char * +bitmask_render_readable(GWBITMASK *bitmask) +{ + char *toobig = "Bitmask is too large to render readable"; + char *empty = "No bits are set"; + char onebit[5]; + char *result; + int count_set = 0; + + spinlock_acquire(&bitmask->lock); + if (999 < bitmask->length) + { + result = malloc(strlen(toobig)); + if (result) + { + strcpy(result, toobig); + } + } + else + { + count_set = bitmask_count_bits_set(bitmask); + if (count_set) + { + result = malloc(1 + (4 * count_set)); + if (result) + { + result[0] = 0; + for (int i = 0; ilength; i++) + { + if (bitmask_isset_without_spinlock(bitmask, i)) + { + sprintf(onebit, "%d,", i); + strcat(result, onebit); + } + } + result[strlen(result)-1] = 0; + } + } + else + { + result = malloc(strlen(empty)); + if (result) + { + strcpy(result, empty); + } + } + } + spinlock_release(&bitmask->lock); + return result; +} + +/** + * Return a count of the number of bits set in a bitmask. Helpful for setting + * the size of string needed to show the set bits in readable form. + * + * @param bitmask Bitmap whose bits are to be counted + * @return int Number of set bits + */ +static int +bitmask_count_bits_set(GWBITMASK *bitmask) +{ + const unsigned char oneBits[] = {0,1,1,2,1,2,2,3,1,2,2,3,2,3,3,4}; + unsigned char partresults; + int result = 0; + unsigned char *ptr, *eptr; + + ptr = bitmask->bits; + eptr = ptr + (bitmask->length / 8); + while (ptr < eptr) + { + partresults = oneBits[*ptr&0x0f]; + partresults += oneBits[*ptr>>4]; + result += partresults; + ptr++; + } + return result; +} \ No newline at end of file diff --git a/server/core/hashtable.c b/server/core/hashtable.c index 3a0becd58..b7810bf56 100644 --- a/server/core/hashtable.c +++ b/server/core/hashtable.c @@ -383,7 +383,7 @@ hashtable_fetch(HASHTABLE *table, void *key) unsigned int hashkey; HASHENTRIES *entry; - if(table == NULL || key == NULL) + if(table == NULL || key == NULL || 0 == table->hashsize) return NULL; hashkey = table->hashfn(key) % table->hashsize; @@ -766,7 +766,9 @@ char buf[40]; key = keyread(fd); value = valueread(fd); if (key == NULL || value == NULL) - break; + { + break; + } hashtable_add(table, key, value); rval++; } diff --git a/server/core/poll.c b/server/core/poll.c index 8b6e455ff..a88405029 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -69,8 +69,7 @@ int max_poll_sleep; * thread utilisation and fairer scheduling of the event * processing. * 07/07/15 Martin Brampton Simplified add and remove DCB, improve error handling. - * 23/08/15 Martin Brampton Provisionally added test so only DCB with a - * session link can be added to the poll list + * 23/08/15 Martin Brampton Added test so only DCB with a session link can be added to the poll list * * @endverbatim */ @@ -91,7 +90,7 @@ static simple_mutex_t epoll_wait_mutex; /*< serializes calls to epoll_wait */ static int n_waiting = 0; /*< No. of threads in epoll_wait */ static int process_pollq(int thread_id); static void poll_add_event_to_dcb(DCB* dcb, GWBUF* buf, __uint32_t ev); - +static bool poll_dcb_session_check(DCB *dcb, const char *); DCB *eventq = NULL; SPINLOCK pollqlock = SPINLOCK_INIT; @@ -294,23 +293,6 @@ poll_add_dcb(DCB *dcb) STRDCBSTATE(dcb->state)))); raise(SIGABRT); } - /* - * This test could be wrong. On the face of it, we don't want to add a - * DCB to the poll list if it is not linked to a session because the code - * that handles events will expect to find a session. Test added by - * Martin as an experiment on 23 August 2015 - */ - if (false && NULL == dcb->session) - { - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "%lu [%s] Error : Attempt to add dcb %p " - "to poll list but it is not linked to a session, crashing.", - __func__, - pthread_self(), - dcb))); - raise(SIGABRT); - } if (DCB_STATE_POLLING == dcb->state || DCB_STATE_LISTENING == dcb->state) { @@ -380,10 +362,6 @@ poll_remove_dcb(DCB *dcb) dcb, STRDCBSTATE(dcb->state)))); } - /*< Set bit for each maxscale thread. This should be done before - * the state is changed, so as to protect the DCB from premature - * destruction. */ - bitmask_copy(&dcb->memdata.bitmask, poll_bitmask()); /*< * Set state to NOPOLLING and remove dcb from poll set. */ @@ -877,8 +855,12 @@ unsigned long qtime; #endif /* FAKE_CODE */ ss_debug(spinlock_acquire(&dcb->dcb_initlock);) ss_dassert(dcb->state != DCB_STATE_ALLOC); - ss_dassert(dcb->state != DCB_STATE_DISCONNECTED); - ss_dassert(dcb->state != DCB_STATE_FREED); + /* It isn't obvious that this is impossible */ + /* ss_dassert(dcb->state != DCB_STATE_DISCONNECTED); */ + if (DCB_STATE_DISCONNECTED == dcb->state) + { + return 0; + } ss_debug(spinlock_release(&dcb->dcb_initlock);) LOGIF(LD, (skygw_log_write( @@ -902,7 +884,10 @@ unsigned long qtime; dcb, &tls_log_info.li_sesid, &tls_log_info.li_enabled_logs))); - dcb->func.write_ready(dcb); + if (poll_dcb_session_check(dcb, "write_ready")) + { + dcb->func.write_ready(dcb); + } } else { char errbuf[STRERROR_BUFLEN]; LOGIF(LD, (skygw_log_write( @@ -933,7 +918,10 @@ unsigned long qtime; dcb, &tls_log_info.li_sesid, &tls_log_info.li_enabled_logs))); - dcb->func.accept(dcb); + if (poll_dcb_session_check(dcb, "accept")) + { + dcb->func.accept(dcb); + } } else { @@ -950,7 +938,10 @@ unsigned long qtime; dcb, &tls_log_info.li_sesid, &tls_log_info.li_enabled_logs))); - dcb->func.read(dcb); + if (poll_dcb_session_check(dcb, "read")) + { + dcb->func.read(dcb); + } } } if (ev & EPOLLERR) @@ -987,7 +978,10 @@ unsigned long qtime; dcb, &tls_log_info.li_sesid, &tls_log_info.li_enabled_logs))); - dcb->func.error(dcb); + if (poll_dcb_session_check(dcb, "error")) + { + dcb->func.error(dcb); + } } if (ev & EPOLLHUP) @@ -1016,7 +1010,10 @@ unsigned long qtime; dcb, &tls_log_info.li_sesid, &tls_log_info.li_enabled_logs))); - dcb->func.hangup(dcb); + if (poll_dcb_session_check(dcb, "hangup EPOLLHUP")) + { + dcb->func.hangup(dcb); + } } else spinlock_release(&dcb->dcb_initlock); @@ -1049,7 +1046,10 @@ unsigned long qtime; dcb, &tls_log_info.li_sesid, &tls_log_info.li_enabled_logs))); - dcb->func.hangup(dcb); + if (poll_dcb_session_check(dcb, "hangup EPOLLRDHUP")) + { + dcb->func.hangup(dcb); + } } else spinlock_release(&dcb->dcb_initlock); @@ -1118,6 +1118,37 @@ unsigned long qtime; } /** + * + * Check that the DCB has a session link before processing. + * If not, log an error. Processing will be bypassed + * + * @param dcb The DCB to check + * @param function The name of the function about to be called + * @return bool Does the DCB have a non-null session link + */ +static bool +poll_dcb_session_check(DCB *dcb, const char *function) +{ + if (dcb->session) + { + return true; + } + else + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "%lu [%s] The dcb %p that was about to be processed by %s does not " + "have a non-null session pointer ", + pthread_self(), + __func__, + dcb, + function))); + return false; + } +} + +/** + * * Shutdown the polling loop */ void @@ -1543,6 +1574,53 @@ uint32_t ev = EPOLLOUT; spinlock_release(&pollqlock); } +/* + * Insert a fake hangup event for a DCB into the polling queue. + * + * This is used when a monitor detects that a server is not responding. + * + * @param dcb DCB to emulate an EPOLLOUT event for + */ +void +poll_fake_hangup_event(DCB *dcb) +{ +uint32_t ev = EPOLLRDHUP; + + spinlock_acquire(&pollqlock); + if (DCB_POLL_BUSY(dcb)) + { + if (dcb->evq.pending_events == 0) + pollStats.evq_pending++; + dcb->evq.pending_events |= ev; + } + else + { + dcb->evq.pending_events = ev; + dcb->evq.inserted = hkheartbeat; + if (eventq) + { + dcb->evq.prev = eventq->evq.prev; + eventq->evq.prev->evq.next = dcb; + eventq->evq.prev = dcb; + dcb->evq.next = eventq; + } + else + { + eventq = dcb; + dcb->evq.prev = dcb; + dcb->evq.next = dcb; + } + pollStats.evq_length++; + pollStats.evq_pending++; + dcb->evq.inserted = hkheartbeat; + if (pollStats.evq_length > pollStats.evq_max) + { + pollStats.evq_max = pollStats.evq_length; + } + } + spinlock_release(&pollqlock); +} + /** * Print the event queue contents * diff --git a/server/core/random_jkiss.c b/server/core/random_jkiss.c new file mode 100644 index 000000000..97a09fa34 --- /dev/null +++ b/server/core/random_jkiss.c @@ -0,0 +1,131 @@ +/* + * This file is distributed as part of the MariaDB Corporation MaxScale. It is free + * software: you can redistribute it and/or modify it under the terms of the + * GNU General Public License as published by the Free Software Foundation, + * version 2. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more + * details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 51 + * Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Copyright MariaDB Corporation Ab 2013-2014 + */ + +/** + * @file random_jkiss.c - Random number generator for the MariaDB Corporation MaxScale + * + * See http://www0.cs.ucl.ac.uk/staff/d.jones/GoodPracticeRNG.pdf for discussion of random + * number generators (RNGs). + * + * @verbatim + * Revision History + * + * Date Who Description + * 26/08/15 Martin Brampton Initial implementation + * + * @endverbatim + */ + +#include +#include +#include +#include +#include +#include +#include + +/* Public domain code for JKISS RNG - Comment header added */ + +/* If possible, the seed variables will be set from /dev/urandom but + * should that fail, these arbitrary numbers will be used as a last resort. + */ +static unsigned int x = 123456789,y = 987654321,z = 43219876,c = 6543217; /* Seed variables */ +static bool init = false; + +static SPINLOCK random_jkiss_spinlock = SPINLOCK_INIT; + +static unsigned int random_jkiss_devrand(void); +static void random_init_jkiss(void); + +/*** + * + * Return a pseudo-random number that satisfies major tests for random sequences + * + * @return uint Random number + * + */ +unsigned int +random_jkiss(void) +{ + unsigned long long t; + unsigned int result; + + spinlock_acquire(&random_jkiss_spinlock); + if (!init) + { + /* Must set init first because initialisation calls this function */ + init = true; + spinlock_release(&random_jkiss_spinlock); + random_init_jkiss(); + } + x = 314527869 * x + 1234567; + y ^= y << 5; + y ^= y >> 7; + y ^= y << 22; + t = 4294584393ULL * z + c; + c = t >> 32; + z = t; + result = x + y + z; + spinlock_release(&random_jkiss_spinlock); + return result; +} + +/* Own code adapted from http://www0.cs.ucl.ac.uk/staff/d.jones/GoodPracticeRNG.pdf */ + +/*** + * + * Obtain a seed random number from /dev/urandom if available. + * + * @return uint Random number + * + */ +static unsigned int +random_jkiss_devrand(void) +{ + int fn; + unsigned int r; + if ((fn = open("/dev/urandom", O_RDONLY)) == -1) return 0; + if (read(fn, &r, sizeof(r)) != sizeof(r)) + { + r = 0; + } + close(fn); + return r; +} + +/*** + * + * Initialise the generator using /dev/urandom if available, and warm up + * with 1000 iterations + * + */ +static void +random_init_jkiss(void) +{ + int newrand, i; + spinlock_acquire(&random_jkiss_spinlock); + if ((newrand = random_jkiss_devrand()) != 0) x = newrand; + if ((newrand = random_jkiss_devrand()) != 0) y = newrand; + if ((newrand = random_jkiss_devrand()) != 0) z = newrand; + if ((newrand = random_jkiss_devrand()) != 0) + c = newrand % 698769068 + 1; /* Should be less than 698769069 */ + spinlock_release(&random_jkiss_spinlock); + + /* "Warm up" our random number generator */ + for (i = 0; i < 100; i++) random_jkiss(); +} diff --git a/server/core/secrets.c b/server/core/secrets.c index 98318dc99..01cf66315 100644 --- a/server/core/secrets.c +++ b/server/core/secrets.c @@ -23,6 +23,7 @@ #include #include #include +#include /** * Generate a random printable character @@ -32,16 +33,13 @@ static unsigned char secrets_randomchar() { - return (char)((rand() % ('~' - ' ')) + ' '); + return(char) ((random_jkiss() % ('~' - ' ')) + ' '); } static int secrets_random_str(unsigned char *output, int len) { - int i; - srand((unsigned long )time(0L) ^ (unsigned long )output); - - for (i = 0; i < len; ++i) + for (int i = 0; i < len; ++i) { output[i] = secrets_randomchar(); } @@ -53,12 +51,12 @@ secrets_random_str(unsigned char *output, int len) * and the AES Init Vector. * If the path parameter is not null the custom path is interpreted as a folder * containing the .secrets file. Otherwise the default location is used. - * @return The keys structure or NULL on error + * @return The keys structure or NULL on error */ static MAXKEYS * secrets_readKeys(const char* path) { - char secret_file[PATH_MAX+1]; + char secret_file[PATH_MAX + 1]; char *home; MAXKEYS *keys; struct stat secret_stats; @@ -74,7 +72,6 @@ secrets_readKeys(const char* path) { snprintf(secret_file, PATH_MAX, "%s/.secrets", get_datadir()); } - /* Try to access secrets file */ if (access(secret_file, R_OK) == -1) { @@ -85,25 +82,23 @@ secrets_readKeys(const char* path) if (!reported) { char errbuf[STRERROR_BUFLEN]; - LOGIF(LM, (skygw_log_write( - LOGFILE_MESSAGE, - "Encrypted password file %s can't be accessed " - "(%s). Password encryption is not used.", - secret_file, - strerror_r(eno, errbuf, sizeof(errbuf))))); + LOGIF(LM, (skygw_log_write(LOGFILE_MESSAGE, + "Encrypted password file %s can't be accessed " + "(%s). Password encryption is not used.", + secret_file, + strerror_r(eno, errbuf, sizeof(errbuf))))); reported = 1; } } else { char errbuf[STRERROR_BUFLEN]; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : access for secrets file " - "[%s] failed. Error %d, %s.", - secret_file, - eno, - strerror_r(eno, errbuf, sizeof(errbuf))))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : access for secrets file " + "[%s] failed. Error %d, %s.", + secret_file, + eno, + strerror_r(eno, errbuf, sizeof(errbuf))))); } return NULL; } @@ -114,30 +109,29 @@ secrets_readKeys(const char* path) int eno = errno; errno = 0; char errbuf[STRERROR_BUFLEN]; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Failed opening secret " - "file [%s]. Error %d, %s.", - secret_file, - eno, - strerror_r(eno, errbuf, sizeof(errbuf))))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : Failed opening secret " + "file [%s]. Error %d, %s.", + secret_file, + eno, + strerror_r(eno, errbuf, sizeof(errbuf))))); return NULL; } /* accessing file details */ - if (fstat(fd, &secret_stats) < 0) { + if (fstat(fd, &secret_stats) < 0) + { int eno = errno; errno = 0; close(fd); char errbuf[STRERROR_BUFLEN]; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : fstat for secret file %s " - "failed. Error %d, %s.", - secret_file, - eno, - strerror_r(eno, errbuf, sizeof(errbuf))))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : fstat for secret file %s " + "failed. Error %d, %s.", + secret_file, + eno, + strerror_r(eno, errbuf, sizeof(errbuf))))); return NULL; } @@ -147,34 +141,30 @@ secrets_readKeys(const char* path) errno = 0; close(fd); char errbuf[STRERROR_BUFLEN]; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Secrets file %s has " - "incorrect size. Error %d, %s.", - secret_file, - eno, - strerror_r(eno, errbuf, sizeof(errbuf))))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : Secrets file %s has " + "incorrect size. Error %d, %s.", + secret_file, + eno, + strerror_r(eno, errbuf, sizeof(errbuf))))); + return NULL; + } + if (secret_stats.st_mode != (S_IRUSR | S_IFREG)) + { + close(fd); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : Ignoring secrets file " + "%s, invalid permissions.", + secret_file))); return NULL; } - if (secret_stats.st_mode != (S_IRUSR|S_IFREG)) + if ((keys = (MAXKEYS *) malloc(sizeof(MAXKEYS))) == NULL) { close(fd); - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Ignoring secrets file " - "%s, invalid permissions.", - secret_file))); - return NULL; - } - - if ((keys = (MAXKEYS *)malloc(sizeof(MAXKEYS))) == NULL) - { - close(fd); - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Memory allocation failed " - "for key structure."))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : Memory allocation failed " + "for key structure."))); return NULL; } @@ -191,31 +181,30 @@ secrets_readKeys(const char* path) close(fd); free(keys); char errbuf[STRERROR_BUFLEN]; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Read from secrets file " - "%s failed. Read %d, expected %d bytes. Error %d, %s.", - secret_file, - len, - sizeof(MAXKEYS), - eno, - strerror_r(eno, errbuf, sizeof(errbuf))))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : Read from secrets file " + "%s failed. Read %d, expected %d bytes. Error %d, %s.", + secret_file, + len, + sizeof(MAXKEYS), + eno, + strerror_r(eno, errbuf, sizeof(errbuf))))); return NULL; } /* Close the file */ - if (close(fd) < 0) { + if (close(fd) < 0) + { int eno = errno; errno = 0; free(keys); char errbuf[STRERROR_BUFLEN]; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Failed closing the " - "secrets file %s. Error %d, %s.", - secret_file, - eno, - strerror_r(eno, errbuf, sizeof(errbuf))))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : Failed closing the " + "secrets file %s. Error %d, %s.", + secret_file, + eno, + strerror_r(eno, errbuf, sizeof(errbuf))))); return NULL; } ss_dassert(keys != NULL); @@ -233,31 +222,30 @@ secrets_readKeys(const char* path) */ int secrets_writeKeys(const char *path) { - int fd,randfd; - unsigned int randval; - MAXKEYS key; - char secret_file[PATH_MAX + 10]; + int fd, randfd; + unsigned int randval; + MAXKEYS key; + char secret_file[PATH_MAX + 10]; if (strlen(path) > PATH_MAX) { - skygw_log_write(LOGFILE_ERROR,"Error: Pathname too long."); + skygw_log_write(LOGFILE_ERROR, "Error: Pathname too long."); return 1; } - snprintf(secret_file,PATH_MAX + 9,"%s/.secrets",path); + snprintf(secret_file, PATH_MAX + 9, "%s/.secrets", path); secret_file[PATH_MAX + 9] = '\0'; /* Open for writing | Create | Truncate the file for writing */ if ((fd = open(secret_file, O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR)) < 0) { char errbuf[STRERROR_BUFLEN]; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : failed opening secret " - "file [%s]. Error %d, %s.", - secret_file, - errno, - strerror_r(errno, errbuf, sizeof(errbuf))))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : failed opening secret " + "file [%s]. Error %d, %s.", + secret_file, + errno, + strerror_r(errno, errbuf, sizeof(errbuf))))); return 1; } @@ -265,27 +253,24 @@ int secrets_writeKeys(const char *path) if ((randfd = open("/dev/random", O_RDONLY)) < 0) { char errbuf[STRERROR_BUFLEN]; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : failed opening /dev/random. Error %d, %s.", - errno, - strerror_r(errno, errbuf, sizeof(errbuf))))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : failed opening /dev/random. Error %d, %s.", + errno, + strerror_r(errno, errbuf, sizeof(errbuf))))); close(fd); return 1; } - if (read(randfd,(void*)&randval,sizeof(unsigned int)) < 1) + if (read(randfd, (void*) &randval, sizeof(unsigned int)) < 1) { - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : failed to read /dev/random."))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : failed to read /dev/random."))); close(fd); close(randfd); return 1; } close(randfd); - srand(randval); secrets_random_str(key.enckey, MAXSCALE_KEYLEN); secrets_random_str(key.initvector, MAXSCALE_IV_LEN); @@ -293,13 +278,12 @@ int secrets_writeKeys(const char *path) if (write(fd, &key, sizeof(key)) < 0) { char errbuf[STRERROR_BUFLEN]; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : failed writing into " - "secret file [%s]. Error %d, %s.", - secret_file, - errno, - strerror_r(errno, errbuf, sizeof(errbuf))))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : failed writing into " + "secret file [%s]. Error %d, %s.", + secret_file, + errno, + strerror_r(errno, errbuf, sizeof(errbuf))))); close(fd); return 1; } @@ -308,25 +292,23 @@ int secrets_writeKeys(const char *path) if (close(fd) < 0) { char errbuf[STRERROR_BUFLEN]; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : failed closing the " - "secret file [%s]. Error %d, %s.", - secret_file, - errno, - strerror_r(errno, errbuf, sizeof(errbuf))))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : failed closing the " + "secret file [%s]. Error %d, %s.", + secret_file, + errno, + strerror_r(errno, errbuf, sizeof(errbuf))))); } if (chmod(secret_file, S_IRUSR) < 0) { char errbuf[STRERROR_BUFLEN]; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : failed to change the permissions of the" - "secret file [%s]. Error %d, %s.", - secret_file, - errno, - strerror_r(errno, errbuf, sizeof(errbuf))))); + LOGIF(LE, (skygw_log_write_flush(LOGFILE_ERROR, + "Error : failed to change the permissions of the" + "secret file [%s]. Error %d, %s.", + secret_file, + errno, + strerror_r(errno, errbuf, sizeof(errbuf))))); } return 0; @@ -341,17 +323,17 @@ int secrets_writeKeys(const char *path) * Note the return is always a malloc'd string that the caller must free * * @param crypt The encrypted password - * @return The decrypted password + * @return The decrypted password */ char * decryptPassword(const char *crypt) { - MAXKEYS *keys; - AES_KEY aeskey; - unsigned char *plain; - const char *ptr; - unsigned char encrypted[80]; - int enlen; + MAXKEYS *keys; + AES_KEY aeskey; + unsigned char *plain; + const char *ptr; + unsigned char encrypted[80]; + int enlen; keys = secrets_readKeys(NULL); if (!keys) @@ -359,9 +341,9 @@ decryptPassword(const char *crypt) return strdup(crypt); } /* - ** If the input is not a HEX string return the input - ** it probably was not encrypted - */ + ** If the input is not a HEX string return the input + ** it probably was not encrypted + */ for (ptr = crypt; *ptr; ptr++) { if (!isxdigit(*ptr)) @@ -374,7 +356,7 @@ decryptPassword(const char *crypt) enlen = strlen(crypt) / 2; gw_hex2bin(encrypted, crypt, strlen(crypt)); - if ((plain = (unsigned char *)malloc(80)) == NULL) + if ((plain = (unsigned char *) malloc(80)) == NULL) { free(keys); return NULL; @@ -385,26 +367,26 @@ decryptPassword(const char *crypt) AES_cbc_encrypt(encrypted, plain, enlen, &aeskey, keys->initvector, AES_DECRYPT); free(keys); - return (char *)plain; + return(char *) plain; } /** * Encrypt a password that can be stored in the MaxScale configuration file. * * Note the return is always a malloc'd string that the caller must free - * @param path Path the the .secrets file - * @param password The password to encrypt - * @return The encrypted password + * @param path Path the the .secrets file + * @param password The password to encrypt + * @return The encrypted password */ char * encryptPassword(const char* path, const char *password) { - MAXKEYS *keys; - AES_KEY aeskey; - int padded_len; - char *hex_output; - unsigned char padded_passwd[80]; - unsigned char encrypted[80]; + MAXKEYS *keys; + AES_KEY aeskey; + int padded_len; + char *hex_output; + unsigned char padded_passwd[80]; + unsigned char encrypted[80]; if ((keys = secrets_readKeys(path)) == NULL) { @@ -412,13 +394,13 @@ encryptPassword(const char* path, const char *password) } memset(padded_passwd, 0, 80); - strncpy((char *)padded_passwd, password, 79); + strncpy((char *) padded_passwd, password, 79); padded_len = ((strlen(password) / AES_BLOCK_SIZE) + 1) * AES_BLOCK_SIZE; AES_set_encrypt_key(keys->enckey, 8 * MAXSCALE_KEYLEN, &aeskey); AES_cbc_encrypt(padded_passwd, encrypted, padded_len, &aeskey, keys->initvector, AES_ENCRYPT); - hex_output = (char *)malloc(padded_len * 2); + hex_output = (char *) malloc(padded_len * 2); gw_bin2hex(hex_output, encrypted, padded_len); free(keys); diff --git a/server/core/server.c b/server/core/server.c index 5d5a42f6e..dad510c1c 100644 --- a/server/core/server.c +++ b/server/core/server.c @@ -150,7 +150,10 @@ server_get_persistent(SERVER *server, char *user, const char *protocol) { DCB *dcb, *previous = NULL; - if (server->persistent && dcb_persistent_clean_count(server->persistent, false) && server->persistent) + if (server->persistent + && dcb_persistent_clean_count(server->persistent, false) + && server->persistent + && (server->status & SERVER_RUNNING)) { spinlock_acquire(&server->persistlock); dcb = server->persistent; diff --git a/server/core/service.c b/server/core/service.c index ccc599748..0bacd9364 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -258,8 +258,7 @@ GWPROTOCOL *funcs; } if (loaded == -1) { - hashtable_free(service->users->data); - free(service->users); + users_free(service->users); service->users = NULL; dcb_close(port->listener); port->listener = NULL; @@ -347,6 +346,7 @@ GWPROTOCOL *funcs; == NULL) { users_free(service->users); + service->users = NULL; dcb_close(port->listener); service->users = NULL; port->listener = NULL; @@ -359,7 +359,6 @@ GWPROTOCOL *funcs; goto retblock; } memcpy(&(port->listener->func), funcs, sizeof(GWPROTOCOL)); - port->listener->session = NULL; if (port->address) sprintf(config_bind, "%s:%d", port->address, port->port); @@ -383,6 +382,7 @@ GWPROTOCOL *funcs; service->name))); users_free(service->users); + service->users = NULL; dcb_close(port->listener); port->listener = NULL; service->users = NULL; diff --git a/server/core/session.c b/server/core/session.c index f3a7375f6..f6cdbf21c 100644 --- a/server/core/session.c +++ b/server/core/session.c @@ -27,6 +27,7 @@ * 02/09/13 Massimiliano Pinto Added session refcounter * 29/05/14 Mark Riddoch Addition of filter mechanism * 23/08/15 Martin Brampton Tidying; slight improvement in safety + * 17/09/15 Martin Brampton Keep failed session in existence - leave DCBs to close * * @endverbatim */ @@ -51,8 +52,10 @@ static size_t session_id; static SPINLOCK session_spin = SPINLOCK_INIT; static SESSION *allSessions = NULL; +static struct session session_dummy_struct; static int session_setup_filters(SESSION *session); +static void session_simple_free(SESSION *session, DCB *dcb); /** * Allocate a new session for a new client of the specified service. @@ -71,55 +74,56 @@ session_alloc(SERVICE *service, DCB *client_dcb) SESSION *session; session = (SESSION *)calloc(1, sizeof(SESSION)); - ss_info_dassert(session != NULL, - "Allocating memory for session failed."); + ss_info_dassert(session != NULL, "Allocating memory for session failed."); if (session == NULL) { - char errbuf[STRERROR_BUFLEN]; + char errbuf[STRERROR_BUFLEN]; LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error : Failed to allocate memory for " "session object due error %d, %s.", errno, strerror_r(errno, errbuf, sizeof(errbuf))))); + /* Does this possibly need a lock? */ + /* + * This is really not the right way to do this. The data in a DCB is + * router specific and should be freed by a function in the relevant + * router. This would be better achieved by placing a function reference + * in the DCB and having dcb_final_free call it to dispose of the data + * at the final destruction of the DCB. However, this piece of code is + * only run following a calloc failure, so the system is probably on + * the point of crashing anyway. + * + */ if (client_dcb->data && !DCB_IS_CLONE(client_dcb)) { - void *clientdata = client_dcb->data; + void * clientdata = client_dcb->data; client_dcb->data = NULL; free(clientdata); } - goto return_session; + return NULL; } #if defined(SS_DEBUG) session->ses_chk_top = CHK_NUM_SESSION; session->ses_chk_tail = CHK_NUM_SESSION; #endif - if (DCB_IS_CLONE(client_dcb)) - { - session->ses_is_child = true; - } + session->ses_is_child = (bool) DCB_IS_CLONE(client_dcb); spinlock_init(&session->ses_lock); - /*< - * Prevent backend threads from accessing before session is completely - * initialized. - */ - spinlock_acquire(&session->ses_lock); session->service = service; - session->client = client_dcb; - session->n_filters = 0; - memset(&session->stats, 0, sizeof(SESSION_STATS)); - session->stats.connect = time(0); - session->state = SESSION_STATE_ALLOC; + session->client = client_dcb; + session->n_filters = 0; + memset(&session->stats, 0, sizeof(SESSION_STATS)); + session->stats.connect = time(0); + session->state = SESSION_STATE_ALLOC; /*< - * Associate the session to the client DCB and set the reference count on - * the session to indicate that there is a single reference to the + * Associate the session to the client DCB and set the reference count on + * the session to indicate that there is a single reference to the * session. There is no need to protect this or use atomic add as the * session has not been made available to the other threads at this * point. */ session->data = client_dcb->data; - client_dcb->session = session; session->refcount = 1; /*< * This indicates that session is ready to be shared with backend @@ -127,139 +131,145 @@ session_alloc(SERVICE *service, DCB *client_dcb) */ session->state = SESSION_STATE_READY; - /*< Release session lock */ - spinlock_release(&session->ses_lock); - - /* - * Only create a router session if we are not the listening - * DCB or an internal DCB. Creating a router session may create a connection to a - * backend server, depending upon the router module implementation - * and should be avoided for the listener session - * - * Router session creation may create other DCBs that link to the - * session, therefore it is important that the session lock is + /* + * Only create a router session if we are not the listening + * DCB or an internal DCB. Creating a router session may create a connection to a + * backend server, depending upon the router module implementation + * and should be avoided for the listener session + * + * Router session creation may create other DCBs that link to the + * session, therefore it is important that the session lock is * relinquished before the router call. - */ - if (client_dcb->state != DCB_STATE_LISTENING && + */ + if (client_dcb->state != DCB_STATE_LISTENING && client_dcb->dcb_role != DCB_ROLE_INTERNAL) - { - session->router_session = - service->router->newSession(service->router_instance, session); + { + session->router_session = service->router->newSession(service->router_instance, session); if (session->router_session == NULL) - { - /** - * Inform other threads that session is closing. - */ - session->state = SESSION_STATE_STOPPING; - /*< - * Decrease refcount, set dcb's session pointer NULL - * and set session pointer to NULL. - */ - session->client = NULL; - session_free(session); - client_dcb->session = NULL; - session = NULL; + { + session->state = SESSION_STATE_TO_BE_FREED; + LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "Error : Failed to create %s session.", + "%lu [%s] Error : Failed to create %s session because router" + "could not establish a new router session, see earlier error.", + pthread_self(), + __func__, service->name))); - goto return_session; } - /* - * Pending filter chain being setup set the head of the chain to - * be the router. As filters are inserted the current head will - * be pushed to the filter and the head updated. - * - * NB This dictates that filters are created starting at the end - * of the chain nearest the router working back to the client - * protocol end of the chain. - */ - session->head.instance = service->router_instance; - session->head.session = session->router_session; + /* + * Pending filter chain being setup set the head of the chain to + * be the router. As filters are inserted the current head will + * be pushed to the filter and the head updated. + * + * NB This dictates that filters are created starting at the end + * of the chain nearest the router working back to the client + * protocol end of the chain. + */ + session->head.instance = service->router_instance; + session->head.session = session->router_session; - session->head.routeQuery = (void *)(service->router->routeQuery); + session->head.routeQuery = (void *)(service->router->routeQuery); - session->tail.instance = session; - session->tail.session = session; - session->tail.clientReply = session_reply; + session->tail.instance = session; + session->tail.session = session; + session->tail.clientReply = session_reply; - if (service->n_filters > 0) - { - if (!session_setup_filters(session)) - { - /** - * Inform other threads that session is closing. - */ - session->state = SESSION_STATE_STOPPING; - /*< - * Decrease refcount, set dcb's session pointer NULL - * and set session pointer to NULL. - */ - session->client = NULL; - session_free(session); - client_dcb->session = NULL; - session = NULL; - LOGIF(LE, (skygw_log_write( - LOGFILE_ERROR, - "Error : Setting up filters failed. " - "Terminating session %s.", - service->name))); - goto return_session; - } - } - } - - spinlock_acquire(&session->ses_lock); - - if (session->state != SESSION_STATE_READY) + if (SESSION_STATE_TO_BE_FREED != session->state + && service->n_filters > 0 + && !session_setup_filters(session)) { - spinlock_release(&session->ses_lock); - session->client = NULL; - session_free(session); - client_dcb->session = NULL; - session = NULL; - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Failed to create %s session.", - service->name))); - spinlock_release(&session_spin); + session->state = SESSION_STATE_TO_BE_FREED; + LOGIF(LE, (skygw_log_write( + LOGFILE_ERROR, + "Error : Setting up filters failed. " + "Terminating session %s.", + service->name))); + } + } + + if (SESSION_STATE_TO_BE_FREED != session->state) + { + session->state = SESSION_STATE_ROUTER_READY; + + if (session->client->user == NULL) + { + LOGIF(LT, (skygw_log_write( + LOGFILE_TRACE, + "Started session [%lu] for %s service ", + session->ses_id, + service->name))); } else { - session->state = SESSION_STATE_ROUTER_READY; - spinlock_release(&session->ses_lock); - spinlock_acquire(&session_spin); - /** Assign a session id and increase */ - session->ses_id = ++session_id; - session->next = allSessions; - allSessions = session; - spinlock_release(&session_spin); - - if (session->client->user == NULL) - { - LOGIF(LT, (skygw_log_write( - LOGFILE_TRACE, - "Started session [%lu] for %s service ", - session->ses_id, - service->name))); - } - else - { - LOGIF(LT, (skygw_log_write( - LOGFILE_TRACE, - "Started %s client session [%lu] for '%s' from %s", - service->name, - session->ses_id, - session->client->user, - session->client->remote))); - } - atomic_add(&service->stats.n_sessions, 1); - atomic_add(&service->stats.n_current, 1); - CHK_SESSION(session); - } -return_session: - return session; + LOGIF(LT, (skygw_log_write( + LOGFILE_TRACE, + "Started %s client session [%lu] for '%s' from %s", + service->name, + session->ses_id, + session->client->user, + session->client->remote))); + } + } + else + { + LOGIF(LT, (skygw_log_write( + LOGFILE_TRACE, + "Start %s client session [%lu] for '%s' from %s failed, will be " + "closed as soon as all related DCBs have been closed.", + service->name, + session->ses_id, + session->client->user, + session->client->remote))); + } + spinlock_acquire(&session_spin); + /** Assign a session id and increase, insert session into list */ + session->ses_id = ++session_id; + session->next = allSessions; + allSessions = session; + spinlock_release(&session_spin); + atomic_add(&service->stats.n_sessions, 1); + atomic_add(&service->stats.n_current, 1); + CHK_SESSION(session); + + client_dcb->session = session; + return SESSION_STATE_TO_BE_FREED == session->state ? NULL : session; +} + +/** + * Allocate a dummy session so that DCBs can always have sessions. + * + * Only one dummy session exists, it is statically declared + * + * @param client_dcb The client side DCB + * @return The dummy created session + */ +SESSION * +session_set_dummy(DCB *client_dcb) +{ + SESSION *session; + + session = &session_dummy_struct; +#if defined(SS_DEBUG) + session->ses_chk_top = CHK_NUM_SESSION; + session->ses_chk_tail = CHK_NUM_SESSION; +#endif + session->ses_is_child = false; + spinlock_init(&session->ses_lock); + session->service = NULL; + session->client = NULL; + session->n_filters = 0; + memset(&session->stats, 0, sizeof(SESSION_STATS)); + session->stats.connect = 0; + session->state = SESSION_STATE_DUMMY; + session->data = NULL; + session->refcount = 1; + session->ses_id = 0; + session->next = NULL; + + client_dcb->session = session; + return session; } /** @@ -356,30 +366,67 @@ int session_unlink_dcb( return nlink; } +/** + * Deallocate the specified session, minimal actions during session_alloc + * Since changes to keep new session in existence until all related DCBs + * have been destroyed, this function is redundant. Just left until we are + * sure of the direction taken. + * + * @param session The session to deallocate + */ +static void +session_simple_free(SESSION *session, DCB *dcb) +{ + /* Does this possibly need a lock? */ + if (dcb->data && !DCB_IS_CLONE(dcb)) + { + void * clientdata = dcb->data; + dcb->data = NULL; + free(clientdata); + } + if (session) + { + if (SESSION_STATE_DUMMY == session->state) + { + return; + } + if (session && session->router_session) + { + session->service->router->freeSession( + session->service->router_instance, + session->router_session); + } + session->state = SESSION_STATE_STOPPING; + } + + free(session); +} + + /** * Deallocate the specified session * * @param session The session to deallocate */ -bool session_free( - SESSION *session) +bool +session_free(SESSION *session) { - bool succp = false; - SESSION *ptr; - int nlink; - int i; - + if (session && SESSION_STATE_DUMMY == session->state) + { + return true; + } CHK_SESSION(session); - /*< + + /* * Remove one reference. If there are no references left, * free session. */ - nlink = session_unlink_dcb(session, NULL); - - if (nlink != 0) { - ss_dassert(nlink > 0); - goto return_succp; + if (atomic_add(&session->refcount, -1) > 1) + { + /* Must be one or more references left */ + return false; } + session->state = SESSION_STATE_TO_BE_FREED; /* First of all remove from the linked list */ spinlock_acquire(&session_spin); @@ -389,13 +436,16 @@ bool session_free( } else { - ptr = allSessions; - while (ptr && ptr->next != session) + SESSION *chksession; + chksession = allSessions; + while (chksession && chksession->next != session) { - ptr = ptr->next; + chksession = chksession->next; } - if (ptr) - ptr->next = session->next; + if (chksession) + { + chksession->next = session->next; + } } spinlock_release(&session_spin); atomic_add(&session->service->stats.n_current, -1); @@ -412,6 +462,7 @@ bool session_free( } if (session->n_filters) { + int i; for (i = 0; i < session->n_filters; i++) { if (session->filters[i].filter) @@ -449,10 +500,7 @@ bool session_free( } free(session); } - succp = true; - -return_succp : - return succp; + return true; } /** @@ -662,11 +710,11 @@ int i; ptr->client->user?ptr->client->user:"", ptr->client->user?"@":"", ptr->client->remote); - dcb_printf(dcb, "\tConnected: %s", + dcb_printf(dcb, "\tConnected: %s\n", asctime_r(localtime_r(&ptr->stats.connect, &result), buf)); if(ptr->client->state == DCB_STATE_POLLING) { - dcb_printf(dcb, "\tIdle: %.0f seconds",idle); + dcb_printf(dcb, "\tIdle: %.0f seconds\n",idle); } } @@ -734,6 +782,8 @@ session_state(int state) { case SESSION_STATE_ALLOC: return "Session Allocated"; + case SESSION_STATE_DUMMY: + return "Dummy Session"; case SESSION_STATE_READY: return "Session Ready"; case SESSION_STATE_ROUTER_READY: diff --git a/server/core/test/CMakeLists.txt b/server/core/test/CMakeLists.txt index 3bd6aee10..149a38f09 100644 --- a/server/core/test/CMakeLists.txt +++ b/server/core/test/CMakeLists.txt @@ -1,10 +1,10 @@ execute_process(COMMAND ${CMAKE_COMMAND} -E copy ${ERRMSG} ${CMAKE_CURRENT_BINARY_DIR}) add_executable(test_mysql_users test_mysql_users.c) -add_executable(test_hash testhash.c) -add_executable(test_hint testhint.c) -add_executable(test_spinlock testspinlock.c) +add_executable(test_hash testhash.c ../random_jkiss.c) +add_executable(test_hint testhint.c ../random_jkiss.c) +add_executable(test_spinlock testspinlock.c ../random_jkiss.c) add_executable(test_filter testfilter.c) -add_executable(test_buffer testbuffer.c) +add_executable(test_buffer testbuffer.c ../random_jkiss.c) add_executable(test_dcb testdcb.c) add_executable(test_modutil testmodutil.c) add_executable(test_poll testpoll.c) @@ -12,9 +12,9 @@ add_executable(test_service testservice.c) add_executable(test_server testserver.c) add_executable(test_users testusers.c) add_executable(test_adminusers testadminusers.c) -add_executable(testmemlog testmemlog.c) +add_executable(testmemlog testmemlog.c ../random_jkiss.c) add_executable(testfeedback testfeedback.c) -add_executable(testmaxscalepcre2 testmaxscalepcre2.c) +add_executable(testmaxscalepcre2 testmaxscalepcre2.c ../random_jkiss.c) target_link_libraries(test_mysql_users MySQLClient fullcore) target_link_libraries(test_hash fullcore log_manager) target_link_libraries(test_hint fullcore log_manager) diff --git a/server/core/test/testservice.c b/server/core/test/testservice.c index 4bb41fee5..d58d37ac1 100644 --- a/server/core/test/testservice.c +++ b/server/core/test/testservice.c @@ -84,28 +84,6 @@ init_test_env(NULL); result = serviceStartAll(); mxs_log_flush_sync(); ss_info_dassert(0 != result, "Start all should succeed"); - - ss_dfprintf(stderr, "\t..done\nTiming out a session."); - - service->conn_timeout = 1; - result = serviceStart(service); - mxs_log_flush_sync(); - ss_info_dassert(0 != result, "Start should succeed"); - serviceStop(service); - mxs_log_flush_sync(); - ss_info_dassert(service->state == SERVICE_STATE_STOPPED, "Stop should succeed"); - - if((dcb = dcb_alloc(DCB_ROLE_REQUEST_HANDLER)) == NULL) - return 1; - ss_info_dassert(dcb != NULL, "DCB allocation failed"); - - session = session_alloc(service,dcb); - ss_info_dassert(session != NULL, "Session allocation failed"); - dcb->state = DCB_STATE_POLLING; - sleep(15); - - ss_info_dassert(dcb->state != DCB_STATE_POLLING, "Session timeout failed"); - ss_dfprintf(stderr, "\t..done\nStopping Service."); serviceStop(service); ss_info_dassert(service->state == SERVICE_STATE_STOPPED, "Stop should succeed"); diff --git a/server/core/test/testsession.c b/server/core/test/testsession.c index 8d2dd46af..bf068bd11 100644 --- a/server/core/test/testsession.c +++ b/server/core/test/testsession.c @@ -66,7 +66,7 @@ int result; sleep(10); poll_shutdown(); ss_dfprintf(stderr, "\t..done\nTidy up."); - dcb_free(dcb); + dcb_close(dcb); ss_dfprintf(stderr, "\t..done\n"); return 0; diff --git a/server/core/utils.c b/server/core/utils.c index c5c60a773..30d346964 100644 --- a/server/core/utils.c +++ b/server/core/utils.c @@ -43,6 +43,7 @@ #include #include #include +#include /* used in the hex2bin function */ #define char_val(X) (X >= '0' && X <= '9' ? X-'0' :\ @@ -97,7 +98,7 @@ char *gw_strend(register const char *s) { * generate a random char *****************************************/ static char gw_randomchar() { - return (char)((rand() % 78) + 30); + return (char)((random_jkiss() % 78) + 30); } /***************************************** @@ -107,7 +108,6 @@ static char gw_randomchar() { int gw_generate_random_str(char *output, int len) { int i; - srand(time(0L)); for ( i = 0; i < len; ++i ) { output[i] = gw_randomchar(); diff --git a/server/include/buffer.h b/server/include/buffer.h index c0555bae4..0e1b34246 100644 --- a/server/include/buffer.h +++ b/server/include/buffer.h @@ -43,6 +43,7 @@ * 03/10/2014 Martin Brampton Pointer arithmetic standard conformity * Add more buffer handling macros * Add gwbuf_rtrim (handle chains) + * 09/11/2014 Martin Brampton Add dprintAllBuffers (conditional compilation) * * @endverbatim */ @@ -52,7 +53,6 @@ #include #include - EXTERN_C_BLOCK_BEGIN /** @@ -168,6 +168,9 @@ typedef struct gwbuf { /*< Consume a number of bytes in the buffer */ #define GWBUF_CONSUME(b, bytes) ((b)->start = bytes > ((char *)(b)->end - (char *)(b)->start) ? (b)->end : (void *)((char *)(b)->start + (bytes))); +/*< Check if a given pointer is within the buffer */ +#define GWBUF_POINTER_IN_BUFFER (ptr, b) ((char *)(ptr) >= (char *)(b)->start && (char *)(ptr) < (char *)(b)->end) + /*< Consume a complete buffer */ #define GWBUF_CONSUME_ALL(b) gwbuf_consume((b), GWBUF_LENGTH((b))) @@ -199,6 +202,9 @@ void gwbuf_add_buffer_object(GWBUF* buf, void* data, void (*donefun_fp)(void *)); void* gwbuf_get_buffer_object_data(GWBUF* buf, bufobj_id_t id); +#if defined(BUFFER_TRACE) +extern void dprintAllBuffers(void *pdcb); +#endif EXTERN_C_BLOCK_END diff --git a/server/include/dcb.h b/server/include/dcb.h index 7339286a1..22e8d21aa 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -179,7 +179,6 @@ typedef enum { DCB_STATE_DISCONNECTED, /*< The socket is now closed */ DCB_STATE_NOPOLLING, /*< Removed from poll mask */ DCB_STATE_ZOMBIE, /*< DCB is no longer active, waiting to free it */ - DCB_STATE_FREED /*< Memory freed */ } dcb_state_t; typedef enum { @@ -227,6 +226,7 @@ typedef struct dcb_callback { typedef struct dcb { skygw_chk_t dcb_chk_top; bool dcb_errhandle_called; /*< this can be called only once */ + bool dcb_is_zombie; /**< Whether the DCB is in the zombie list */ dcb_role_t dcb_role; SPINLOCK dcb_initlock; DCBEVENTQ evq; /**< The event queue for this DCB */ @@ -339,6 +339,7 @@ int dcb_count_by_usage(DCB_USAGE); /* Return counts of DCBs */ int dcb_persistent_clean_count(DCB *, bool); /* Clean persistent and return count */ void dcb_call_foreach (struct server* server, DCB_REASON reason); +void dcb_hangup_foreach (struct server* server); size_t dcb_get_session_id(DCB* dcb); bool dcb_get_ses_log_info(DCB* dcb, size_t* sesid, int* enabled_logs); char *dcb_role_name(DCB *); /* Return the name of a role */ diff --git a/server/include/gwbitmask.h b/server/include/gwbitmask.h index baeb1ef77..26d5e3621 100644 --- a/server/include/gwbitmask.h +++ b/server/include/gwbitmask.h @@ -27,6 +27,7 @@ * * Date Who Description * 28/06/13 Mark Riddoch Initial implementation + * 17/10/15 Martin Brampton Add bitmask_render_readable * * @endverbatim */ @@ -51,4 +52,6 @@ extern void bitmask_clear(GWBITMASK *, int); extern int bitmask_isset(GWBITMASK *, int); extern int bitmask_isallclear(GWBITMASK *); extern void bitmask_copy(GWBITMASK *, GWBITMASK *); +extern char *bitmask_render_readable(GWBITMASK *bitmask); + #endif diff --git a/server/include/poll.h b/server/include/poll.h index e778c580c..f054667d4 100644 --- a/server/include/poll.h +++ b/server/include/poll.h @@ -29,6 +29,7 @@ * * Date Who Description * 19/06/13 Mark Riddoch Initial implementation + * 17/10/15 Martin Brampton Declare fake event functions * * @endverbatim */ @@ -60,9 +61,11 @@ extern void poll_set_maxwait(unsigned int); extern void poll_set_nonblocking_polls(unsigned int); extern void dprintPollStats(DCB *); extern void dShowThreads(DCB *dcb); -void poll_add_epollin_event_to_dcb(DCB* dcb, GWBUF* buf); +void poll_add_epollin_event_to_dcb(DCB* dcb, GWBUF* buf); extern void dShowEventQ(DCB *dcb); extern void dShowEventStats(DCB *dcb); -extern int poll_get_stat(POLL_STAT stat); +extern int poll_get_stat(POLL_STAT stat); extern RESULTSET *eventTimesGetList(); +extern void poll_fake_hangup_event(DCB *dcb); +extern void poll_fake_write_event(DCB *dcb); #endif diff --git a/server/include/random_jkiss.h b/server/include/random_jkiss.h new file mode 100644 index 000000000..21d638c82 --- /dev/null +++ b/server/include/random_jkiss.h @@ -0,0 +1,22 @@ +/* + * File: random_jkiss.h + * Author: mbrampton + * + * Created on 26 August 2015, 15:34 + */ + +#ifndef RANDOM_JKISS_H +#define RANDOM_JKISS_H + +#ifdef __cplusplus +extern "C" { +#endif + +extern unsigned int random_jkiss(void); + +#ifdef __cplusplus +} +#endif + +#endif /* RANDOM_H */ + diff --git a/server/include/router.h b/server/include/router.h index 3815253bf..655075be5 100644 --- a/server/include/router.h +++ b/server/include/router.h @@ -30,6 +30,7 @@ * 15/07/2013 Massimiliano Pinto Added clientReply entry point * 16/07/2013 Massimiliano Pinto Added router commands values * 22/10/2013 Massimiliano Pinto Added router errorReply entry point + * 27/10/2015 Martin Brampton Add RCAP_TYPE_NO_RSESSION * */ #include @@ -45,8 +46,7 @@ typedef void *ROUTER; typedef enum error_action { ERRACT_NEW_CONNECTION = 0x001, - ERRACT_REPLY_CLIENT = 0x002, - ERRACT_RESET = 0x004 + ERRACT_REPLY_CLIENT = 0x002 } error_action_t; /** @@ -86,7 +86,7 @@ typedef struct router_object { DCB* backend_dcb, error_action_t action, bool* succp); - uint8_t (*getCapabilities)(ROUTER *instance, void* router_session); + int (*getCapabilities)(); } ROUTER_OBJECT; /** @@ -101,8 +101,9 @@ typedef struct router_object { */ typedef enum router_capability_t { RCAP_TYPE_UNDEFINED = 0x00, - RCAP_TYPE_STMT_INPUT = 0x01, /*< statement per buffer */ - RCAP_TYPE_PACKET_INPUT = 0x02 /*< data as it was read from DCB */ + RCAP_TYPE_STMT_INPUT = 0x01, /*< statement per buffer */ + RCAP_TYPE_PACKET_INPUT = 0x02, /*< data as it was read from DCB */ + RCAP_TYPE_NO_RSESSION = 0x04 /*< router does not use router sessions */ } router_capability_t; diff --git a/server/include/session.h b/server/include/session.h index 7a9381507..edb4af1d4 100644 --- a/server/include/session.h +++ b/server/include/session.h @@ -64,7 +64,8 @@ typedef enum { SESSION_STATE_LISTENER, /*< for listener session */ SESSION_STATE_LISTENER_STOPPED, /*< for listener session */ SESSION_STATE_TO_BE_FREED, /*< ready to be freed as soon as there are no references */ - SESSION_STATE_FREE /*< for all sessions */ + SESSION_STATE_FREE, /*< for all sessions */ + SESSION_STATE_DUMMY /*< dummy session for consistency */ } session_state_t; /** @@ -162,6 +163,7 @@ typedef struct session { SESSION *get_all_sessions(); SESSION *session_alloc(struct service *, struct dcb *); +SESSION *session_set_dummy(struct dcb *); bool session_free(SESSION *); int session_isvalid(SESSION *); int session_reply(void *inst, void *session, GWBUF *data); diff --git a/server/modules/filter/tee.c b/server/modules/filter/tee.c index 3906367a9..4a9193ad1 100644 --- a/server/modules/filter/tee.c +++ b/server/modules/filter/tee.c @@ -207,6 +207,7 @@ int route_single_query(TEE_INSTANCE* my_instance, GWBUF* buffer, GWBUF* clone); int reset_session_state(TEE_SESSION* my_session, GWBUF* buffer); +void create_orphan(SESSION* ses); static void orphan_free(void* data) @@ -552,6 +553,7 @@ char *remote, *userName; if ((ses = session_alloc(my_instance->service, dcb)) == NULL) { + filter_free(dummy); dcb_close(dcb); freeSession(instance, (void *)my_session); my_session = NULL; @@ -567,39 +569,27 @@ char *remote, *userName; dummy->obj = GetModuleObject(); dummy->filter = NULL; - + my_session->branch_session = ses; + my_session->branch_dcb = dcb; + my_session->dummy_filterdef = dummy; if((dummy_upstream = filterUpstream( dummy, my_session, &ses->tail)) == NULL) { - spinlock_acquire(&ses->ses_lock); - ses->state = SESSION_STATE_STOPPING; - spinlock_release(&ses->ses_lock); - - ses->service->router->closeSession( - ses->service->router_instance, - ses->router_session); - - ses->client = NULL; - dcb->session = NULL; - session_free(ses); + filter_free(dummy); + closeSession(instance,(void*)my_session); dcb_close(dcb); - freeSession(instance, (void *) my_session); - my_session = NULL; + free(my_session); LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error : tee: Allocating memory for" "dummy upstream failed." " Terminating session."))); - goto retblock; + return NULL; } ses->tail = *dummy_upstream; - my_session->branch_session = ses; - my_session->branch_dcb = dcb; - my_session->dummy_filterdef = dummy; - MySQLProtocol* protocol = (MySQLProtocol*)session->client->protocol; my_session->use_ok = protocol->client_capabilities & (1 << 6); free(dummy_upstream); @@ -707,19 +697,7 @@ skygw_log_write(LOGFILE_TRACE,"Tee free: %d", atomic_add(&debug_seq,1)); } else if(state == SESSION_STATE_STOPPING) { - orphan_session_t* orphan; - if((orphan = malloc(sizeof(orphan_session_t))) == NULL) - { - skygw_log_write(LOGFILE_ERROR,"Error : Failed to " - "allocate memory for orphan session struct, " - "child session might leak memory."); - }else{ - orphan->session = ses; - spinlock_acquire(&orphanLock); - orphan->next = allOrphans; - allOrphans = orphan; - spinlock_release(&orphanLock); - } + create_orphan(ses); } } if (my_session->dummy_filterdef) @@ -1396,3 +1374,20 @@ int reset_session_state(TEE_SESSION* my_session, GWBUF* buffer) return 1; } + +void create_orphan(SESSION* ses) +{ + orphan_session_t* orphan; + if((orphan = malloc(sizeof(orphan_session_t))) == NULL) + { + skygw_log_write(LOGFILE_ERROR,"Error : Failed to " + "allocate memory for orphan session struct, " + "child session might leak memory."); + }else{ + orphan->session = ses; + spinlock_acquire(&orphanLock); + orphan->next = allOrphans; + allOrphans = orphan; + spinlock_release(&orphanLock); + } +} diff --git a/server/modules/include/readwritesplit.h b/server/modules/include/readwritesplit.h index 58eb48996..0e229f807 100644 --- a/server/modules/include/readwritesplit.h +++ b/server/modules/include/readwritesplit.h @@ -290,7 +290,6 @@ struct router_client_session { rwsplit_config_t rses_config; /*< copied config info from router instance */ int rses_nbackends; int rses_nsescmd; /*< Number of executed session commands */ - int rses_capabilities; /*< input type, for example */ bool rses_autocommit_enabled; bool rses_transaction_active; bool rses_load_active; /*< If LOAD DATA LOCAL INFILE is diff --git a/server/modules/include/schemarouter.h b/server/modules/include/schemarouter.h index e184e96e3..d4f220903 100644 --- a/server/modules/include/schemarouter.h +++ b/server/modules/include/schemarouter.h @@ -316,7 +316,6 @@ struct router_client_session { backend_ref_t* rses_backend_ref; /*< Pointer to backend reference array */ schemarouter_config_t rses_config; /*< Copied config info from router instance */ int rses_nbackends; /*< Number of backends */ - int rses_capabilities; /*< Input type, for example */ bool rses_autocommit_enabled; /*< Is autocommit enabled */ bool rses_transaction_active; /*< Is a transaction active */ struct router_instance *router; /*< The router instance */ diff --git a/server/modules/include/shardrouter.h b/server/modules/include/shardrouter.h index 3fc70fcc1..f288505c1 100644 --- a/server/modules/include/shardrouter.h +++ b/server/modules/include/shardrouter.h @@ -192,7 +192,6 @@ struct router_client_session { rses_property_t* rses_properties[RSES_PROP_TYPE_COUNT]; shard_config_t rses_config; /*< copied config info from router instance */ - int rses_capabilities; /*< input type, for example */ bool rses_autocommit_enabled; bool rses_transaction_active; struct router_instance *router; /*< The router instance */ diff --git a/server/modules/monitor/galeramon.c b/server/modules/monitor/galeramon.c index e81339832..c986943f9 100644 --- a/server/modules/monitor/galeramon.c +++ b/server/modules/monitor/galeramon.c @@ -35,12 +35,14 @@ * 20/04/15 Guillaume Lefranc Added availableWhenDonor feature * 22/04/15 Martin Brampton Addition of disableMasterRoleSetting * 08/05/15 Markus Makela Addition of launchable scripts + * 17/10/15 Martin Brampton Change DCB callback to hangup * * @endverbatim */ #include +#include static void monitorMain(void *); @@ -483,13 +485,13 @@ monitor_event_t evtype; if (!(SERVER_IS_RUNNING(ptr->server)) || !(SERVER_IS_IN_CLUSTER(ptr->server))) { - dcb_call_foreach(ptr->server,DCB_REASON_NOT_RESPONDING); + dcb_hangup_foreach(ptr->server); } if (SERVER_IS_DOWN(ptr->server)) { /** Increase this server'e error count */ - dcb_call_foreach(ptr->server,DCB_REASON_NOT_RESPONDING); + dcb_hangup_foreach(ptr->server); ptr->mon_err_count += 1; } diff --git a/server/modules/monitor/mmmon.c b/server/modules/monitor/mmmon.c index 2339a69a9..7029c4691 100644 --- a/server/modules/monitor/mmmon.c +++ b/server/modules/monitor/mmmon.c @@ -25,12 +25,13 @@ * Date Who Description * 08/09/14 Massimiliano Pinto Initial implementation * 08/05/15 Markus Makela Addition of launchable scripts + * 17/10/15 Martin Brampton Change DCB callback to hangup * * @endverbatim */ - #include +#include static void monitorMain(void *); @@ -562,7 +563,7 @@ detect_stale_master = handle->detectStaleMaster; if (mon_status_changed(ptr)) { - dcb_call_foreach(ptr->server,DCB_REASON_NOT_RESPONDING); + dcb_hangup_foreach(ptr->server); } if (mon_status_changed(ptr) || diff --git a/server/modules/monitor/mysql_mon.c b/server/modules/monitor/mysql_mon.c index bd8e3e54f..2b934667f 100644 --- a/server/modules/monitor/mysql_mon.c +++ b/server/modules/monitor/mysql_mon.c @@ -47,11 +47,13 @@ * 18/11/14 Massimiliano Pinto One server only in configuration becomes master. * servers=server1 must be present in mysql_mon and in router sections as well. * 08/05/15 Markus Makela Added launchable scripts + * 17/10/15 Martin Brampton Change DCB callback to hangup * * @endverbatim */ #include +#include #include static void monitorMain(void *); @@ -814,8 +816,7 @@ detect_stale_master = handle->detectStaleMaster; if (!(SERVER_IS_RUNNING(ptr->server)) || !(SERVER_IS_IN_CLUSTER(ptr->server))) { - dcb_call_foreach(ptr->server,DCB_REASON_NOT_RESPONDING); - + dcb_hangup_foreach(ptr->server); } diff --git a/server/modules/protocol/httpd.c b/server/modules/protocol/httpd.c index ef1754040..daa73d538 100644 --- a/server/modules/protocol/httpd.c +++ b/server/modules/protocol/httpd.c @@ -345,9 +345,6 @@ int n_connect = 0; client->remote = strdup(inet_ntoa(addr.sin_addr)); memcpy(&client->func, &MyObject, sizeof(GWPROTOCOL)); - /* we don't need the session */ - client->session = NULL; - /* create the session data for HTTPD */ client_data = (HTTPD_session *)calloc(1, sizeof(HTTPD_session)); client->data = client_data; @@ -355,9 +352,10 @@ int n_connect = 0; client->session = session_alloc(dcb->session->service, client); - if (poll_add_dcb(client) == -1) + if (NULL == client->session || poll_add_dcb(client) == -1) { close(so); + dcb_close(client); return n_connect; } n_connect++; diff --git a/server/modules/protocol/maxscaled.c b/server/modules/protocol/maxscaled.c index e1051c3f2..d18d99fb8 100644 --- a/server/modules/protocol/maxscaled.c +++ b/server/modules/protocol/maxscaled.c @@ -151,7 +151,7 @@ char *password; maxscaled->username = strndup(GWBUF_DATA(head), GWBUF_LENGTH(head)); maxscaled->state = MAXSCALED_STATE_PASSWD; dcb_printf(dcb, "PASSWORD"); - gwbuf_consume(head, GWBUF_LENGTH(head)); + while ((head = gwbuf_consume(head, GWBUF_LENGTH(head))) != NULL); break; case MAXSCALED_STATE_PASSWD: password = strndup(GWBUF_DATA(head), GWBUF_LENGTH(head)); @@ -165,7 +165,7 @@ char *password; dcb_printf(dcb, "FAILED"); maxscaled->state = MAXSCALED_STATE_LOGIN; } - gwbuf_consume(head, GWBUF_LENGTH(head)); + while ((head = gwbuf_consume(head, GWBUF_LENGTH(head))) != NULL); free(password); break; case MAXSCALED_STATE_DATA: @@ -177,7 +177,7 @@ char *password; else { // Force the free of the buffer header - gwbuf_consume(head, 0); + while ((head = gwbuf_consume(head, GWBUF_LENGTH(head))) != NULL); } } } @@ -286,7 +286,7 @@ int n_connect = 0; client_dcb->session = session_alloc(dcb->session->service, client_dcb); - if (poll_add_dcb(client_dcb)) + if (NULL == client_dcb->session || poll_add_dcb(client_dcb)) { dcb_close(dcb); return n_connect; diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index d4c616a66..e48c00f98 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -44,6 +44,8 @@ * 24/10/2014 Massimiliano Pinto Added Mysql user@host @db authentication support * 10/11/2014 Massimiliano Pinto Client charset is passed to backend * 19/06/2015 Martin Brampton Persistent connection handling + * 07/10/2015 Martin Brampton Remove calls to dcb_close - should be done by routers + * 27/10/2015 Martin Brampton Test for RCAP_TYPE_NO_RSESSION before calling clientReply * */ #include @@ -135,7 +137,7 @@ static MYSQL_session* gw_get_shared_session_auth_info( spinlock_acquire(&dcb->session->ses_lock); - if (dcb->session->state != SESSION_STATE_ALLOC) { + if (dcb->session->state != SESSION_STATE_ALLOC && dcb->session->state != SESSION_STATE_DUMMY) { auth_info = dcb->session->data; } else { LOGIF(LE, (skygw_log_write_flush( @@ -162,7 +164,7 @@ static int gw_read_backend_event(DCB *dcb) { int rc = 0; CHK_DCB(dcb); - if (!dcb->session && dcb->persistentstart) + if (dcb->persistentstart) { dcb->dcb_errhandle_called = true; goto return_rc; @@ -280,7 +282,7 @@ static int gw_read_backend_event(DCB *dcb) { SESSION *session = dcb->session; int receive_rc = 0; - if (session == NULL) + if (SESSION_STATE_DUMMY == session->state) { rc = 0; goto return_with_lock; @@ -390,27 +392,34 @@ static int gw_read_backend_event(DCB *dcb) { "Authentication with backend failed. " "Session will be closed."); - router->handleError(router_instance, + if (rsession) + { + router->handleError(router_instance, rsession, errbuf, dcb, ERRACT_REPLY_CLIENT, &succp); + } + else + { + gwbuf_free(errbuf); + dcb->dcb_errhandle_called = true; + /* + * I'm pretty certain this is best removed and + * causes trouble if present, but have left it + * here just for now as a comment. Martin + */ + /* dcb_close(dcb); */ + rc = 1; + goto return_rc; + } gwbuf_free(errbuf); - LOGIF(LD, (skygw_log_write( - LOGFILE_DEBUG, - "%lu [gw_read_backend_event] " - "after calling handleError. Backend " - "DCB %p, session %p", - pthread_self(), - dcb, - dcb->session))); spinlock_acquire(&session->ses_lock); session->state = SESSION_STATE_STOPPING; spinlock_release(&session->ses_lock); ss_dassert(dcb->dcb_errhandle_called); - dcb_close(dcb); rc = 1; goto return_rc; } @@ -483,8 +492,6 @@ static int gw_read_backend_event(DCB *dcb) { session->state = SESSION_STATE_STOPPING; spinlock_release(&session->ses_lock); } - ss_dassert(dcb->dcb_errhandle_called); - dcb_close(dcb); rc = 0; goto return_rc; } @@ -565,7 +572,8 @@ static int gw_read_backend_event(DCB *dcb) { */ if (dcb->session->state == SESSION_STATE_ROUTER_READY && dcb->session->client != NULL && - dcb->session->client->state == DCB_STATE_POLLING) + dcb->session->client->state == DCB_STATE_POLLING && + (session->router_session || router->getCapabilities() & RCAP_TYPE_NO_RSESSION)) { client_protocol = SESSION_PROTOCOL(dcb->session, MySQLProtocol); @@ -825,6 +833,11 @@ static int gw_error_backend_event(DCB *dcb) CHK_DCB(dcb); session = dcb->session; CHK_SESSION(session); + if (SESSION_STATE_DUMMY == session->state) + { + dcb_close(dcb); + return 1; + } rsession = session->router_session; router = session->service->router; router_instance = session->service->router_instance; @@ -920,8 +933,6 @@ static int gw_error_backend_event(DCB *dcb) session->state = SESSION_STATE_STOPPING; spinlock_release(&session->ses_lock); } - ss_dassert(dcb->dcb_errhandle_called); - dcb_close(dcb); retblock: return 1; @@ -1062,7 +1073,7 @@ gw_backend_hangup(DCB *dcb) session_state_t ses_state; CHK_DCB(dcb); - if (!dcb->session && dcb->persistentstart) + if (dcb->persistentstart) { dcb->dcb_errhandle_called = true; goto retblock; @@ -1120,6 +1131,12 @@ gw_backend_hangup(DCB *dcb) } } gwbuf_free(errbuf); + /* + * I'm pretty certain this is best removed and + * causes trouble if present, but have left it + * here just for now as a comment. Martin + */ + /* dcb_close(dcb); */ goto retblock; } #if defined(SS_DEBUG) @@ -1151,8 +1168,6 @@ gw_backend_hangup(DCB *dcb) session->state = SESSION_STATE_STOPPING; spinlock_release(&session->ses_lock); } - ss_dassert(dcb->dcb_errhandle_called); - dcb_close(dcb); retblock: return 1; @@ -1204,10 +1219,13 @@ gw_backend_close(DCB *dcb) { if (session->client->state == DCB_STATE_POLLING) { + DCB *temp; spinlock_release(&session->ses_lock); /** Close client DCB */ - dcb_close(session->client); + temp = session->client; + session->client = NULL; + dcb_close(temp); } else { @@ -1332,8 +1350,6 @@ static int backend_write_delayqueue(DCB *dcb) spinlock_acquire(&session->ses_lock); session->state = SESSION_STATE_STOPPING; spinlock_release(&session->ses_lock); - ss_dassert(dcb->dcb_errhandle_called); - dcb_close(dcb); } } } diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 51ca80e89..113a10e0f 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -39,6 +39,8 @@ * 10/11/2014 Massimiliano Pinto Added: client charset added to protocol struct * 29/05/2015 Markus Makela Added SSL support * 11/06/2015 Martin Brampton COM_QUIT suppressed for persistent connections + * 04/09/2015 Martin Brampton Introduce DUMMY session to fulfill guarantee DCB always has session + * 09/09/2015 Martin Brampton Modify error handler calls */ #include #include @@ -745,76 +747,38 @@ int gw_read_client_event( session = dcb->session; - if (protocol->protocol_auth_state == MYSQL_IDLE && session != NULL) + if (protocol->protocol_auth_state == MYSQL_IDLE && session != NULL && SESSION_STATE_DUMMY != session->state) { CHK_SESSION(session); router = session->service->router; router_instance = session->service->router_instance; rsession = session->router_session; - ss_dassert(rsession != NULL); - if (router_instance != NULL && rsession != NULL) { + if (NULL == router_instance || NULL == rsession) + { + /** Send ERR 1045 to client */ + mysql_send_auth_error( + dcb, + 2, + 0, + "failed to create new session"); + while (read_buffer) + { + read_buffer = gwbuf_consume(read_buffer, GWBUF_LENGTH(read_buffer)); + } + return 0; + } - /** Ask what type of input the router expects */ - cap = router->getCapabilities(router_instance, rsession); - - if (cap == 0 || (cap == RCAP_TYPE_PACKET_INPUT)) - { - stmt_input = false; - } - else if (cap == RCAP_TYPE_STMT_INPUT) - { - stmt_input = true; - /** Mark buffer to as MySQL type */ - gwbuf_set_type(read_buffer, GWBUF_TYPE_MYSQL); - } - - /** - * If router doesn't implement getCapabilities correctly we end - * up here. - */ - else - { - GWBUF* errbuf; - bool succp; - - LOGIF(LD, (skygw_log_write_flush( - LOGFILE_DEBUG, - "%lu [gw_read_client_event] Reading router " - "capabilities failed.", - pthread_self()))); - - errbuf = mysql_create_custom_error( - 1, - 0, - "Read invalid router capabilities. Routing failed. " - "Session will be closed."); - - router->handleError( - router_instance, - rsession, - errbuf, - dcb, - ERRACT_REPLY_CLIENT, - &succp); - gwbuf_free(errbuf); - /** - * If there are not enough backends close - * session - */ - if (!succp) - { - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Routing the query failed. " - "Session will be closed."))); - dcb_close(dcb); - } - rc = 1; - goto return_rc; - } - } - } + /** Ask what type of input the router expects */ + cap = router->getCapabilities(router_instance, rsession); + + if (cap & RCAP_TYPE_STMT_INPUT) + { + stmt_input = true; + /** Mark buffer to as MySQL type */ + gwbuf_set_type(read_buffer, GWBUF_TYPE_MYSQL); + } + } if (stmt_input) { @@ -907,7 +871,7 @@ int gw_read_client_event( if (session != NULL) { CHK_SESSION(session); - ss_dassert(session->state != SESSION_STATE_ALLOC); + ss_dassert(session->state != SESSION_STATE_ALLOC && session->state != SESSION_STATE_DUMMY); protocol->protocol_auth_state = MYSQL_IDLE; /** @@ -1007,7 +971,7 @@ int gw_read_client_event( if (session != NULL) { CHK_SESSION(session); - ss_dassert(session->state != SESSION_STATE_ALLOC); + ss_dassert(session->state != SESSION_STATE_ALLOC && session->state != SESSION_STATE_DUMMY); protocol->protocol_auth_state = MYSQL_IDLE; /** @@ -1091,7 +1055,7 @@ int gw_read_client_event( session_state_t ses_state; session = dcb->session; - ss_dassert(session!= NULL); + ss_dassert(session!= NULL && SESSION_STATE_DUMMY != session->state); if (session != NULL) { @@ -1117,6 +1081,7 @@ int gw_read_client_event( /* Temporarily suppressed: SESSION_ROUTE_QUERY(session, read_buffer); */ /* Replaced with freeing the read buffer. */ gwbuf_free(read_buffer); + read_buffer = NULL; /** * Close router session which causes closing of backends. */ @@ -1125,7 +1090,7 @@ int gw_read_client_event( else { /** Reset error handler when routing of the new query begins */ - router->handleError(NULL, NULL, NULL, dcb, ERRACT_RESET, NULL); + dcb->dcb_errhandle_called = false; if (stmt_input) { @@ -1139,12 +1104,18 @@ int gw_read_client_event( { /** add incomplete mysql packet to read queue */ dcb->dcb_readqueue = gwbuf_append(dcb->dcb_readqueue, read_buffer); + read_buffer = NULL; } } - else + else if (NULL != session->router_session || cap & RCAP_TYPE_NO_RSESSION) { /** Feed whole packet to router */ rc = SESSION_ROUTE_QUERY(session, read_buffer); + read_buffer = NULL; + } + else + { + rc = 0; } /** Routing succeed */ @@ -1187,8 +1158,11 @@ int gw_read_client_event( "Error : Routing the query failed. " "Session will be closed."))); - dcb_close(dcb); } + while (read_buffer) + { + read_buffer = gwbuf_consume(read_buffer, GWBUF_LENGTH(read_buffer)); + } } } } @@ -1678,6 +1652,7 @@ int gw_MySQLAccept(DCB *listener) } client_dcb->service = listener->session->service; + client_dcb->session = session_set_dummy(client_dcb); client_dcb->fd = c_sock; // get client address @@ -1837,7 +1812,7 @@ gw_client_close(DCB *dcb) * session may be NULL if session_alloc failed. * In that case, router session wasn't created. */ - if (session != NULL) + if (session != NULL && SESSION_STATE_DUMMY != session->state) { CHK_SESSION(session); spinlock_acquire(&session->ses_lock); @@ -1892,9 +1867,11 @@ gw_client_hangup_event(DCB *dcb) goto retblock; } #if defined(SS_DEBUG) - LOGIF(LD, (skygw_log_write_flush( - LOGFILE_DEBUG, - "Client hangup error handling."))); + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Client hangup error handling, session state %s, dcb state %s.", + session_state(session->state), + STRDCBSTATE(dcb->state)))); #endif dcb_close(dcb); diff --git a/server/modules/protocol/telnetd.c b/server/modules/protocol/telnetd.c index ea236dbe2..943154c18 100644 --- a/server/modules/protocol/telnetd.c +++ b/server/modules/protocol/telnetd.c @@ -306,6 +306,11 @@ int n_connect = 0; memcpy(&client_dcb->func, &MyObject, sizeof(GWPROTOCOL)); client_dcb->session = session_alloc(dcb->session->service, client_dcb); + if (NULL == client_dcb->session) + { + dcb_close(client_dcb); + return n_connect; + } telnetd_pr = (TELNETD *)malloc(sizeof(TELNETD)); client_dcb->protocol = (void *)telnetd_pr; diff --git a/server/modules/routing/binlog/CMakeLists.txt b/server/modules/routing/binlog/CMakeLists.txt index e89664cd8..dda5889d3 100644 --- a/server/modules/routing/binlog/CMakeLists.txt +++ b/server/modules/routing/binlog/CMakeLists.txt @@ -16,8 +16,10 @@ add_executable(maxbinlogcheck maxbinlogcheck.c blr_file.c blr_cache.c blr_master ${CMAKE_SOURCE_DIR}/server/core/resultset.c ${CMAKE_SOURCE_DIR}/server/core/load_utils.c ${CMAKE_SOURCE_DIR}/server/core/monitor.c ${CMAKE_SOURCE_DIR}/server/core/gw_utils.c ${CMAKE_SOURCE_DIR}/server/core/thread.c ${CMAKE_SOURCE_DIR}/server/core/secrets.c + ${CMAKE_SOURCE_DIR}/server/core/random_jkiss.c ${CMAKE_SOURCE_DIR}/log_manager/log_manager.cc) + target_link_libraries(maxbinlogcheck utils ssl pthread ${EMBEDDED_LIB} ${PCRE_LINK_FLAGS} aio rt crypt dl crypto inih z m stdc++ ${CURL_LIBRARIES}) install(TARGETS maxbinlogcheck DESTINATION bin) diff --git a/server/modules/routing/binlog/blr.c b/server/modules/routing/binlog/blr.c index 013e687f7..5411b7dec 100644 --- a/server/modules/routing/binlog/blr.c +++ b/server/modules/routing/binlog/blr.c @@ -46,8 +46,10 @@ * If set those values are sent to slaves instead of * saved master responses * 23/08/2015 Massimiliano Pinto Added strerror_r + * 09/09/2015 Martin Brampton Modify error handler * 30/09/2015 Massimiliano Pinto Addition of send_slave_heartbeat option * 23/10/2015 Markus Makela Added current_safe_event + * 27/10/2015 Martin Brampton Amend getCapabilities to return RCAP_TYPE_NO_RSESSION * * @endverbatim */ @@ -97,7 +99,7 @@ static void errorReply( error_action_t action, bool *succp); -static uint8_t getCapabilities (ROUTER* inst, void* router_session); +static int getCapabilities (); static int blr_handler_config(void *userdata, const char *section, const char *name, const char *value); static int blr_handle_config_item(const char *name, const char *value, ROUTER_INSTANCE *inst); static int blr_set_service_mysql_user(SERVICE *service); @@ -111,7 +113,6 @@ extern int blr_read_events_all_events(ROUTER_INSTANCE *router, int fix, int debu void blr_master_close(ROUTER_INSTANCE *); char * blr_last_event_description(ROUTER_INSTANCE *router); extern int MaxScaleUptime(); -static uint8_t getCapabilities (ROUTER* inst, void* router_session); char *blr_get_event_description(ROUTER_INSTANCE *router, uint8_t event); /** The module object definition */ @@ -1391,8 +1392,8 @@ int len; * @param router_session The router session * @param message The error message to reply * @param backend_dcb The backend DCB - * @param action The action: REPLY, REPLY_AND_CLOSE, NEW_CONNECTION - * @param succp Result of action + * @param action The action: ERRACT_NEW_CONNECTION or ERRACT_REPLY_CLIENT + * @param succp Result of action: true iff router can continue * */ static void @@ -1405,12 +1406,6 @@ char msg[STRERROR_BUFLEN + 1 + 5] = ""; char *errmsg; unsigned long mysql_errno; - if (action == ERRACT_RESET) - { - backend_dcb->dcb_errhandle_called = false; - return; - } - /** Don't handle same error twice on same DCB */ if (backend_dcb->dcb_errhandle_called) { @@ -1464,6 +1459,7 @@ unsigned long mysql_errno; if (errmsg) free(errmsg); *succp = true; + dcb_close(backend_dcb); LOGIF(LM, (skygw_log_write_flush( LOGFILE_MESSAGE, "%s: Master %s disconnected after %ld seconds. " @@ -1522,9 +1518,9 @@ static void rses_end_locked_router_action(ROUTER_SLAVE * rses) } -static uint8_t getCapabilities(ROUTER *inst, void *router_session) +static int getCapabilities() { - return 0; + return RCAP_TYPE_NO_RSESSION; } /** diff --git a/server/modules/routing/binlog/blr_slave.c b/server/modules/routing/binlog/blr_slave.c index 7dd481e8f..7ed44eb78 100644 --- a/server/modules/routing/binlog/blr_slave.c +++ b/server/modules/routing/binlog/blr_slave.c @@ -58,6 +58,7 @@ * 25/09/2015 Massimiliano Pinto Addition of slave heartbeat: * the period set during registration is checked * and heartbeat event might be sent to the affected slave. + * 25/09/2015 Martin Brampton Block callback processing when no router session in the DCB * 23/10/15 Markus Makela Added current_safe_event * * @endverbatim @@ -2224,6 +2225,15 @@ blr_slave_callback(DCB *dcb, DCB_REASON reason, void *data) ROUTER_SLAVE *slave = (ROUTER_SLAVE *)data; ROUTER_INSTANCE *router = slave->router; + if (NULL == dcb->session->router_session) + { + /* + * The following processing will fail if there is no router session, + * because the "data" parameter will not contain meaningful data, + * so we have no choice but to stop here. + */ + return 0; + } if (reason == DCB_REASON_DRAINED) { if (slave->state == BLRS_DUMPING) diff --git a/server/modules/routing/cli.c b/server/modules/routing/cli.c index 311c15c3f..19bb15cc7 100644 --- a/server/modules/routing/cli.c +++ b/server/modules/routing/cli.c @@ -62,7 +62,7 @@ static void closeSession(ROUTER *instance, void *router_session); static void freeSession(ROUTER *instance, void *router_session); static int execute(ROUTER *instance, void *router_session, GWBUF *queue); static void diagnostics(ROUTER *instance, DCB *dcb); -static uint8_t getCapabilities (ROUTER* inst, void* router_session); +static int getCapabilities (); /** The module object definition */ static ROUTER_OBJECT MyObject = { @@ -287,9 +287,7 @@ diagnostics(ROUTER *instance, DCB *dcb) return; /* Nothing to do currently */ } -static uint8_t getCapabilities( - ROUTER* inst, - void* router_session) +static int getCapabilities() { return 0; } diff --git a/server/modules/routing/debugcli.c b/server/modules/routing/debugcli.c index 3467136e6..9f6213809 100644 --- a/server/modules/routing/debugcli.c +++ b/server/modules/routing/debugcli.c @@ -61,7 +61,7 @@ static void closeSession(ROUTER *instance, void *router_session); static void freeSession(ROUTER *instance, void *router_session); static int execute(ROUTER *instance, void *router_session, GWBUF *queue); static void diagnostics(ROUTER *instance, DCB *dcb); -static uint8_t getCapabilities (ROUTER* inst, void* router_session); +static int getCapabilities (); /** The module object definition */ static ROUTER_OBJECT MyObject = { @@ -310,9 +310,7 @@ diagnostics(ROUTER *instance, DCB *dcb) return; /* Nothing to do currently */ } -static uint8_t getCapabilities( - ROUTER* inst, - void* router_session) +static int getCapabilities() { return 0; } diff --git a/server/modules/routing/debugcmd.c b/server/modules/routing/debugcmd.c index b3e846b57..51dbbf6cb 100644 --- a/server/modules/routing/debugcmd.c +++ b/server/modules/routing/debugcmd.c @@ -44,6 +44,7 @@ * 16/10/14 Mark Riddoch Add show eventq * 05/03/15 Massimiliano Pinto Added enable/disable feedback * 27/05/15 Martin Brampton Add show persistent [server] + * 06/11/15 Martin Brampton Add show buffers (conditional compilation) * * @endverbatim */ @@ -60,6 +61,7 @@ #include #include #include +#include #include #include #include @@ -108,6 +110,12 @@ static void telnetdShowUsers(DCB *); * The subcommands of the show command */ struct subcommand showoptions[] = { +#if defined(BUFFER_TRACE) + { "buffers", 0, dprintAllBuffers, + "Show all buffers with backtrace", + "Show all buffers with backtrace", + {0, 0, 0} }, +#endif { "dcbs", 0, dprintAllDCBs, "Show all descriptor control blocks (network connections)", "Show all descriptor control blocks (network connections)", diff --git a/server/modules/routing/maxinfo/maxinfo.c b/server/modules/routing/maxinfo/maxinfo.c index 96085a6b4..edb13e94b 100644 --- a/server/modules/routing/maxinfo/maxinfo.c +++ b/server/modules/routing/maxinfo/maxinfo.c @@ -26,6 +26,7 @@ * Date Who Description * 16/02/15 Mark Riddoch Initial implementation * 27/02/15 Massimiliano Pinto Added maxinfo_add_mysql_user + * 09/09/2015 Martin Brampton Modify error handler * * @endverbatim */ @@ -81,7 +82,7 @@ static void closeSession(ROUTER *instance, void *router_session); static void freeSession(ROUTER *instance, void *router_session); static int execute(ROUTER *instance, void *router_session, GWBUF *queue); static void diagnostics(ROUTER *instance, DCB *dcb); -static uint8_t getCapabilities (ROUTER* inst, void* router_session); +static int getCapabilities (); static void handleError( ROUTER *instance, void *router_session, @@ -286,7 +287,8 @@ static void freeSession( * @param router_session The router session * @param message The error message to reply * @param backend_dcb The backend DCB - * @param action The action: REPLY, REPLY_AND_CLOSE, NEW_CONNECTION + * @param action The action: ERRACT_NEW_CONNECTION or ERRACT_REPLY_CLIENT + * @param succp Result of action: true iff router can continue * */ static void handleError( @@ -302,13 +304,6 @@ static void handleError( SESSION *session = backend_dcb->session; session_state_t sesstate; - /** Reset error handle flag from a given DCB */ - if (action == ERRACT_RESET) - { - backend_dcb->dcb_errhandle_called = false; - return; - } - /** Don't handle same error twice on same DCB */ if (backend_dcb->dcb_errhandle_called) { @@ -336,6 +331,7 @@ static void handleError( } /** false because connection is not available anymore */ + dcb_close(backend_dcb); *succp = false; } @@ -419,10 +415,8 @@ diagnostics(ROUTER *instance, DCB *dcb) * * Not used for the maxinfo router */ -static uint8_t -getCapabilities( - ROUTER* inst, - void* router_session) +static int +getCapabilities() { return 0; } diff --git a/server/modules/routing/readconnroute.c b/server/modules/routing/readconnroute.c index 8cc75e17a..545cb5423 100644 --- a/server/modules/routing/readconnroute.c +++ b/server/modules/routing/readconnroute.c @@ -67,13 +67,17 @@ * 06/03/2014 Massimiliano Pinto Server connection counter is now updated in closeSession * 24/06/2014 Massimiliano Pinto New rules for selecting the Master server * 27/06/2014 Mark Riddoch Addition of server weighting - * 11/06/2015 Martin Brampton Remove decrement n_current (moved to dcb.c) + * 11/06/2015 Martin Brampton Remove decrement n_current (moved to dcb.c) + * 09/09/2015 Martin Brampton Modify error handler + * 25/09/2015 Martin Brampton Block callback processing when no router session in the DCB + * 09/11/2015 Martin Brampton Modified routeQuery - must free "queue" regardless of outcome * * @endverbatim */ #include #include #include +#include #include #include #include @@ -117,10 +121,10 @@ static void handleError( ROUTER *instance, void *router_session, GWBUF *errbuf, - DCB *backend_dcb, + DCB *problem_dcb, error_action_t action, bool *succp); -static uint8_t getCapabilities (ROUTER* inst, void* router_session); +static int getCapabilities (); /** The module object definition */ @@ -723,6 +727,7 @@ routeQuery(ROUTER *instance, void *router_session, GWBUF *queue) "server.%s", mysql_command,rses_is_closed ? " Session is closed." : ""))); rc = 0; + while((queue = GWBUF_CONSUME_ALL(queue)) != NULL); goto return_rc; } @@ -829,63 +834,68 @@ clientReply( /** * Error Handler routine * - * The routine will handle errors that occurred in backend writes. + * The routine will handle errors that occurred in writes. * * @param instance The router instance * @param router_session The router session * @param message The error message to reply - * @param backend_dcb The backend DCB - * @param action The action: REPLY, REPLY_AND_CLOSE, NEW_CONNECTION + * @param problem_dcb The DCB related to the error + * @param action The action: ERRACT_NEW_CONNECTION or ERRACT_REPLY_CLIENT + * @param succp Result of action: true if router can continue * */ static void handleError( - ROUTER *instance, - void *router_session, - GWBUF *errbuf, - DCB *backend_dcb, - error_action_t action, - bool *succp) + ROUTER *instance, + void *router_session, + GWBUF *errbuf, + DCB *problem_dcb, + error_action_t action, + bool *succp) { - DCB *client_dcb; - SESSION *session = backend_dcb->session; - session_state_t sesstate; + DCB *client_dcb; + SESSION *session = problem_dcb->session; + session_state_t sesstate; + ROUTER_CLIENT_SES *router_cli_ses = (ROUTER_CLIENT_SES *)router_session; - /** Reset error handle flag from a given DCB */ - if (action == ERRACT_RESET) - { - backend_dcb->dcb_errhandle_called = false; - return; - } + /** Don't handle same error twice on same DCB */ + if (problem_dcb->dcb_errhandle_called) + { + /** we optimistically assume that previous call succeed */ + *succp = true; + return; + } + else + { + problem_dcb->dcb_errhandle_called = true; + } + spinlock_acquire(&session->ses_lock); + sesstate = session->state; + client_dcb = session->client; - /** Don't handle same error twice on same DCB */ - if (backend_dcb->dcb_errhandle_called) - { - /** we optimistically assume that previous call succeed */ - *succp = true; - return; - } - else - { - backend_dcb->dcb_errhandle_called = true; - } - spinlock_acquire(&session->ses_lock); - sesstate = session->state; - client_dcb = session->client; - - if (sesstate == SESSION_STATE_ROUTER_READY) - { - CHK_DCB(client_dcb); - spinlock_release(&session->ses_lock); - client_dcb->func.write(client_dcb, gwbuf_clone(errbuf)); - } - else - { - spinlock_release(&session->ses_lock); - } - - /** false because connection is not available anymore */ - *succp = false; + if (sesstate == SESSION_STATE_ROUTER_READY) + { + CHK_DCB(client_dcb); + spinlock_release(&session->ses_lock); + client_dcb->func.write(client_dcb, gwbuf_clone(errbuf)); + } + else + { + spinlock_release(&session->ses_lock); + } + + if (dcb_isclient(problem_dcb)) + { + dcb_close(problem_dcb); + } + else if (router_cli_ses && problem_dcb == router_cli_ses->backend_dcb) + { + router_cli_ses->backend_dcb = NULL; + dcb_close(problem_dcb); + } + + /** false because connection is not available anymore */ + *succp = false; } /** to be inline'd */ @@ -947,11 +957,9 @@ static void rses_end_locked_router_action( } -static uint8_t getCapabilities( - ROUTER* inst, - void* router_session) +static int getCapabilities() { - return 0; + return RCAP_TYPE_PACKET_INPUT; } /******************************** @@ -993,6 +1001,14 @@ static int handle_state_switch(DCB* dcb,DCB_REASON reason, void * routersession) SERVICE* service = session->service; ROUTER* router = (ROUTER *)service->router; + if (NULL == dcb->session->router_session && DCB_REASON_ERROR != reason) + { + /* + * We cannot handle a DCB that does not have a router session, + * except in the case where error processing is invoked. + */ + return 0; + } switch(reason) { case DCB_REASON_CLOSE: diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index dc52f026a..086dc4514 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -65,6 +65,9 @@ MODULE_INFO info = { * 18/07/2013 Massimiliano Pinto routeQuery now handles COM_QUIT * as QUERY_TYPE_SESSION_WRITE * 17/07/2014 Massimiliano Pinto Server connection counter is updated in closeSession + * + * 09/09/2015 Martin Brampton Modify error handler + * 25/09/2015 Martin Brampton Block callback processing when no router session in the DCB * * @endverbatim */ @@ -127,7 +130,7 @@ static bool route_single_stmt( GWBUF* querybuf); -static uint8_t getCapabilities (ROUTER* inst, void* router_session); +static int getCapabilities(); #if defined(NOT_USED) static bool router_option_configured( @@ -909,7 +912,6 @@ static void* newSession( client_rses->rses_master_ref = master_ref; /* assert with master_host */ ss_dassert(master_ref && (master_ref->bref_backend->backend_server && SERVER_MASTER)); - client_rses->rses_capabilities = RCAP_TYPE_STMT_INPUT; client_rses->rses_backend_ref = backend_ref; client_rses->rses_nbackends = router_nservers; /*< # of backend servers */ @@ -987,7 +989,7 @@ static void closeSession( int i; /** * This sets router closed. Nobody is allowed to use router - * whithout checking this first. + * without checking this first. */ router_cli_ses->rses_closed = true; @@ -1427,6 +1429,8 @@ static route_target_t get_route_target ( QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ)|| /*< read user var */ QUERY_IS_TYPE(qtype, QUERY_TYPE_SYSVAR_READ) || /*< read sys var */ QUERY_IS_TYPE(qtype, QUERY_TYPE_EXEC_STMT) || /*< prepared stmt exec */ + QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_STMT) || + QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_NAMED_STMT) || QUERY_IS_TYPE(qtype, QUERY_TYPE_GSYSVAR_READ))) /*< read global sys var */ { /** First set expected targets before evaluating hints */ @@ -1444,6 +1448,8 @@ static route_target_t get_route_target ( if (QUERY_IS_TYPE(qtype, QUERY_TYPE_MASTER_READ) || QUERY_IS_TYPE(qtype, QUERY_TYPE_EXEC_STMT) || + QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_STMT) || + QUERY_IS_TYPE(qtype, QUERY_TYPE_PREPARE_NAMED_STMT) || /** Configured not to allow reading variables from slaves */ (use_sql_variables_in == TYPE_MASTER && (QUERY_IS_TYPE(qtype, QUERY_TYPE_USERVAR_READ) || @@ -3686,10 +3692,10 @@ static bool select_connect_backend_servers( ss_dassert(backend_ref[i].bref_backend->backend_conn_count > 0); /** disconnect opened connections */ - dcb_close(backend_ref[i].bref_dcb); bref_clear_state(&backend_ref[i], BREF_IN_USE); /** Decrease backend's connection counter. */ atomic_add(&backend_ref[i].bref_backend->backend_conn_count, -1); + dcb_close(backend_ref[i].bref_dcb); } } } @@ -4440,27 +4446,11 @@ static void tracelog_routed_query( /** - * Return rc, rc < 0 if router session is closed. rc == 0 if there are no - * capabilities specified, rc > 0 when there are capabilities. + * Return RCAP_TYPE_STMT_INPUT. */ -static uint8_t getCapabilities ( - ROUTER* inst, - void* router_session) +static int getCapabilities () { - ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *)router_session; - uint8_t rc; - - if (!rses_begin_locked_router_action(rses)) - { - rc = 0xff; - goto return_rc; - } - rc = rses->rses_capabilities; - - rses_end_locked_router_action(rses); - -return_rc: - return rc; + return RCAP_TYPE_STMT_INPUT; } /** @@ -4829,9 +4819,8 @@ static void rwsplit_process_router_options( * @param router_session The router session * @param errmsgbuf The error message to reply * @param backend_dcb The backend DCB - * @param action The action: REPLY, REPLY_AND_CLOSE, NEW_CONNECTION - * @param succp Result of action. True if there is at least master - * and enough slaves to continue session. Otherwise false. + * @param action The action: ERRACT_NEW_CONNECTION or ERRACT_REPLY_CLIENT + * @param succp Result of action: true iff router can continue * * Even if succp == true connecting to new slave may have failed. succp is to * tell whether router has enough master/slave connections to continue work. @@ -4850,17 +4839,14 @@ static void handleError ( CHK_DCB(backend_dcb); - /** Reset error handle flag from a given DCB */ - if (action == ERRACT_RESET) - { - backend_dcb->dcb_errhandle_called = false; - return; - } - /** Don't handle same error twice on same DCB */ if (backend_dcb->dcb_errhandle_called) { - /** we optimistically assume that previous call succeed */ + /** we optimistically assume that previous call succeed */ + /* + * The return of true is potentially misleading, but appears to + * be safe with the code as it stands on 9 Sept 2015 - MNB + */ *succp = true; return; } @@ -4873,12 +4859,13 @@ static void handleError ( if (session == NULL || rses == NULL) { *succp = false; - return; } - CHK_SESSION(session); - CHK_CLIENT_RSES(rses); + else + { + CHK_SESSION(session); + CHK_CLIENT_RSES(rses); - switch (action) { + switch (action) { case ERRACT_NEW_CONNECTION: { SERVER* srv; @@ -4886,7 +4873,7 @@ static void handleError ( if (!rses_begin_locked_router_action(rses)) { *succp = false; - return; + break; } srv = rses->rses_master_ref->bref_backend->backend_server; /** @@ -4896,6 +4883,25 @@ static void handleError ( if (rses->rses_master_ref->bref_dcb == backend_dcb && !SERVER_IS_MASTER(srv)) { + backend_ref_t* bref; + bref = get_bref_from_dcb(rses, backend_dcb); + if (bref != NULL) + { + CHK_BACKEND_REF(bref); + bref_clear_state(bref, BREF_IN_USE); + bref_set_state(bref, BREF_CLOSED); + } + else + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : server %s:%d lost the " + "master status but could not locate the " + "corresponding backend ref.", + srv->name, + srv->port))); + dcb_close(backend_dcb); + } if (!srv->master_err_is_logged) { LOGIF(LE, (skygw_log_write_flush( @@ -4939,7 +4945,9 @@ static void handleError ( default: *succp = false; break; + } } + dcb_close(backend_dcb); } @@ -4995,7 +5003,7 @@ static bool handle_error_new_connection( DCB* backend_dcb, GWBUF* errmsg) { - ROUTER_CLIENT_SES* myrses; + ROUTER_CLIENT_SES* myrses; SESSION* ses; int router_nservers; int max_nslaves; @@ -5003,7 +5011,7 @@ static bool handle_error_new_connection( backend_ref_t* bref; bool succp; - myrses = *rses; + myrses = *rses; ss_dassert(SPINLOCK_IS_LOCKED(&myrses->rses_lock)); ses = backend_dcb->session; @@ -5053,7 +5061,6 @@ static bool handle_error_new_connection( DCB_REASON_NOT_RESPONDING, &router_handle_state_switch, (void *)bref); - router_nservers = router_get_servercount(inst); max_nslaves = rses_get_max_slavecount(myrses, router_nservers); max_slave_rlag = rses_get_max_replication_lag(myrses); @@ -5311,9 +5318,16 @@ static int router_handle_state_switch( backend_ref_t* bref; int rc = 1; SERVER* srv; - ROUTER_CLIENT_SES* rses; - SESSION* ses; CHK_DCB(dcb); + if (NULL == dcb->session->router_session) + { + /* + * The following processing will fail if there is no router session, + * because the "data" parameter will not contain meaningful data, + * so we have no choice but to stop here. + */ + return 0; + } bref = (backend_ref_t *)data; CHK_BACKEND_REF(bref); @@ -5332,7 +5346,10 @@ static int router_handle_state_switch( srv->port, STRSRVSTATUS(srv)))); CHK_SESSION(((SESSION*)dcb->session)); - CHK_CLIENT_RSES(((ROUTER_CLIENT_SES *)dcb->session->router_session)); + if (dcb->session->router_session) + { + CHK_CLIENT_RSES(((ROUTER_CLIENT_SES *)dcb->session->router_session)); + } switch (reason) { case DCB_REASON_NOT_RESPONDING: diff --git a/server/modules/routing/schemarouter/schemarouter.c b/server/modules/routing/schemarouter/schemarouter.c index 544fa80df..3bf85d16c 100644 --- a/server/modules/routing/schemarouter/schemarouter.c +++ b/server/modules/routing/schemarouter/schemarouter.c @@ -61,6 +61,8 @@ MODULE_INFO info = { * * Date Who Description * 01/12/2014 Vilho Raatikka/Markus Mäkelä Initial implementation + * 09/09/2015 Martin Brampton Modify error handler + * 25/09/2015 Martin Brampton Block callback processing when no router session in the DCB * * @endverbatim */ @@ -97,7 +99,7 @@ static route_target_t get_shard_route_target ( bool trx_active, HINT* hint); -static uint8_t getCapabilities (ROUTER* inst, void* router_session); +static int getCapabilities (); static bool connect_backend_servers( backend_ref_t* backend_ref, @@ -1239,7 +1241,6 @@ static void* newSession( goto return_rses; } /** Copy backend pointers to router session. */ - client_rses->rses_capabilities = RCAP_TYPE_STMT_INPUT; client_rses->rses_backend_ref = backend_ref; client_rses->rses_nbackends = router_nservers; /*< # of backend servers */ @@ -3932,27 +3933,11 @@ static void tracelog_routed_query( /** - * Return rc, rc < 0 if router session is closed. rc == 0 if there are no - * capabilities specified, rc > 0 when there are capabilities. + * Return RCAP_TYPE_STMT_INPUT. */ -static uint8_t getCapabilities ( - ROUTER* inst, - void* router_session) +static int getCapabilities () { - ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *)router_session; - uint8_t rc; - - if (!rses_begin_locked_router_action(rses)) - { - rc = 0xff; - goto return_rc; - } - rc = rses->rses_capabilities; - - rses_end_locked_router_action(rses); - -return_rc: - return rc; + return RCAP_TYPE_STMT_INPUT; } /** @@ -4197,47 +4182,45 @@ return_succp: * @param router_session The router session * @param errmsgbuf The error message to reply * @param backend_dcb The backend DCB - * @param action The action: REPLY, REPLY_AND_CLOSE, NEW_CONNECTION - * @param succp Result of action. + * @param action The action: ERRACT_NEW_CONNECTION or ERRACT_REPLY_CLIENT + * @param succp Result of action: true iff router can continue * * Even if succp == true connecting to new slave may have failed. succp is to * tell whether router has enough master/slave connections to continue work. */ static void handleError ( - ROUTER* instance, - void* router_session, - GWBUF* errmsgbuf, - DCB* backend_dcb, - error_action_t action, - bool* succp) + ROUTER* instance, + void* router_session, + GWBUF* errmsgbuf, + DCB* backend_dcb, + error_action_t action, + bool* succp) { - SESSION* session; - ROUTER_INSTANCE* inst = (ROUTER_INSTANCE *)instance; - ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *)router_session; + SESSION* session; + ROUTER_INSTANCE* inst = (ROUTER_INSTANCE *)instance; + ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *)router_session; - CHK_DCB(backend_dcb); - if(succp == NULL || action == ERRACT_RESET) - { - return; - } - /** Don't handle same error twice on same DCB */ - if (backend_dcb->dcb_errhandle_called) - { - /** we optimistically assume that previous call succeed */ - *succp = true; - return; - } - else - { - backend_dcb->dcb_errhandle_called = true; - } - session = backend_dcb->session; + CHK_DCB(backend_dcb); - if (session == NULL || rses == NULL) - { - *succp = false; - return; - } + /** Don't handle same error twice on same DCB */ + if (backend_dcb->dcb_errhandle_called) + { + /** we optimistically assume that previous call succeed */ + *succp = true; + return; + } + else + { + backend_dcb->dcb_errhandle_called = true; + } + session = backend_dcb->session; + + if (session == NULL || rses == NULL) + { + *succp = false; + } + else + { CHK_SESSION(session); CHK_CLIENT_RSES(rses); @@ -4247,7 +4230,7 @@ static void handleError ( if (!rses_begin_locked_router_action(rses)) { *succp = false; - return; + break; } /** * This is called in hope of getting replacement for @@ -4275,6 +4258,8 @@ static void handleError ( *succp = false; break; } + } + dcb_close(backend_dcb); } @@ -4502,6 +4487,15 @@ router_handle_state_switch( SERVER* srv; CHK_DCB(dcb); + if (NULL == dcb->session->router_session) + { + /* + * The following processing will fail if there is no router session, + * because the "data" parameter will not contain meaningful data, + * so we have no choice but to stop here. + */ + return 0; + } bref = (backend_ref_t *) data; CHK_BACKEND_REF(bref); diff --git a/server/modules/routing/schemarouter/shardrouter.c b/server/modules/routing/schemarouter/shardrouter.c index 272f48664..8e7dd479f 100644 --- a/server/modules/routing/schemarouter/shardrouter.c +++ b/server/modules/routing/schemarouter/shardrouter.c @@ -59,6 +59,7 @@ MODULE_INFO info = { * * Date Who Description * 20/01/2015 Markus Mäkelä/Vilho Raatikka Initial implementation + * 09/09/2015 Martin Brampton Modify error handler * * @endverbatim */ @@ -115,7 +116,7 @@ static route_target_t get_shard_route_target( bool trx_active, HINT* hint); -static uint8_t getCapabilities(ROUTER* inst, void* router_session); +static int getCapabilities(); void subsvc_clear_state(SUBSERVICE* svc,subsvc_state_t state); void subsvc_set_state(SUBSERVICE* svc,subsvc_state_t state); @@ -1157,9 +1158,6 @@ newSession( free(dummy_upstream); } - - /** Copy backend pointers to router session. */ - client_rses->rses_capabilities = RCAP_TYPE_STMT_INPUT; router->stats.n_sessions += 1; /** @@ -2570,27 +2568,12 @@ mysql_sescmd_get_property( } /** - * Return rc, rc < 0 if router session is closed. rc == 0 if there are no - * capabilities specified, rc > 0 when there are capabilities. + * Return RCAP_TYPE_STMT_INPUT */ -static uint8_t -getCapabilities(ROUTER* inst, - void* router_session) +static int +getCapabilities() { - ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *) router_session; - uint8_t rc; - - if(!rses_begin_locked_router_action(rses)) - { - rc = 0xff; - goto return_rc; - } - rc = rses->rses_capabilities; - - rses_end_locked_router_action(rses); - -return_rc: - return rc; + return RCAP_TYPE_STMT_INPUT; } /** @@ -2772,8 +2755,8 @@ return_succp: * @param router_session The router session * @param errmsgbuf The error message to reply * @param backend_dcb The backend DCB - * @param action The action: REPLY, REPLY_AND_CLOSE, NEW_CONNECTION - * @param succp Result of action. + * @param action The action: ERRACT_NEW_CONNECTION or ERRACT_REPLY_CLIENT + * @param succp Result of action: true if router can continue * * Even if succp == true connecting to new slave may have failed. succp is to * tell whether router has enough master/slave connections to continue work. @@ -2790,10 +2773,8 @@ handleError( SESSION* session; ROUTER_CLIENT_SES* rses = (ROUTER_CLIENT_SES *) router_session; - if(action == ERRACT_RESET) - return; - CHK_DCB(backend_dcb); + /** Don't handle same error twice on same DCB */ if(backend_dcb->dcb_errhandle_called) { @@ -2809,38 +2790,39 @@ handleError( if(session == NULL || rses == NULL) { - if(succp) - *succp = false; - return; - } - CHK_SESSION(session); - CHK_CLIENT_RSES(rses); - - switch(action) - { - case ERRACT_NEW_CONNECTION: - { - if(!rses_begin_locked_router_action(rses)) - { - *succp = false; - return; - } - - rses_end_locked_router_action(rses); - break; - } - - case ERRACT_REPLY_CLIENT: - { - - *succp = false; /*< no new backend servers were made available */ - break; - } - - default: *succp = false; - break; } + else + { + CHK_SESSION(session); + CHK_CLIENT_RSES(rses); + + switch(action) + { + case ERRACT_NEW_CONNECTION: + { + if(!rses_begin_locked_router_action(rses)) + { + *succp = false; + break; + } + + rses_end_locked_router_action(rses); + break; + } + + case ERRACT_REPLY_CLIENT: + { + *succp = false; /*< no new backend servers were made available */ + break; + } + + default: + *succp = false; + break; + } + } + dcb_close(backend_dcb); } diff --git a/server/modules/routing/testroute.c b/server/modules/routing/testroute.c index 968e3892c..b0d963601 100644 --- a/server/modules/routing/testroute.c +++ b/server/modules/routing/testroute.c @@ -35,7 +35,7 @@ static void freeSession(ROUTER *instance, void *session); static int routeQuery(ROUTER *instance, void *session, GWBUF *queue); static void clientReply(ROUTER *instance, void *session, GWBUF *queue,DCB*); static void diagnostic(ROUTER *instance, DCB *dcb); -static uint8_t getCapabilities (ROUTER* inst, void* router_session); +static int getCapabilities (); static void handleError( ROUTER *instance, void *router_session, @@ -166,9 +166,7 @@ diagnostic(ROUTER *instance, DCB *dcb) { } -static uint8_t getCapabilities( - ROUTER* inst, - void* router_session) +static int getCapabilities() { return 0; } diff --git a/utils/skygw_debug.h b/utils/skygw_debug.h index fe99fcd85..359d98cdd 100644 --- a/utils/skygw_debug.h +++ b/utils/skygw_debug.h @@ -206,16 +206,16 @@ typedef enum skygw_chk_t { ((s) == DCB_STATE_LISTENING ? "DCB_STATE_LISTENING" : \ ((s) == DCB_STATE_DISCONNECTED ? "DCB_STATE_DISCONNECTED" : \ ((s) == DCB_STATE_NOPOLLING ? "DCB_STATE_NOPOLLING" : \ - ((s) == DCB_STATE_FREED ? "DCB_STATE_FREED" : \ - ((s) == DCB_STATE_ZOMBIE ? "DCB_STATE_ZOMBIE" : \ - ((s) == DCB_STATE_UNDEFINED ? "DCB_STATE_UNDEFINED" : "DCB_STATE_UNKNOWN")))))))) + ((s) == DCB_STATE_ZOMBIE ? "DCB_STATE_ZOMBIE" : \ + ((s) == DCB_STATE_UNDEFINED ? "DCB_STATE_UNDEFINED" : "DCB_STATE_UNKNOWN"))))))) #define STRSESSIONSTATE(s) ((s) == SESSION_STATE_ALLOC ? "SESSION_STATE_ALLOC" : \ - ((s) == SESSION_STATE_READY ? "SESSION_STATE_READY" : \ - ((s) == SESSION_STATE_LISTENER ? "SESSION_STATE_LISTENER" : \ - ((s) == SESSION_STATE_LISTENER_STOPPED ? "SESSION_STATE_LISTENER_STOPPED" : \ - (s) == SESSION_STATE_ROUTER_READY ? "SESSION_STATE_ROUTER_READY":\ - "SESSION_STATE_UNKNOWN")))) + ((s) == SESSION_STATE_DUMMY ? "SESSION_STATE_DUMMY" : \ + ((s) == SESSION_STATE_READY ? "SESSION_STATE_READY" : \ + ((s) == SESSION_STATE_LISTENER ? "SESSION_STATE_LISTENER" : \ + ((s) == SESSION_STATE_LISTENER_STOPPED ? "SESSION_STATE_LISTENER_STOPPED" : \ + (s) == SESSION_STATE_ROUTER_READY ? "SESSION_STATE_ROUTER_READY":\ + "SESSION_STATE_UNKNOWN"))))) #define STRPROTOCOLSTATE(s) ((s) == MYSQL_ALLOC ? "MYSQL_ALLOC" : \ ((s) == MYSQL_PENDING_CONNECT ? "MYSQL_PENDING_CONNECT" : \ diff --git a/utils/skygw_utils.cc b/utils/skygw_utils.cc index 95391039e..a082d1cf8 100644 --- a/utils/skygw_utils.cc +++ b/utils/skygw_utils.cc @@ -33,6 +33,7 @@ #include #include "skygw_utils.h" #include +#include #include #if defined(MLIST) @@ -1276,7 +1277,7 @@ void acquire_lock( misscount += 1; if (misscount > 10) { - ts1.tv_nsec = (rand()%misscount)*1000000; + ts1.tv_nsec = (random_jkiss()%misscount)*1000000; nanosleep(&ts1, NULL); } }