From 7b3c287ac3af4002f55ac7486e9db9fb958660b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 13 Mar 2017 22:56:18 +0200 Subject: [PATCH] Close sessions in MaxScale core The core now provides a simple function to close a session. This removes the need for the modules to directly call the API entry points when the session should be closed. It is also in line with the style that other objects, namely the DCBs, use. This makes the new session_close very similar to dcb_close. --- include/maxscale/session.h | 11 +++++ server/core/session.c | 17 +++++++ server/modules/filter/tee/tee.c | 17 +------ .../MySQL/MySQLBackend/mysql_backend.c | 47 +++++++----------- .../protocol/MySQL/MySQLClient/mysql_client.c | 48 ++----------------- 5 files changed, 50 insertions(+), 90 deletions(-) diff --git a/include/maxscale/session.h b/include/maxscale/session.h index a85b9292a..92d38002a 100644 --- a/include/maxscale/session.h +++ b/include/maxscale/session.h @@ -324,6 +324,17 @@ static inline bool session_set_autocommit(MXS_SESSION* ses, bool autocommit) */ MXS_SESSION* session_get_by_id(int id); +/** + * @brief Close a session + * + * Calling this function will start the session shutdown process. The shutdown + * closes all related backend DCBs by calling the closeSession entry point + * of the router session. + * + * @param session The session to close + */ +void session_close(MXS_SESSION *session); + /** * @brief Release a session reference * diff --git a/server/core/session.c b/server/core/session.c index fff7f9832..d286f2507 100644 --- a/server/core/session.c +++ b/server/core/session.c @@ -303,6 +303,23 @@ session_simple_free(MXS_SESSION *session, DCB *dcb) session_final_free(session); } +void session_close(MXS_SESSION *session) +{ + if (!session->ses_is_child && session->router_session) + { + if (session->state != SESSION_STATE_STOPPING) + { + session->state = SESSION_STATE_STOPPING; + } + + MXS_ROUTER_OBJECT* router = session->service->router; + void* router_instance = session->service->router_instance; + + /** Close router session and all its connections */ + router->closeSession(router_instance, session->router_session); + } +} + /** * Deallocate the specified session * diff --git a/server/modules/filter/tee/tee.c b/server/modules/filter/tee/tee.c index 768c6b88a..da8a961da 100644 --- a/server/modules/filter/tee/tee.c +++ b/server/modules/filter/tee/tee.c @@ -536,22 +536,9 @@ closeSession(MXS_FILTER *instance, MXS_FILTER_SESSION *session) if ((bsession = my_session->branch_session) != NULL) { CHK_SESSION(bsession); - - if (bsession->state != SESSION_STATE_STOPPING) - { - bsession->state = SESSION_STATE_STOPPING; - } - router = bsession->service->router; - router_instance = bsession->service->router_instance; - rsession = bsession->router_session; - - /** Close router session and all its connections */ - router->closeSession(router_instance, rsession); + bsession->ses_is_child = false; + session_close(bsession); } - /* No need to free the session, this is done as - * a side effect of closing the client DCB of the - * session. - */ if (my_session->waiting[PARENT]) { diff --git a/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c b/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c index bed3034f8..56ccb4a75 100644 --- a/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c +++ b/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c @@ -1112,46 +1112,31 @@ static int gw_backend_hangup(DCB *dcb) */ static int gw_backend_close(DCB *dcb) { - MXS_SESSION* session; - GWBUF* quitbuf; - CHK_DCB(dcb); - session = dcb->session; - - MXS_DEBUG("%lu [gw_backend_close]", pthread_self()); - - quitbuf = mysql_create_com_quit(NULL, 0); - gwbuf_set_type(quitbuf, GWBUF_TYPE_MYSQL); + ss_dassert(dcb->session); /** Send COM_QUIT to the backend being closed */ + GWBUF* quitbuf = mysql_create_com_quit(NULL, 0); + gwbuf_set_type(quitbuf, GWBUF_TYPE_MYSQL); mysql_send_com_quit(dcb, 0, quitbuf); + /** Free protocol data */ mysql_protocol_done(dcb); - if (session) - { - CHK_SESSION(session); - /** - * The lock is needed only to protect the read of session->state and - * session->client_dcb values. Client's state may change by other thread - * but client's close and adding client's DCB to zombies list is executed - * only if client's DCB's state does _not_ change in parallel. - */ + MXS_SESSION* session = dcb->session; + CHK_SESSION(session); - /** - * If session->state is STOPPING, start closing client session. - * Otherwise only this backend connection is closed. - */ - if (session->state == SESSION_STATE_STOPPING && - session->client_dcb != NULL) - { - if (session->client_dcb->state == DCB_STATE_POLLING) - { - /** Close client DCB */ - dcb_close(session->client_dcb); - } - } + /** + * If session state is SESSION_STATE_STOPPING, start closing client session. + * Otherwise only this backend connection is closed. + */ + if (session->client_dcb && + session->state == SESSION_STATE_STOPPING && + session->client_dcb->state == DCB_STATE_POLLING) + { + dcb_close(session->client_dcb); } + return 1; } diff --git a/server/modules/protocol/MySQL/MySQLClient/mysql_client.c b/server/modules/protocol/MySQL/MySQLClient/mysql_client.c index a9aaff2ab..2f3888c27 100644 --- a/server/modules/protocol/MySQL/MySQLClient/mysql_client.c +++ b/server/modules/protocol/MySQL/MySQLClient/mysql_client.c @@ -1347,52 +1347,12 @@ retblock: return 1; } -static int -gw_client_close(DCB *dcb) +static int gw_client_close(DCB *dcb) { - MXS_SESSION* session; - MXS_ROUTER_OBJECT* router; - void* router_instance; -#if defined(SS_DEBUG) - MySQLProtocol* protocol = (MySQLProtocol *)dcb->protocol; - - if (dcb->state == DCB_STATE_POLLING || - dcb->state == DCB_STATE_NOPOLLING || - dcb->state == DCB_STATE_ZOMBIE) - { - if (!DCB_IS_CLONE(dcb)) - { - CHK_PROTOCOL(protocol); - } - } -#endif - MXS_DEBUG("%lu [gw_client_close]", pthread_self()); + CHK_DCB(dcb); + ss_dassert(dcb->protocol); mysql_protocol_done(dcb); - session = dcb->session; - /** - * session may be NULL if session_alloc failed. - * In that case, router session wasn't created. - */ - if (session != NULL && SESSION_STATE_DUMMY != session->state) - { - CHK_SESSION(session); - - if (session->state != SESSION_STATE_STOPPING) - { - session->state = SESSION_STATE_STOPPING; - } - router_instance = session->service->router_instance; - router = session->service->router; - /** - * If router session is being created concurrently router - * session might be NULL and it shouldn't be closed. - */ - if (session->router_session != NULL) - { - /** Close router session and all its connections */ - router->closeSession(router_instance, session->router_session); - } - } + session_close(dcb->session); return 1; }