From dffb019d279df9b5f79e09bc8dbbacc229f887a7 Mon Sep 17 00:00:00 2001 From: Nischal Sheth Date: Mon, 8 Jun 2015 10:49:43 -0700 Subject: [PATCH] Fix concurrency issue in XmppSession::WriteReady It's possible that XmppSession::WriteReady gets called after the xmpp connection for the session has already been deleted. Prevent WriteReady from from accessing freed memory by clearing the back pointer to the xmpp connection in the session when the session in the xmpp connection is cleared. The above fix is not sufficient since there's a concurrency issue wherein XmppSession::WriteReady could try to access the connection the connection is trying to clear the back pointer in the session. Fix is to make XmppSession::WriteReady enqueue sessions to a work queue and process the work queue in context of bgp::Config task. Add XmppConnectionManager class so that this can be done for both XmppClient and XmppServer. Other common functionality can be moved into this class in future. Remove dead code for XmppStream. Change-Id: Id9ea3311e6618a0afc20058c3571330a1627fcec Closes-Bug: 1463122 --- src/bgp/test/bgp_xmpp_basic_test.cc | 4 +- src/xmpp/SConscript | 1 + src/xmpp/test/xmpp_regex_test.cc | 4 +- src/xmpp/xmpp_client.cc | 10 ++-- src/xmpp/xmpp_client.h | 7 ++- src/xmpp/xmpp_connection.cc | 16 +++++- src/xmpp/xmpp_connection.h | 3 +- src/xmpp/xmpp_connection_manager.cc | 80 +++++++++++++++++++++++++++++ src/xmpp/xmpp_connection_manager.h | 37 +++++++++++++ src/xmpp/xmpp_server.cc | 52 +++++++++++-------- src/xmpp/xmpp_server.h | 13 +++-- src/xmpp/xmpp_session.cc | 57 +++++++++++++++----- src/xmpp/xmpp_session.h | 49 +++++------------- src/xmpp/xmpp_state_machine.cc | 5 +- src/xmpp/xmpp_state_machine.h | 2 +- 15 files changed, 246 insertions(+), 94 deletions(-) create mode 100644 src/xmpp/xmpp_connection_manager.cc create mode 100644 src/xmpp/xmpp_connection_manager.h diff --git a/src/bgp/test/bgp_xmpp_basic_test.cc b/src/bgp/test/bgp_xmpp_basic_test.cc index a875fdaaee7..c227740f783 100644 --- a/src/bgp/test/bgp_xmpp_basic_test.cc +++ b/src/bgp/test/bgp_xmpp_basic_test.cc @@ -141,11 +141,11 @@ class BgpXmppBasicTest : public ::testing::Test { } size_t GetConnectionQueueSize(XmppServer *xs) { - return xs->GetQueueSize(); + return xs->GetConnectionQueueSize(); } void SetConnectionQueueDisable(XmppServer *xs, bool flag) { - xs->SetQueueDisable(flag); + xs->SetConnectionQueueDisable(flag); } void SetLifetimeManagerDestroyDisable(bool disabled) { diff --git a/src/xmpp/SConscript b/src/xmpp/SConscript index a83a5afba16..eea972e9d8d 100644 --- a/src/xmpp/SConscript +++ b/src/xmpp/SConscript @@ -33,6 +33,7 @@ libxmpp = env.Library('xmpp', 'xmpp_channel.cc', 'xmpp_config.cc', 'xmpp_connection.cc', + 'xmpp_connection_manager.cc', 'xmpp_factory.cc', 'xmpp_lifetime.cc', 'xmpp_session', diff --git a/src/xmpp/test/xmpp_regex_test.cc b/src/xmpp/test/xmpp_regex_test.cc index e12abe1e074..ae7f3b0ea77 100644 --- a/src/xmpp/test/xmpp_regex_test.cc +++ b/src/xmpp/test/xmpp_regex_test.cc @@ -18,8 +18,8 @@ using namespace std; class XmppRegexMock : public XmppSession { public: - XmppRegexMock(SslServer *server, SslSocket *sock) : - XmppSession(server, sock), p1("<(iq|message)"), bufx_("") { } + XmppRegexMock(XmppConnectionManager *manager, SslSocket *sock) + : XmppSession(manager, sock), p1("<(iq|message)"), bufx_("") { } ~XmppRegexMock() { } //boost::regex Regex() { return p1; } diff --git a/src/xmpp/xmpp_client.cc b/src/xmpp/xmpp_client.cc index 3e9a44b1fcd..a00d853295e 100644 --- a/src/xmpp/xmpp_client.cc +++ b/src/xmpp/xmpp_client.cc @@ -27,7 +27,8 @@ class XmppClient::DeleteActor : public LifetimeActor { DeleteActor(XmppClient *client) : LifetimeActor(client->lifetime_manager()), client_(client) { } virtual bool MayDelete() const { - return true; + CHECK_CONCURRENCY("bgp::Config"); + return (client_->GetSessionQueueSize() == 0); } virtual void Shutdown() { CHECK_CONCURRENCY("bgp::Config"); @@ -41,7 +42,7 @@ class XmppClient::DeleteActor : public LifetimeActor { }; XmppClient::XmppClient(EventManager *evm) - : SslServer(evm, ssl::context::tlsv1_client, false, false), + : XmppConnectionManager(evm, ssl::context::tlsv1_client, false, false), config_mgr_(new XmppConfigManager), lifetime_manager_(new LifetimeManager( TaskScheduler::GetInstance()->GetTaskId("bgp::Config"))), @@ -50,7 +51,8 @@ XmppClient::XmppClient(EventManager *evm) } XmppClient::XmppClient(EventManager *evm, const XmppChannelConfig *config) - : SslServer(evm, ssl::context::tlsv1_client, config->auth_enabled, true), + : XmppConnectionManager( + evm, ssl::context::tlsv1_client, config->auth_enabled, true), config_mgr_(new XmppConfigManager), lifetime_manager_(new LifetimeManager( TaskScheduler::GetInstance()->GetTaskId("bgp::Config"))), @@ -131,7 +133,7 @@ TcpSession *XmppClient::CreateSession() { } void XmppClient::Shutdown() { - TcpServer::Shutdown(); + XmppConnectionManager::Shutdown(); deleter_->Delete(); } diff --git a/src/xmpp/xmpp_client.h b/src/xmpp/xmpp_client.h index 7e4498039e6..d13c74473ae 100644 --- a/src/xmpp/xmpp_client.h +++ b/src/xmpp/xmpp_client.h @@ -11,15 +11,14 @@ #include "io/ssl_session.h" #include "xmpp/xmpp_config.h" #include "xmpp/xmpp_connection.h" +#include "xmpp/xmpp_connection_manager.h" class LifetimeActor; class LifetimeManager; class XmppSession; // Class to represent Xmpp Client -// We derive from the common TCP server base class -// which abstracts both server & client side methods. -class XmppClient : public SslServer { +class XmppClient : public XmppConnectionManager { public: typedef boost::asio::ip::tcp::endpoint Endpoint; @@ -50,7 +49,7 @@ class XmppClient : public SslServer { XmppConfigManager *xmpp_config_mgr() { return config_mgr_.get(); } LifetimeManager *lifetime_manager(); - LifetimeActor *deleter(); + virtual LifetimeActor *deleter(); protected: virtual SslSession *AllocSession(SslSocket *socket); diff --git a/src/xmpp/xmpp_connection.cc b/src/xmpp/xmpp_connection.cc index 00d5127d73d..d797ec16551 100644 --- a/src/xmpp/xmpp_connection.cc +++ b/src/xmpp/xmpp_connection.cc @@ -80,9 +80,18 @@ void XmppConnection::SetConfig(const XmppChannelConfig *config) { void XmppConnection::set_session(XmppSession *session) { tbb::spin_mutex::scoped_lock lock(spin_mutex_); + assert(session); session_ = session; } +void XmppConnection::clear_session() { + tbb::spin_mutex::scoped_lock lock(spin_mutex_); + if (!session_) + return; + session_->ClearConnection(); + session_ = NULL; +} + const XmppSession *XmppConnection::session() const { return session_; } @@ -91,8 +100,9 @@ XmppSession *XmppConnection::session() { return session_; } -void XmppConnection::WriteReady(const boost::system::error_code &ec) { - mux_->WriteReady(ec); +void XmppConnection::WriteReady() { + boost::system::error_code ec; + mux_->WriteReady(ec); } void XmppConnection::Shutdown() { @@ -315,6 +325,8 @@ void XmppConnection::SendClose(XmppSession *session) { void XmppConnection::ProcessSslHandShakeResponse(SslSessionPtr session, const boost::system::error_code& error) { + if (!state_machine()) + return; if (error) { inc_handshake_failure(); diff --git a/src/xmpp/xmpp_connection.h b/src/xmpp/xmpp_connection.h index bfefbcce70c..fe9ca64656f 100644 --- a/src/xmpp/xmpp_connection.h +++ b/src/xmpp/xmpp_connection.h @@ -95,6 +95,7 @@ class XmppConnection { void StopKeepAliveTimer(); void set_session(XmppSession *session); + void clear_session(); void SetFrom(const std::string &); void SetTo(const std::string &); @@ -145,7 +146,7 @@ class XmppConnection { virtual uint32_t flap_count() const = 0; virtual const std::string last_flap_at() const = 0; - virtual void WriteReady(const boost::system::error_code &ec); + virtual void WriteReady(); friend class XmppStateMachineTest; diff --git a/src/xmpp/xmpp_connection_manager.cc b/src/xmpp/xmpp_connection_manager.cc new file mode 100644 index 00000000000..88d67bd4474 --- /dev/null +++ b/src/xmpp/xmpp_connection_manager.cc @@ -0,0 +1,80 @@ +/* + * Copyright (c) 2013 Juniper Networks, Inc. All rights reserved. + */ + +#include "xmpp/xmpp_connection_manager.h" + +#include + +#include "base/lifetime.h" +#include "base/task_annotations.h" +#include "xmpp/xmpp_session.h" + +// +// Constructor for XmppConnectionManager. +// +XmppConnectionManager::XmppConnectionManager(EventManager *evm, + boost::asio::ssl::context::method m, + bool ssl_enabled, bool ssl_handshake_delayed) + : SslServer(evm, m, ssl_enabled, ssl_handshake_delayed), + session_queue_(TaskScheduler::GetInstance()->GetTaskId("bgp::Config"), + 0, boost::bind(&XmppConnectionManager::DequeueSession, this, _1)) { +} + +// +// Shutdown the XmppConnectionManager. +// +// Register an exit callback to the session WorkQueue so that we can retry +// deletion when the WorkQueue becomes empty. +// +void XmppConnectionManager::Shutdown() { + TcpServer::Shutdown(); + session_queue_.SetExitCallback( + boost::bind(&XmppConnectionManager::WorkQueueExitCallback, this, _1)); + session_queue_.Shutdown(); +} + +// +// Concurrency: called in the context of io thread. +// +// Add a XmppSession to the queue of write ready sessions. +// Take a reference to make sure that XmppSession doesn't get deleted before +// it's processed. +// +void XmppConnectionManager::EnqueueSession(XmppSession *session) { + if (deleter()->IsDeleted()) + return; + session_queue_.Enqueue(TcpSessionPtr(session)); +} + +// +// Concurrency: called in the context of bgp::Config task. +// +// Handler for XmppSessions that are dequeued from the session WorkQueue. +// +// The Xmpp[Client|Server] doesn't get destroyed if the WorkQueue is non-empty. +// +bool XmppConnectionManager::DequeueSession(TcpSessionPtr tcp_session) { + CHECK_CONCURRENCY("bgp::Config"); + XmppSession *session = static_cast(tcp_session.get()); + session->ProcessWriteReady(); + return true; +} + +// +// Exit callback for the session WorkQueue. +// +void XmppConnectionManager::WorkQueueExitCallback(bool done) { + CHECK_CONCURRENCY("bgp::Config"); + if (!deleter()->IsDeleted()) + return; + deleter()->RetryDelete(); +} + +// +// Return size of WorkQueue of write ready XmppSessions. +// +size_t XmppConnectionManager::GetSessionQueueSize() const { + CHECK_CONCURRENCY("bgp::Config"); + return session_queue_.Length(); +} diff --git a/src/xmpp/xmpp_connection_manager.h b/src/xmpp/xmpp_connection_manager.h new file mode 100644 index 00000000000..c435506e578 --- /dev/null +++ b/src/xmpp/xmpp_connection_manager.h @@ -0,0 +1,37 @@ +/* + * Copyright (c) 2013 Juniper Networks, Inc. All rights reserved. + */ + +#ifndef XMPP_XMPP_CONNECTION_MANAGER_H +#define XMPP_XMPP_CONNECTION_MANAGER_H + +#include "base/queue_task.h" +#include "io/ssl_server.h" + +class LifetimeActor; +class XmppSession; + +// +// Common class to represent XmppClient and XmppServer +// +class XmppConnectionManager : public SslServer { +public: + XmppConnectionManager(EventManager *evm, + boost::asio::ssl::context::method m, + bool ssl_enabled, bool ssl_handshake_delayed); + + void Shutdown(); + void EnqueueSession(XmppSession *session); + size_t GetSessionQueueSize() const; + virtual LifetimeActor *deleter() = 0; + +private: + bool DequeueSession(TcpSessionPtr tcp_session); + void WorkQueueExitCallback(bool done); + + WorkQueue session_queue_; + + DISALLOW_COPY_AND_ASSIGN(XmppConnectionManager); +}; + +#endif // XMPP_XMPP_CONNECTION_MANAGER_H diff --git a/src/xmpp/xmpp_server.cc b/src/xmpp/xmpp_server.cc index d040659afdb..72d307db11c 100644 --- a/src/xmpp/xmpp_server.cc +++ b/src/xmpp/xmpp_server.cc @@ -35,7 +35,7 @@ class XmppServer::DeleteActor : public LifetimeActor { } virtual bool MayDelete() const { CHECK_CONCURRENCY("bgp::Config"); - return true; + return server_->MayDelete(); } virtual void Shutdown() { CHECK_CONCURRENCY("bgp::Config"); @@ -53,16 +53,16 @@ class XmppServer::DeleteActor : public LifetimeActor { XmppServer::XmppServer(EventManager *evm, const string &server_addr, const XmppChannelConfig *config) - : SslServer(evm, ssl::context::tlsv1_server, config->auth_enabled, true), - + : XmppConnectionManager( + evm, ssl::context::tlsv1_server, config->auth_enabled, true), lifetime_manager_(XmppObjectFactory::Create( TaskScheduler::GetInstance()->GetTaskId("bgp::Config"))), deleter_(new DeleteActor(this)), server_addr_(server_addr), log_uve_(false), auth_enabled_(config->auth_enabled), - work_queue_(TaskScheduler::GetInstance()->GetTaskId("bgp::Config"), 0, - boost::bind(&XmppServer::DequeueConnection, this, _1)) { + connection_queue_(TaskScheduler::GetInstance()->GetTaskId("bgp::Config"), + 0, boost::bind(&XmppServer::DequeueConnection, this, _1)) { if (config->auth_enabled) { @@ -86,28 +86,28 @@ XmppServer::XmppServer(EventManager *evm, const string &server_addr, } XmppServer::XmppServer(EventManager *evm, const string &server_addr) - : SslServer(evm, ssl::context::tlsv1_server, false, false), + : XmppConnectionManager(evm, ssl::context::tlsv1_server, false, false), lifetime_manager_(XmppObjectFactory::Create( TaskScheduler::GetInstance()->GetTaskId("bgp::Config"))), deleter_(new DeleteActor(this)), server_addr_(server_addr), log_uve_(false), auth_enabled_(false), - work_queue_(TaskScheduler::GetInstance()->GetTaskId("bgp::Config"), 0, - boost::bind(&XmppServer::DequeueConnection, this, _1)) { + connection_queue_(TaskScheduler::GetInstance()->GetTaskId("bgp::Config"), + 0, boost::bind(&XmppServer::DequeueConnection, this, _1)) { } XmppServer::XmppServer(EventManager *evm) - : SslServer(evm, ssl::context::tlsv1_server, false, false), + : XmppConnectionManager(evm, ssl::context::tlsv1_server, false, false), max_connections_(0), lifetime_manager_(new LifetimeManager( TaskScheduler::GetInstance()->GetTaskId("bgp::Config"))), deleter_(new DeleteActor(this)), log_uve_(false), auth_enabled_(false), - work_queue_(TaskScheduler::GetInstance()->GetTaskId("bgp::Config"), 0, - boost::bind(&XmppServer::DequeueConnection, this, _1)) { + connection_queue_(TaskScheduler::GetInstance()->GetTaskId("bgp::Config"), + 0, boost::bind(&XmppServer::DequeueConnection, this, _1)) { } bool XmppServer::IsPeerCloseGraceful() { @@ -144,7 +144,17 @@ bool XmppServer::Initialize(short port, bool logUVE) { // Can be removed after Shutdown is renamed to ManagedDelete. // void XmppServer::SessionShutdown() { - TcpServer::Shutdown(); + XmppConnectionManager::Shutdown(); +} + +// +// Return true if it's possible to delete the XmppServer. +// +// No need to check the connection WorkQueue since XmppServerConnections on it +// are dependents of the XmppServer. +// +bool XmppServer::MayDelete() const { + return (GetSessionQueueSize() == 0); } // @@ -164,7 +174,7 @@ void XmppServer::Shutdown() { // void XmppServer::Terminate() { ClearSessions(); - work_queue_.Shutdown(); + connection_queue_.Shutdown(); } LifetimeActor *XmppServer::deleter() { @@ -296,7 +306,7 @@ bool XmppServer::AcceptSession(TcpSession *tcp_session) { session->set_read_on_connect(false); connection->set_session(session); connection->set_on_work_queue(); - work_queue_.Enqueue(connection); + connection_queue_.Enqueue(connection); return true; } @@ -357,8 +367,7 @@ XmppServerConnection *XmppServer::CreateConnection(XmppSession *session) { // // Since the XmppServerConnections on the WorkQueue are dependents of the // XmppServer, we are guaranteed that the XmppServer won't get destroyed -// before the WorkQueue is drained. Hence we don't need to check for the -// WorkQueue being empty in DeleteActor::MayDelete. +// before the connection WorkQueue is drained. // bool XmppServer::DequeueConnection(XmppServerConnection *connection) { CHECK_CONCURRENCY("bgp::Config"); @@ -372,7 +381,7 @@ bool XmppServer::DequeueConnection(XmppServerConnection *connection) { } XmppSession *session = connection->session(); - connection->set_session(NULL); + connection->clear_session(); Endpoint remote_endpoint = session->remote_endpoint(); XmppServerConnection *old_connection = FindConnection(remote_endpoint); @@ -395,13 +404,12 @@ bool XmppServer::DequeueConnection(XmppServerConnection *connection) { return true; } - -size_t XmppServer::GetQueueSize() const { - return work_queue_.Length(); +size_t XmppServer::GetConnectionQueueSize() const { + return connection_queue_.Length(); } -void XmppServer::SetQueueDisable(bool disabled) { - work_queue_.set_disable(disabled); +void XmppServer::SetConnectionQueueDisable(bool disabled) { + connection_queue_.set_disable(disabled); } // diff --git a/src/xmpp/xmpp_server.h b/src/xmpp/xmpp_server.h index 4c71f7bb271..aeb1c8dd3cc 100644 --- a/src/xmpp/xmpp_server.h +++ b/src/xmpp/xmpp_server.h @@ -14,6 +14,7 @@ #include "net/address.h" #include "xmpp/xmpp_session.h" #include "xmpp/xmpp_config.h" +#include "xmpp/xmpp_connection_manager.h" #include "xmpp/xmpp_channel_mux.h" class LifetimeActor; @@ -25,7 +26,7 @@ class XmppConnectionEndpoint; class XmppServerConnection; // Class to represent Xmpp Server -class XmppServer : public SslServer { +class XmppServer : public XmppConnectionManager { public: typedef boost::asio::ip::tcp::endpoint Endpoint; @@ -43,12 +44,13 @@ class XmppServer : public SslServer { size_t ConnectionEventCount() const; LifetimeManager *lifetime_manager(); - LifetimeActor *deleter(); + virtual LifetimeActor *deleter(); virtual TcpSession *CreateSession(); virtual bool Initialize(short port); virtual bool Initialize(short port, bool logUVE); void SessionShutdown(); + bool MayDelete() const; void Shutdown(); void Terminate(); @@ -93,8 +95,9 @@ class XmppServer : public SslServer { typedef std::map ConnectionEventCbMap; bool DequeueConnection(XmppServerConnection *connection); - size_t GetQueueSize() const; - void SetQueueDisable(bool disabled); + size_t GetConnectionQueueSize() const; + void SetConnectionQueueDisable(bool disabled); + void WorkQueueExitCallback(bool done); ConnectionMap connection_map_; ConnectionSet deleted_connection_set_; @@ -111,7 +114,7 @@ class XmppServer : public SslServer { std::string server_addr_; bool log_uve_; bool auth_enabled_; - WorkQueue work_queue_; + WorkQueue connection_queue_; DISALLOW_COPY_AND_ASSIGN(XmppServer); }; diff --git a/src/xmpp/xmpp_session.cc b/src/xmpp/xmpp_session.cc index 9df0b0e222e..8e6db47ab98 100644 --- a/src/xmpp/xmpp_session.cc +++ b/src/xmpp/xmpp_session.cc @@ -7,6 +7,7 @@ #include "xmpp/xmpp_connection.h" #include "xmpp/xmpp_log.h" #include "xmpp/xmpp_proto.h" +#include "xmpp/xmpp_server.h" #include "xmpp/xmpp_state_machine.h" #include "sandesh/sandesh_types.h" @@ -27,32 +28,62 @@ const boost::regex XmppSession::starttls_patt_(rXMPP_STREAM_STARTTLS); const boost::regex XmppSession::proceed_patt_(rXMPP_STREAM_PROCEED); const boost::regex XmppSession::end_patt_(rXMPP_STREAM_STANZA_END); -const std::string XmppStream::close_string = sXML_STREAM_C; - -XmppSession::XmppSession(SslServer *server, SslSocket *socket, bool async_ready) - : SslSession(server, socket, async_ready), - connection_(NULL), - buf_(""), offset_(), tag_known_(0), - stats_(XmppStanza::RESERVED_STANZA, XmppSession::StatsPair(0,0)) { - +XmppSession::XmppSession(XmppConnectionManager *manager, SslSocket *socket, + bool async_ready) + : SslSession(manager, socket, async_ready), + manager_(manager), + connection_(NULL), + tag_known_(0), + index_(-1), + stats_(XmppStanza::RESERVED_STANZA, XmppSession::StatsPair(0, 0)) { buf_.reserve(kMaxMessageSize); offset_ = buf_.begin(); stream_open_matched_ = false; } - XmppSession::~XmppSession() { set_observer(NULL); connection_ = NULL; } -int XmppSession::GetSessionInstance() const { - return (connection_->GetIndex()); +void XmppSession::SetConnection(XmppConnection *connection) { + connection_ = connection; + index_ = connection_->GetIndex(); +} + +void XmppSession::ClearConnection() { + connection_ = NULL; + index_ = -1; +} + +// +// Concurrency: called in the context of bgp::Config task. +// +// Process write ready callback. +// +void XmppSession::ProcessWriteReady() { + if (!connection_) + return; + connection_->WriteReady(); } +// +// Concurrency: called in the context of io thread. +// +// Handle write ready callback. +// +// Enqueue session to the XmppConnectionManager. The session is added to a +// WorkQueue gets processed in the context of bgp::Config task. Doing this +// ensures that we don't access the XmppConnection while the XmppConnection +// is trying to clear our back pointer to it. +// +// We can ignore any errors since the StateMachine will get informed of the +// TcpSession close independently and react to it. +// void XmppSession::WriteReady(const boost::system::error_code &error) { - if (connection_) - connection_->WriteReady(error); + if (error) + return; + manager_->EnqueueSession(this); } XmppSession::StatsPair XmppSession::Stats(unsigned int type) const { diff --git a/src/xmpp/xmpp_session.h b/src/xmpp/xmpp_session.h index 147292136b5..512da04c7df 100644 --- a/src/xmpp/xmpp_session.h +++ b/src/xmpp/xmpp_session.h @@ -10,24 +10,23 @@ #include "io/ssl_server.h" #include "io/ssl_session.h" -class XmppStream; class XmppServer; class XmppConnection; +class XmppConnectionManager; class XmppRegexMock; class XmppSession : public SslSession { public: - XmppSession(SslServer *server, SslSocket *sock, bool async_ready = true); + XmppSession(XmppConnectionManager *manager, SslSocket *sock, + bool async_ready = true); virtual ~XmppSession(); - void SetConnection(XmppConnection *connection) { - this->connection_ = connection; - } + void SetConnection(XmppConnection *connection); + void ClearConnection(); XmppConnection *Connection() { return connection_; } - - XmppStream *SessionStream() { return stream_; } - void SessionStreamSet(XmppStream *strm) { this->stream_ = strm;} + virtual void WriteReady(const boost::system::error_code &error); + void ProcessWriteReady(); typedef std::pair StatsPair; // (packets, bytes) StatsPair Stats(unsigned int message_type) const; @@ -35,13 +34,13 @@ class XmppSession : public SslSession { static const int kMaxMessageSize = 4096; friend class XmppRegexMock; - - virtual int GetSessionInstance() const; - + + virtual int GetSessionInstance() const { return index_; } + protected: std::string jid; virtual void OnRead(Buffer buffer); - + private: typedef std::deque BufferQueue; @@ -52,13 +51,14 @@ class XmppSession : public SslSession { void ReplaceBuf(const std::string &); bool LeftOver() const; + XmppConnectionManager *manager_; XmppConnection *connection_; BufferQueue queue_; - XmppStream *stream_; std::string begin_tag_; std::string buf_; std::string::const_iterator offset_; int tag_known_; + int index_; boost::match_results res_; std::vector stats_; // packet count bool stream_open_matched_; @@ -75,27 +75,4 @@ class XmppSession : public SslSession { DISALLOW_COPY_AND_ASSIGN(XmppSession); }; -class XmppStream { -public: - - enum XmppStreamNS { - JABBER_CLIENT = 1, - JABBER_SERVER = 2 - }; - - XmppStream(std::string resource, XmppStreamNS ns) - : resource(resource), ns(ns) { - } - std::string xmppStreamFQDNJid(); - static const std::string close_string; - std::string resource; - XmppStreamNS ns; - -private: - std::string resource_ ; - XmppStreamNS ns_; - DISALLOW_COPY_AND_ASSIGN(XmppStream); -}; - - #endif // __XMPP_SESSION_H__ diff --git a/src/xmpp/xmpp_state_machine.cc b/src/xmpp/xmpp_state_machine.cc index 237ef756579..9be89a8269d 100644 --- a/src/xmpp/xmpp_state_machine.cc +++ b/src/xmpp/xmpp_state_machine.cc @@ -1123,6 +1123,7 @@ XmppStateMachine::XmppStateMachine(XmppConnection *connection, bool active, is_active_(active), auth_enabled_(auth_enabled), state_(xmsm::IDLE), + last_state_(xmsm::IDLE), openconfirm_state_(xmsm::OPENCONFIRM_INIT) { handshake_cb_ = boost::bind( &XmppConnection::ProcessSslHandShakeResponse, connection, _1, _2); @@ -1170,7 +1171,7 @@ void XmppStateMachine::clear_session() { session_->set_observer(NULL); session_->SetConnection(NULL); session_->Close(); - connection_->set_session(NULL); + connection_->clear_session(); session_ = NULL; } } @@ -1186,7 +1187,7 @@ void XmppStateMachine::DeleteSession(XmppSession *session) { void XmppStateMachine::set_session(TcpSession *session) { if (session_ != NULL) { - connection_->set_session(NULL); + connection_->clear_session(); DeleteSession(session_); } session_ = static_cast(session); diff --git a/src/xmpp/xmpp_state_machine.h b/src/xmpp/xmpp_state_machine.h index 3cb4c32e998..9b3fbbe92c5 100644 --- a/src/xmpp/xmpp_state_machine.h +++ b/src/xmpp/xmpp_state_machine.h @@ -186,8 +186,8 @@ class XmppStateMachine : bool is_active_; bool auth_enabled_; xmsm::XmState state_; - xmsm::XmOpenConfirmState openconfirm_state_; xmsm::XmState last_state_; + xmsm::XmOpenConfirmState openconfirm_state_; uint64_t state_since_; std::string last_event_; uint64_t last_event_at_;