diff --git a/server/core/dcb.cc b/server/core/dcb.cc index f466d2135..de0572e6d 100644 --- a/server/core/dcb.cc +++ b/server/core/dcb.cc @@ -2533,11 +2533,6 @@ DCB* dcb_accept(DCB* dcb) } } - if (client_dcb) - { - mxb::atomic::add(&client_dcb->service->client_count, 1); - } - return client_dcb; } diff --git a/server/core/session.cc b/server/core/session.cc index 70da75a36..16b7c0654 100644 --- a/server/core/session.cc +++ b/server/core/session.cc @@ -361,18 +361,7 @@ private: static void session_free(MXS_SESSION* session) { MXS_INFO("Stopped %s client session [%" PRIu64 "]", session->service->name, session->ses_id); - Service* service = static_cast(session->service); - session_final_free(session); - bool should_destroy = !mxb::atomic::load(&service->active); - - if (mxb::atomic::add(&service->client_count, -1) == 1 && should_destroy) - { - // Destroy the service in the main routing worker thread - mxs::RoutingWorker* main_worker = mxs::RoutingWorker::get(mxs::RoutingWorker::MAIN); - main_worker->execute(std::unique_ptr(new ServiceDestroyTask(service)), - Worker::EXECUTE_AUTO); - } } static void session_final_free(MXS_SESSION* ses) @@ -1264,6 +1253,7 @@ Session::Session(SERVICE* service) mxb::atomic::add(&service->stats.n_current, 1, mxb::atomic::RELAXED); mxb_assert(service->stats.n_current >= 0); + mxb::atomic::add(&service->client_count, 1, mxb::atomic::RELAXED); } Session::~Session() @@ -1281,6 +1271,17 @@ Session::~Session() mxb::atomic::add(&service->stats.n_current, -1, mxb::atomic::RELAXED); mxb_assert(service->stats.n_current >= 0); + + bool should_destroy = !mxb::atomic::load(&service->active); + + if (mxb::atomic::add(&service->client_count, -1) == 1 && should_destroy) + { + // Destroy the service in the main routing worker thread + mxs::RoutingWorker* main_worker = mxs::RoutingWorker::get(mxs::RoutingWorker::MAIN); + main_worker->execute( + std::unique_ptr(new ServiceDestroyTask(static_cast(service))), + Worker::EXECUTE_AUTO); + } } namespace diff --git a/server/modules/filter/test/mock_session.cc b/server/modules/filter/test/mock_session.cc index aaa3d233a..c4b445795 100644 --- a/server/modules/filter/test/mock_session.cc +++ b/server/modules/filter/test/mock_session.cc @@ -44,6 +44,9 @@ Session::Session(Client* pClient) m_client_dcb.data = &m_mysql_session; service = &dummy_service; + + // This prevents the destruction of the dummy service + service->active = true; } Session::~Session() diff --git a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc index 638a235ca..9bf24c96c 100644 --- a/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc +++ b/server/modules/protocol/MySQL/mariadbclient/mysql_client.cc @@ -740,20 +740,12 @@ static void check_packet(DCB* dcb, GWBUF* buf, int bytes) */ static int gw_read_do_authentication(DCB* dcb, GWBUF* read_buffer, int nbytes_read) { - // If this function fails and dcb is closed, the service client count must be decremented manually. - // After authentication, the session closing code decrements the count. - auto decrement_and_close = [](DCB* dcb) - { - mxb::atomic::add(&dcb->service->client_count, -1); - dcb_close(dcb); - }; - MXB_AT_DEBUG(check_packet(dcb, read_buffer, nbytes_read)); /** Allocate the shared session structure */ if (dcb->data == NULL && (dcb->data = mysql_session_alloc()) == NULL) { - decrement_and_close(dcb); + dcb_close(dcb); return 1; } @@ -820,7 +812,7 @@ static int gw_read_do_authentication(DCB* dcb, GWBUF* read_buffer, int nbytes_re MYSQL_session* ses = (MYSQL_session*)dcb->data; if ((dcb->user = MXS_STRDUP(ses->user)) == NULL) { - decrement_and_close(dcb); + dcb_close(dcb); gwbuf_free(read_buffer); return 0; } @@ -873,7 +865,7 @@ static int gw_read_do_authentication(DCB* dcb, GWBUF* read_buffer, int nbytes_re /** * Close DCB and which will release MYSQL_session */ - decrement_and_close(dcb); + dcb_close(dcb); } /* One way or another, the buffer is now fully processed */ gwbuf_free(read_buffer);