diff --git a/include/maxscale/session.hh b/include/maxscale/session.hh index 21068ac0c..e6f01d6e4 100644 --- a/include/maxscale/session.hh +++ b/include/maxscale/session.hh @@ -41,7 +41,6 @@ typedef enum 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_DUMMY /*< dummy session for consistency */ } mxs_session_state_t; #define STRSESSIONSTATE(s) \ @@ -79,14 +78,7 @@ typedef enum SESSION_STATE_FREE \ ? \ "SESSION_STATE_TO_BE_FREE" \ - : (( \ - s) \ - == \ - SESSION_STATE_DUMMY \ - ? \ - "SESSION_STATE_DUMMY" \ - : \ - "SESSION_STATE_UNKNOWN"))))))))) + : "SESSION_STATE_UNKNOWN")))))))) typedef enum { @@ -207,6 +199,9 @@ typedef char* (* session_variable_handler_t)(void* context, */ struct MXS_SESSION { + MXS_SESSION(DCB* client_dcb); + ~MXS_SESSION(); + mxs_session_state_t state; /*< Current descriptor state */ uint64_t ses_id; /*< Unique session identifier */ DCB* client_dcb; /*< The client connection */ @@ -304,13 +299,6 @@ MXS_SESSION* session_alloc(SERVICE*, DCB*); */ bool session_start(MXS_SESSION* session); -MXS_SESSION* session_set_dummy(DCB*); - -static inline bool session_is_dummy(MXS_SESSION* session) -{ - return session->state == SESSION_STATE_DUMMY; -} - const char* session_get_remote(const MXS_SESSION*); const char* session_get_user(const MXS_SESSION*); diff --git a/server/core/dcb.cc b/server/core/dcb.cc index 240f4179f..ca1decb29 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -150,14 +150,13 @@ static MXB_WORKER* get_dcb_owner(dcb_role_t role) DCB::DCB(dcb_role_t role, const SListener& listener, SERVICE* service) : MXB_POLL_DATA{dcb_poll_handler, get_dcb_owner(role)} , dcb_role(role) + , session(nullptr) , listener(listener) , high_water(config_writeq_high_water()) , low_water(config_writeq_low_water()) , service(service) , last_read(mxs_clock()) { - session_set_dummy(this); - // TODO: Remove DCB_ROLE_INTERNAL to always have a valid listener if (listener) { @@ -245,25 +244,22 @@ static void dcb_final_free(DCB* dcb) */ MXS_SESSION* local_session = dcb->session; dcb->session = NULL; - if (SESSION_STATE_DUMMY != local_session->state) + if (dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER) { - if (dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER) - { - session_unlink_backend_dcb(local_session, dcb); - } - else - { - /** - * The client DCB is only freed once all other DCBs that the session - * uses have been freed. This will guarantee that the authentication - * data will be usable for all DCBs even if the client DCB has already - * been closed. - */ + session_unlink_backend_dcb(local_session, dcb); + } + else + { + /** + * The client DCB is only freed once all other DCBs that the session + * uses have been freed. This will guarantee that the authentication + * data will be usable for all DCBs even if the client DCB has already + * been closed. + */ - mxb_assert(dcb->dcb_role == DCB_ROLE_CLIENT_HANDLER || dcb->dcb_role == DCB_ROLE_INTERNAL); - session_put_ref(local_session); - return; - } + mxb_assert(dcb->dcb_role == DCB_ROLE_CLIENT_HANDLER || dcb->dcb_role == DCB_ROLE_INTERNAL); + session_put_ref(local_session); + return; } } @@ -1237,18 +1233,8 @@ static bool dcb_maybe_add_persistent(DCB* dcb) MXS_DEBUG("Adding DCB to persistent pool, user %s.", dcb->user); dcb->was_persistent = false; dcb->persistentstart = time(NULL); - if (dcb->session) - /*< - * Terminate client session. - */ - { - MXS_SESSION* local_session = dcb->session; - session_set_dummy(dcb); - if (SESSION_STATE_DUMMY != local_session->state) - { - session_unlink_backend_dcb(local_session, dcb); - } - } + session_unlink_backend_dcb(dcb->session, dcb); + dcb->session = nullptr; while ((loopcallback = dcb->callbacks) != NULL) { @@ -1563,7 +1549,7 @@ void dprintDCB(DCB* pdcb, DCB* dcb) dcb_printf(pdcb, "\tProtocol: %s\n", dcb->listener->protocol()); } - if (dcb->session && dcb->session->state != SESSION_STATE_DUMMY) + if (dcb->session) { dcb_printf(pdcb, "\tOwning Session: %" PRIu64 "\n", dcb->session->ses_id); } @@ -2493,9 +2479,7 @@ public: dcb && atomic_load_int32(&m_more); dcb = dcb->thread.next) { - mxb_assert(dcb->session); - - if (dcb->session->state != SESSION_STATE_DUMMY) + if (dcb->session) { if (!m_func(dcb, m_data)) { @@ -2503,6 +2487,10 @@ public: break; } } + else + { + mxb_assert_message(dcb->persistentstart > 0, "The DCB must be in a connection pool"); + } } } diff --git a/server/core/internal/session.hh b/server/core/internal/session.hh index 8d7b19682..dc2d3a858 100644 --- a/server/core/internal/session.hh +++ b/server/core/internal/session.hh @@ -164,7 +164,7 @@ public: using FilterList = std::vector; - Session(SERVICE* service); + Session(SERVICE* service, DCB* client_dcb); ~Session(); bool setup_filters(Service* service); diff --git a/server/core/session.cc b/server/core/session.cc index f3ef5a652..8793e4f04 100644 --- a/server/core/session.cc +++ b/server/core/session.cc @@ -69,18 +69,8 @@ struct SESSION_DUMP_STATEMENTS_NEVER }; -static MXS_SESSION dummy_session() -{ - MXS_SESSION session = {}; - session.state = SESSION_STATE_DUMMY; - session.refcount = 1; - return session; } -} - -static MXS_SESSION session_dummy_struct = dummy_session(); - static void session_initialize(void* session); static int session_setup_filters(MXS_SESSION* session); static void session_simple_free(MXS_SESSION* session, DCB* dcb); @@ -98,50 +88,33 @@ static void session_deliver_response(MXS_SESSION* session); */ static int session_reply(MXS_FILTER* inst, MXS_FILTER_SESSION* session, GWBUF* data); +MXS_SESSION::MXS_SESSION(DCB* client_dcb) + : state(SESSION_STATE_READY) + , ses_id(session_get_next_id()) + , client_dcb(client_dcb) + , router_session(nullptr) + , stats{time(0)} + , service(client_dcb->service) + , head{} + , tail{} + , refcount(1) + , trx_state(SESSION_TRX_INACTIVE) + , autocommit(config_get_global_options()->qc_sql_mode == QC_SQL_MODE_ORACLE ? false : true) + , client_protocol_data(0) + , qualifies_for_pooling(false) + , response{} + , close_reason(SESSION_CLOSE_NONE) + , load_active(false) +{ +} + +MXS_SESSION::~MXS_SESSION() +{ +} + MXS_SESSION* session_alloc(SERVICE* service, DCB* client_dcb) { - Session* session = new (std::nothrow) Session(service); - - if (session == nullptr) - { - return NULL; - } - - session->state = SESSION_STATE_READY; - session->ses_id = session_get_next_id(); - session->client_dcb = client_dcb; - session->router_session = NULL; - session->stats.connect = time(0); - session->service = service; - memset(&session->head, 0, sizeof(session->head)); - memset(&session->tail, 0, sizeof(session->tail)); - session->load_active = false; - - /*< - * 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. - * - * Note: Strictly speaking, we do have to increment it with a relaxed atomic - * operation but in practice it doesn't matter. - */ - session->refcount = 1; - session->trx_state = SESSION_TRX_INACTIVE; - - MXS_CONFIG* config = config_get_global_options(); - // If MaxScale is running in Oracle mode, then autocommit needs to - // initially be off. - bool autocommit = (config->qc_sql_mode == QC_SQL_MODE_ORACLE) ? false : true; - session_set_autocommit(session, autocommit); - - session->client_protocol_data = 0; - session->qualifies_for_pooling = false; - memset(&session->response, 0, sizeof(session->response)); - session->close_reason = SESSION_CLOSE_NONE; - - return session; + return new(std::nothrow) Session(service, client_dcb); } bool session_start(MXS_SESSION* session) @@ -200,21 +173,6 @@ bool session_start(MXS_SESSION* session) return true; } -/** - * 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 - */ -MXS_SESSION* session_set_dummy(DCB* client_dcb) -{ - MXS_SESSION* session = &session_dummy_struct; - client_dcb->session = session; - return session; -} - void session_link_backend_dcb(MXS_SESSION* session, DCB* dcb) { mxb_assert(dcb->dcb_role == DCB_ROLE_BACKEND_HANDLER); @@ -255,11 +213,7 @@ static void session_simple_free(MXS_SESSION* session, DCB* dcb) } if (session) { - if (SESSION_STATE_DUMMY == session->state) - { - return; - } - if (session && session->router_session) + if (session->router_session) { session->service->router->freeSession(session->service->router_instance, session->router_session); @@ -402,8 +356,7 @@ void printAllSessions() /** Callback for dprintAllSessions */ bool dprintAllSessions_cb(DCB* dcb, void* data) { - if (dcb->dcb_role == DCB_ROLE_CLIENT_HANDLER - && dcb->session->state != SESSION_STATE_DUMMY) + if (dcb->dcb_role == DCB_ROLE_CLIENT_HANDLER) { DCB* out_dcb = (DCB*)data; dprintSession(out_dcb, dcb->session); @@ -520,9 +473,6 @@ const char* session_state(mxs_session_state_t state) case SESSION_STATE_ALLOC: return "Session Allocated"; - case SESSION_STATE_DUMMY: - return "Dummy Session"; - case SESSION_STATE_READY: return "Session Ready"; @@ -765,7 +715,7 @@ MXS_SESSION* session_get_ref(MXS_SESSION* session) void session_put_ref(MXS_SESSION* session) { - if (session && session->state != SESSION_STATE_DUMMY) + if (session) { /** Remove one reference. If there are no references left, free session */ if (mxb::atomic::add(&session->refcount, -1) == 1) @@ -904,7 +854,6 @@ void session_qualify_for_pool(MXS_SESSION* session) bool session_valid_for_pool(const MXS_SESSION* session) { - mxb_assert(session->state != SESSION_STATE_DUMMY); return session->qualifies_for_pooling; } @@ -1148,7 +1097,8 @@ const char* session_get_close_reason(const MXS_SESSION* session) } } -Session::Session(SERVICE* service) +Session::Session(SERVICE* service, DCB* client_dcb) + : MXS_SESSION(client_dcb) { if (service->retain_last_statements != -1) // Explicitly set for the service { diff --git a/server/modules/filter/test/mock_session.cc b/server/modules/filter/test/mock_session.cc index 29686de51..cd8f40d71 100644 --- a/server/modules/filter/test/mock_session.cc +++ b/server/modules/filter/test/mock_session.cc @@ -27,13 +27,13 @@ namespace mock { Session::Session(Client* pClient) - : mxs::Session(&dummy_service) + : mxs::Session(&dummy_service, &m_client_dcb) , m_client(*pClient) , m_client_dcb(this, pClient->user(), pClient->host(), pClient) { MXS_SESSION* pSession = this; - memset(pSession, 0, sizeof(MXS_SESSION)); + memset((void*)pSession, 0, sizeof(MXS_SESSION)); pSession->state = SESSION_STATE_ALLOC; diff --git a/server/modules/protocol/CDC/cdc.cc b/server/modules/protocol/CDC/cdc.cc index 6d7885af3..5e482eb62 100644 --- a/server/modules/protocol/CDC/cdc.cc +++ b/server/modules/protocol/CDC/cdc.cc @@ -293,9 +293,6 @@ static int cdc_accept(DCB* client_dcb) client_dcb->protocol = (CDC_protocol*) protocol; - /* Dummy session */ - client_dcb->session = session_set_dummy(client_dcb); - if (NULL == client_dcb->session || poll_add_dcb(client_dcb)) { dcb_close(client_dcb); diff --git a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc index 4bbbf3c18..8764f18c0 100644 --- a/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc +++ b/server/modules/protocol/MySQL/mariadbbackend/mysql_backend.cc @@ -347,11 +347,8 @@ static void handle_error_response(DCB* dcb, GWBUF* buffer) || errcode == ER_DBACCESS_DENIED_ERROR || errcode == ER_ACCESS_DENIED_NO_PASSWORD_ERROR) { - if (dcb->session->state != SESSION_STATE_DUMMY) - { - // Authentication failed, reload users - service_refresh_users(dcb->service); - } + // Authentication failed, reload users + service_refresh_users(dcb->service); } } @@ -404,12 +401,8 @@ static inline void prepare_for_write(DCB* dcb, GWBUF* buffer) { MySQLProtocol* proto = (MySQLProtocol*)dcb->protocol; - /** - * The DCB's session is set to the dummy session when it is put into the - * persistent connection pool. If this is not the dummy session, track - * the current command being executed. - */ - if (!session_is_dummy(dcb->session)) + // The DCB's session is set to null when it is put into the persistent connection pool. + if (dcb->session) { uint64_t capabilities = service_get_capabilities(dcb->session->service); @@ -486,11 +479,7 @@ static int gw_read_backend_event(DCB* dcb) return 0; } - if (dcb->session == NULL - || dcb->session->state == SESSION_STATE_DUMMY) - { - return 0; - } + mxb_assert(dcb->session); MySQLProtocol* proto = (MySQLProtocol*)dcb->protocol; @@ -1299,7 +1288,7 @@ static int gw_error_backend_event(DCB* dcb) { MXS_SESSION* session = dcb->session; - if (session->state == SESSION_STATE_DUMMY) + if (!session) { if (dcb->persistentstart == 0) { diff --git a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc index ee4bedad7..083deab0d 100644 --- a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc +++ b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc @@ -774,8 +774,7 @@ static int gw_read_do_authentication(DCB* dcb, GWBUF* read_buffer, int nbytes_re */ if (session_start(dcb->session)) { - mxb_assert(dcb->session->state != SESSION_STATE_ALLOC - && dcb->session->state != SESSION_STATE_DUMMY); + mxb_assert(dcb->session->state != SESSION_STATE_ALLOC); // For the time being only the sql_mode is stored in MXS_SESSION::client_protocol_data. dcb->session->client_protocol_data = QC_SQL_MODE_DEFAULT; protocol->protocol_auth_state = MXS_AUTH_STATE_COMPLETE; @@ -1464,8 +1463,7 @@ static int gw_client_close(DCB* dcb) { MXS_SESSION* target = dcb->session; - if (target->state != SESSION_STATE_TO_BE_FREED - && target->state != SESSION_STATE_DUMMY) + if (target->state != SESSION_STATE_TO_BE_FREED) { mxb_assert(target->state == SESSION_STATE_ROUTER_READY || target->state == SESSION_STATE_STOPPING); @@ -1492,7 +1490,7 @@ static int gw_client_hangup_event(DCB* dcb) if (session) { - if (session->state != SESSION_STATE_DUMMY && !session_valid_for_pool(session)) + if (!session_valid_for_pool(session)) { if (session_get_dump_statements() == SESSION_DUMP_STATEMENTS_ON_ERROR) { diff --git a/server/modules/protocol/MySQL/mysql_common.cc b/server/modules/protocol/MySQL/mysql_common.cc index 6aa68e675..8dc061450 100644 --- a/server/modules/protocol/MySQL/mysql_common.cc +++ b/server/modules/protocol/MySQL/mysql_common.cc @@ -596,8 +596,7 @@ bool gw_get_shared_session_auth_info(DCB* dcb, MYSQL_session* session) mxb_assert(dcb->data); memcpy(session, dcb->data, sizeof(MYSQL_session)); } - else if (dcb->session->state != SESSION_STATE_ALLOC - && dcb->session->state != SESSION_STATE_DUMMY) + else if (dcb->session->state != SESSION_STATE_ALLOC) { memcpy(session, dcb->session->client_dcb->data, sizeof(MYSQL_session)); }