From e67c99cd451d95fa22e979bbbcc0ee1a204f7e23 Mon Sep 17 00:00:00 2001 From: Ananth Suryanarayana Date: Thu, 3 Mar 2016 18:42:48 -0800 Subject: [PATCH] GR: Maintain 1:1 mapping between associated xmpp connection and endpoints o Bring back asserts in ReleaseEndpoint() based on the above design premise o Provide API in XmppChannel to determine if Close is in progress o In XmppChannel, maintain state of clients who close gracefully using a simple count. Provide API to check and update this state. o Trigger Sweep process in clients (BgpXmppChannel) after routes sweep is complete in PeerCloseManager o Simplify BgpTable route deletion and make sure that deleted rt is never accessed o Get rid of FindConnectionEndpoint(XmppConnection *) API and use FindConnectionEndpoint(string) instead, by using connection->ToString() o Associate connection with endpoint inside the LocateConnectionEndpoint API TODO ==== o Use IP-Address as index value for XmppStaetMachine task id Change-Id: Iebb5aeafd1d3b6bab1738da65d0c8806eda7a47a Partial-Bug: #1537933 --- src/bgp/bgp_peer_close.cc | 46 ++++++++++++++++++--------- src/bgp/bgp_peer_close.h | 3 ++ src/bgp/bgp_table.cc | 41 +++++++++++++----------- src/bgp/bgp_table.h | 2 ++ src/bgp/bgp_xmpp_channel.cc | 8 ++--- src/bgp/test/bgp_xmpp_channel_test.cc | 2 ++ src/bgp/test/bgp_xmpp_parse_test.cc | 2 ++ src/vnsw/agent/test/test_cmn_util.h | 2 ++ src/xmpp/xmpp_channel.h | 2 ++ src/xmpp/xmpp_channel_mux.cc | 43 ++++++++++++++++++++++++- src/xmpp/xmpp_channel_mux.h | 4 +++ src/xmpp/xmpp_connection.cc | 5 ++- src/xmpp/xmpp_connection.h | 4 ++- src/xmpp/xmpp_server.cc | 41 +++++++++++------------- src/xmpp/xmpp_server.h | 1 - src/xmpp/xmpp_state_machine.cc | 45 ++++++++++++-------------- 16 files changed, 159 insertions(+), 92 deletions(-) diff --git a/src/bgp/bgp_peer_close.cc b/src/bgp/bgp_peer_close.cc index 953c8174fc9..e5ae94d4fe0 100644 --- a/src/bgp/bgp_peer_close.cc +++ b/src/bgp/bgp_peer_close.cc @@ -25,15 +25,20 @@ // Create an instance of PeerCloseManager with back reference to parent IPeer PeerCloseManager::PeerCloseManager(IPeer *peer) : - peer_(peer), stale_timer_(NULL), state_(NONE), close_again_(false) { + peer_(peer), stale_timer_(NULL), sweep_timer_(NULL), state_(NONE), + close_again_(false) { stats_.init++; - if (peer->server()) + if (peer->server()) { stale_timer_ = TimerManager::CreateTimer(*peer->server()->ioservice(), "Graceful Restart StaleTimer"); + sweep_timer_ = TimerManager::CreateTimer(*peer->server()->ioservice(), + "Graceful Restart SweepTimer"); + } } PeerCloseManager::~PeerCloseManager() { TimerManager::DeleteTimer(stale_timer_); + TimerManager::DeleteTimer(sweep_timer_); } const std::string PeerCloseManager::GetStateName(State state) const { @@ -149,7 +154,6 @@ void PeerCloseManager::ProcessClosure() { if (peer_->IsReady()) { MOVE_TO_STATE(SWEEP); stats_.sweep++; - peer_->peer_close()->GracefulRestartSweep(); } else { MOVE_TO_STATE(DELETE); stats_.deletes++; @@ -178,6 +182,7 @@ bool PeerCloseManager::IsCloseInProgress() { void PeerCloseManager::CloseComplete() { MOVE_TO_STATE(NONE); stale_timer_->Cancel(); + sweep_timer_->Cancel(); stats_.init++; // Nested closures trigger fresh GR @@ -187,6 +192,22 @@ void PeerCloseManager::CloseComplete() { } } +bool PeerCloseManager::ProcessSweepStateActions() { + assert(state_ == SWEEP); + + // Notify clients to trigger sweep as appropriate. + peer_->peer_close()->GracefulRestartSweep(); + CloseComplete(); + return false; +} + +void PeerCloseManager::TriggerSweepStateActions() { + PEER_CLOSE_MANAGER_LOG("Sweep Timer started to fire right away"); + sweep_timer_->Cancel(); + sweep_timer_->Start(0, + boost::bind(&PeerCloseManager::ProcessSweepStateActions, this)); +} + // Concurrency: Runs in the context of the BGP peer rib membership task. // // Close process for this peer in terms of walking RibIns and RibOuts are @@ -216,8 +237,7 @@ void PeerCloseManager::UnregisterPeerComplete(IPeer *ipeer, BgpTable *table) { return; } - // Handle SWEEP state and restart GR for nested closures. - CloseComplete(); + TriggerSweepStateActions(); } // Get the type of RibIn close action at start (Not during graceful restart @@ -278,7 +298,7 @@ void PeerCloseManager::ProcessRibIn(DBTablePartBase *root, BgpRoute *rt, if (action == MembershipRequest::INVALID) return; - bool delete_rt = false; + bool notify_rt = false; // Process all paths sourced from this peer_. Multiple paths could exist // in ecmp cases. @@ -334,17 +354,13 @@ void PeerCloseManager::ProcessRibIn(DBTablePartBase *root, BgpRoute *rt, } // Feed the route modify/delete request to the table input process. - delete_rt = table->InputCommon(root, rt, path, peer_, NULL, oper, - attrs, path->GetPathId(), - path->GetFlags() | stale, - path->GetLabel()); + notify_rt |= table->InputCommon(root, rt, path, peer_, NULL, oper, + attrs, path->GetPathId(), + path->GetFlags() | stale, + path->GetLabel()); } - // rt can be now deleted safely. - if (delete_rt) - root->Delete(rt); - - return; + table->InputCommonPostProcess(root, rt, notify_rt); } void PeerCloseManager::FillCloseInfo(BgpNeighborResp *resp) { diff --git a/src/bgp/bgp_peer_close.h b/src/bgp/bgp_peer_close.h index 5f3138a953b..d4aa44fdd62 100644 --- a/src/bgp/bgp_peer_close.h +++ b/src/bgp/bgp_peer_close.h @@ -77,10 +77,13 @@ class PeerCloseManager { void ProcessClosure(); void CloseComplete(); + bool ProcessSweepStateActions(); + void TriggerSweepStateActions(); const std::string GetStateName(State state) const; IPeer *peer_; Timer *stale_timer_; + Timer *sweep_timer_; State state_; bool close_again_; Stats stats_; diff --git a/src/bgp/bgp_table.cc b/src/bgp/bgp_table.cc index 9821dfbfa01..ea5f2dcbd9d 100644 --- a/src/bgp/bgp_table.cc +++ b/src/bgp/bgp_table.cc @@ -271,7 +271,7 @@ bool BgpTable::InputCommon(DBTablePartBase *root, BgpRoute *rt, BgpPath *path, const IPeer *peer, DBRequest *req, DBRequest::DBOperation oper, BgpAttrPtr attrs, uint32_t path_id, uint32_t flags, uint32_t label) { - bool delete_rt = false; + bool notify_rt = false; switch (oper) { case DBRequest::DB_ENTRY_ADD_CHANGE: { @@ -307,7 +307,7 @@ bool BgpTable::InputCommon(DBTablePartBase *root, BgpRoute *rt, BgpPath *path, table); } rt->InsertPath(new_path); - root->Notify(rt); + notify_rt = true; break; } @@ -320,13 +320,7 @@ bool BgpTable::InputCommon(DBTablePartBase *root, BgpRoute *rt, BgpPath *path, if (path->NeedsResolution()) path_resolver_->StopPathResolution(root->index(), path); rt->RemovePath(BgpPath::BGP_XMPP, peer, path_id); - - // Delete the route only if all paths are gone. - if (rt->front() == NULL) { - delete_rt = true; - } else { - root->Notify(rt); - } + notify_rt = true; } break; } @@ -336,7 +330,7 @@ bool BgpTable::InputCommon(DBTablePartBase *root, BgpRoute *rt, BgpPath *path, break; } } - return delete_rt; + return notify_rt; } void BgpTable::Input(DBTablePartition *root, DBClient *client, @@ -393,7 +387,7 @@ void BgpTable::Input(DBTablePartition *root, DBClient *client, int count = 0; ExtCommunityDB *extcomm_db = rtinstance_->server()->extcomm_db(); BgpAttrPtr attr = data ? data->attrs() : NULL; - bool delete_rt = false; + bool notify_rt = false; // Process each of the paths sourced and create/update paths accordingly. if (data) { @@ -426,9 +420,9 @@ void BgpTable::Input(DBTablePartition *root, DBClient *client, attr = data->attrs()->attr_db()->Locate(clone); } - delete_rt = InputCommon(root, rt, path, peer, req, req->oper, - attr, path_id, nexthop.flags_, - nexthop.label_); + notify_rt |= InputCommon(root, rt, path, peer, req, req->oper, + attr, path_id, nexthop.flags_, + nexthop.label_); } } @@ -436,14 +430,23 @@ void BgpTable::Input(DBTablePartition *root, DBClient *client, for (map::iterator it = deleted_paths.begin(); it != deleted_paths.end(); it++) { BgpPath *path = it->first; - delete_rt = InputCommon(root, rt, path, peer, req, - DBRequest::DB_ENTRY_DELETE, NULL, - path->GetPathId(), 0, 0); + notify_rt |= InputCommon(root, rt, path, peer, req, + DBRequest::DB_ENTRY_DELETE, NULL, + path->GetPathId(), 0, 0); } - // rt can be now deleted safely. - if (delete_rt) + InputCommonPostProcess(root, rt, notify_rt); +} + +void BgpTable::InputCommonPostProcess(DBTablePartBase *root, + BgpRoute *rt, bool notify_rt) { + if (!notify_rt) + return; + + if (rt->front() == NULL) root->Delete(rt); + else + root->Notify(rt); } bool BgpTable::MayDelete() const { diff --git a/src/bgp/bgp_table.h b/src/bgp/bgp_table.h index d2d651cc443..42f7d51ded1 100644 --- a/src/bgp/bgp_table.h +++ b/src/bgp/bgp_table.h @@ -118,6 +118,8 @@ class BgpTable : public RouteTable { const IPeer *peer, DBRequest *req, DBRequest::DBOperation oper, BgpAttrPtr attrs, uint32_t path_id, uint32_t flags, uint32_t label); + void InputCommonPostProcess(DBTablePartBase *root, BgpRoute *rt, + bool notify_rt); LifetimeActor *deleter(); const LifetimeActor *deleter() const; diff --git a/src/bgp/bgp_xmpp_channel.cc b/src/bgp/bgp_xmpp_channel.cc index 50cfbeef3e5..67840219276 100644 --- a/src/bgp/bgp_xmpp_channel.cc +++ b/src/bgp/bgp_xmpp_channel.cc @@ -196,12 +196,9 @@ class BgpXmppChannel::PeerClose : public IPeerClose { return; parent_->set_peer_closed(false); - XmppConnection *connection = - const_cast(parent_->channel_->connection()); - // Restart state machine. - if (connection && connection->state_machine()) - connection->state_machine()->Initialize(); + // Indicate to Channel that GR Closure is now complete + parent_->channel_->CloseComplete(); } virtual void Delete() { @@ -217,6 +214,7 @@ class BgpXmppChannel::PeerClose : public IPeerClose { void Close() { if (parent_) { assert(parent_->peer_deleted()); + assert(parent_->channel_->IsCloseInProgress()); manager_->Close(); } } diff --git a/src/bgp/test/bgp_xmpp_channel_test.cc b/src/bgp/test/bgp_xmpp_channel_test.cc index d33e7a999a9..3d0f098cf4a 100644 --- a/src/bgp/test/bgp_xmpp_channel_test.cc +++ b/src/bgp/test/bgp_xmpp_channel_test.cc @@ -29,6 +29,8 @@ class XmppChannelMock : public XmppChannel { XmppChannelMock() { } virtual ~XmppChannelMock() { } void Close() { } + void CloseComplete() { } + bool IsCloseInProgress() const { return false; } bool Send(const uint8_t *, size_t, xmps::PeerId, SendReadyCb) { return true; } diff --git a/src/bgp/test/bgp_xmpp_parse_test.cc b/src/bgp/test/bgp_xmpp_parse_test.cc index 3ee9a086aa6..cb0863a7cbe 100644 --- a/src/bgp/test/bgp_xmpp_parse_test.cc +++ b/src/bgp/test/bgp_xmpp_parse_test.cc @@ -22,6 +22,8 @@ class XmppChannelMock : public XmppChannel { XmppChannelMock() { } virtual ~XmppChannelMock() { } void Close() { } + void CloseComplete() { } + bool IsCloseInProgress() const { return false; } bool Send(const uint8_t *, size_t, xmps::PeerId, SendReadyCb) { return true; } diff --git a/src/vnsw/agent/test/test_cmn_util.h b/src/vnsw/agent/test/test_cmn_util.h index 013aa62073a..01e7563cb8b 100644 --- a/src/vnsw/agent/test/test_cmn_util.h +++ b/src/vnsw/agent/test/test_cmn_util.h @@ -396,6 +396,8 @@ class XmppChannelMock : public XmppChannel { return true; } void Close() { } + void CloseComplete() { } + bool IsCloseInProgress() const { return false; } MOCK_METHOD2(RegisterReceive, void(xmps::PeerId, ReceiveCb)); MOCK_METHOD1(UnRegisterReceive, void(xmps::PeerId)); MOCK_METHOD1(UnRegisterWriteReady, void(xmps::PeerId)); diff --git a/src/xmpp/xmpp_channel.h b/src/xmpp/xmpp_channel.h index 60eed73da9a..d0b0c56b746 100644 --- a/src/xmpp/xmpp_channel.h +++ b/src/xmpp/xmpp_channel.h @@ -50,6 +50,8 @@ class XmppChannel { virtual void RegisterRxMessageTraceCallback(RxMessageTraceCb cb) = 0; virtual void UnRegisterWriteReady(xmps::PeerId id) = 0; virtual void Close() = 0; + virtual void CloseComplete() = 0; + virtual bool IsCloseInProgress() const = 0; virtual std::string ToString() const = 0; virtual std::string StateName() const = 0; virtual std::string LastStateName() const = 0; diff --git a/src/xmpp/xmpp_channel_mux.cc b/src/xmpp/xmpp_channel_mux.cc index 51b7886a10c..cb8eb3add63 100644 --- a/src/xmpp/xmpp_channel_mux.cc +++ b/src/xmpp/xmpp_channel_mux.cc @@ -13,16 +13,55 @@ using namespace std; using namespace xmsm; XmppChannelMux::XmppChannelMux(XmppConnection *connection) - : connection_(connection), rx_message_trace_cb_(NULL) { + : connection_(connection), rx_message_trace_cb_(NULL), closing_count_(0) { } XmppChannelMux::~XmppChannelMux() { } void XmppChannelMux::Close() { + if (closing_count_) + return; + InitializeClosingCount(); connection_->Clear(); } +// Track clients who close gracefully. At the moment, only BGP cares about this. +void XmppChannelMux::InitializeClosingCount() { + + BOOST_FOREACH(const ReceiveCbMap::value_type &value, rxmap_) { + switch (value.first) { + + // Currently, Only BgpXmppChannel client cares about GR. + case xmps::BGP: + closing_count_++; + break; + + case xmps::CONFIG: + case xmps::DNS: + case xmps::OTHER: + break; + } + } +} + +// Check if the channel is being closed (Graceful Restart) +bool XmppChannelMux::IsCloseInProgress() const { + return closing_count_ != 0; +} + +// API for the clients to indicate GR Closure is complete +void XmppChannelMux::CloseComplete() { + assert(closing_count_); + closing_count_--; + if (closing_count_) + return; + + // Restart state machine. + if (connection() && connection()->state_machine()) + connection()->state_machine()->Initialize(); +} + xmps::PeerState XmppChannelMux::GetPeerState() const { xmsm::XmState st = connection_->GetStateMcState(); return (st == xmsm::ESTABLISHED) ? xmps::READY : @@ -158,6 +197,8 @@ void XmppChannelMux::HandleStateEvent(xmsm::XmState state) { } else { // Event to create the peer on server XmppServer *server = static_cast(connection_->server()); + if (st == xmps::NOT_READY) + InitializeClosingCount(); server->NotifyConnectionEvent(this, st); } } diff --git a/src/xmpp/xmpp_channel_mux.h b/src/xmpp/xmpp_channel_mux.h index d55e23b94c1..8419ec927e0 100644 --- a/src/xmpp/xmpp_channel_mux.h +++ b/src/xmpp/xmpp_channel_mux.h @@ -19,6 +19,8 @@ class XmppChannelMux : public XmppChannel { virtual ~XmppChannelMux(); virtual void Close(); + virtual bool IsCloseInProgress() const; + virtual void CloseComplete(); virtual bool Send(const uint8_t *, size_t, xmps::PeerId, SendReadyCb); virtual void RegisterReceive(xmps::PeerId, ReceiveCb); virtual void UnRegisterReceive(xmps::PeerId); @@ -64,6 +66,7 @@ class XmppChannelMux : public XmppChannel { private: void RegisterWriteReady(xmps::PeerId, SendReadyCb); + void InitializeClosingCount(); typedef std::map WriteReadyCbMap; typedef std::map ReceiveCbMap; @@ -74,6 +77,7 @@ class XmppChannelMux : public XmppChannel { XmppConnection *connection_; tbb::mutex mutex_; RxMessageTraceCb rx_message_trace_cb_; + int closing_count_; }; #endif // __XMPP_CHANNEL_MUX_H__ diff --git a/src/xmpp/xmpp_connection.cc b/src/xmpp/xmpp_connection.cc index e46a2761375..04c071a8087 100644 --- a/src/xmpp/xmpp_connection.cc +++ b/src/xmpp/xmpp_connection.cc @@ -626,8 +626,7 @@ class XmppServerConnection::DeleteActor : public LifetimeActor { } if (parent_->session() || server_->IsPeerCloseGraceful()) { - server_->NotifyConnectionEvent(parent_->ChannelMux(), - xmps::NOT_READY); + parent_->ChannelMux()->HandleStateEvent(xmsm::IDLE); } if (parent_->logUVE()) { @@ -727,7 +726,7 @@ uint32_t XmppServerConnection::flap_count() const { void XmppServerConnection::increment_flap_count() { XmppConnectionEndpoint *conn_endpoint = conn_endpoint_; if (!conn_endpoint) - conn_endpoint = server()->FindConnectionEndpoint(this); + conn_endpoint = server()->FindConnectionEndpoint(ToString()); if (!conn_endpoint) return; conn_endpoint->increment_flap_count(); diff --git a/src/xmpp/xmpp_connection.h b/src/xmpp/xmpp_connection.h index 22d1730bf15..b45c14e3f6e 100644 --- a/src/xmpp/xmpp_connection.h +++ b/src/xmpp/xmpp_connection.h @@ -123,6 +123,8 @@ class XmppConnection { // TODO, this is needed to ensure no two reader tasks on the // same session can run in parallel, use endpoint_.port() // return endpoint_.port(); + // + // GR TODO: Use IP Address return 0; } @@ -295,7 +297,7 @@ class XmppServerConnection : public XmppConnection { XmppConnectionEndpoint *conn_endpoint() { return conn_endpoint_; } void set_conn_endpoint(XmppConnectionEndpoint *conn_endpoint) { - conn_endpoint = conn_endpoint; + conn_endpoint_ = conn_endpoint; } void FillShowInfo(ShowXmppConnection *show_connection) const; diff --git a/src/xmpp/xmpp_server.cc b/src/xmpp/xmpp_server.cc index ef2b2eb5f87..f1c65770692 100644 --- a/src/xmpp/xmpp_server.cc +++ b/src/xmpp/xmpp_server.cc @@ -496,17 +496,6 @@ XmppConnectionEndpoint *XmppServer::FindConnectionEndpoint( return (loc != connection_endpoint_map_.end() ? loc->second : NULL); } -XmppConnectionEndpoint *XmppServer::FindConnectionEndpoint( - XmppServerConnection *connection) { - if (!connection) - return NULL; - tbb::mutex::scoped_lock lock(endpoint_map_mutex_); - - ConnectionEndpointMap::const_iterator loc = - connection_endpoint_map_.find(connection->ToString()); - return (loc != connection_endpoint_map_.end() ? loc->second : NULL); -} - XmppConnectionEndpoint *XmppServer::LocateConnectionEndpoint( XmppServerConnection *connection, bool &created) { created = false; @@ -517,12 +506,20 @@ XmppConnectionEndpoint *XmppServer::LocateConnectionEndpoint( ConnectionEndpointMap::const_iterator loc = connection_endpoint_map_.find(connection->ToString()); - if (loc != connection_endpoint_map_.end()) - return loc->second; + XmppConnectionEndpoint *conn_endpoint; + + if (loc != connection_endpoint_map_.end()) { + conn_endpoint = loc->second; + if (!conn_endpoint->connection()) { + created = true; + conn_endpoint->set_connection(connection); + connection->set_conn_endpoint(conn_endpoint); + } + return conn_endpoint; + } created = true; - XmppConnectionEndpoint *conn_endpoint = - new XmppConnectionEndpoint(connection->ToString()); + conn_endpoint = new XmppConnectionEndpoint(connection->ToString()); bool result; tie(loc, result) = connection_endpoint_map_.insert( make_pair(connection->ToString(), conn_endpoint)); @@ -538,15 +535,13 @@ XmppConnectionEndpoint *XmppServer::LocateConnectionEndpoint( // simply have called XmppConnectionEndpoint::reset_connection directly. // void XmppServer::ReleaseConnectionEndpoint(XmppServerConnection *connection) { - XmppConnectionEndpoint *conn_endpoint = connection->conn_endpoint(); - if (!conn_endpoint) - conn_endpoint = FindConnectionEndpoint(connection); - if (!conn_endpoint) - return; - tbb::mutex::scoped_lock lock(endpoint_map_mutex_); - if (conn_endpoint->connection() == connection) - conn_endpoint->reset_connection(); + + if (!connection->conn_endpoint()) + return; + assert(connection->conn_endpoint()->connection() == connection); + connection->conn_endpoint()->reset_connection(); + connection->set_conn_endpoint(NULL); } void XmppServer::FillShowConnections( diff --git a/src/xmpp/xmpp_server.h b/src/xmpp/xmpp_server.h index 78562806d67..942c939d3a5 100644 --- a/src/xmpp/xmpp_server.h +++ b/src/xmpp/xmpp_server.h @@ -71,7 +71,6 @@ class XmppServer : public XmppConnectionManager { XmppConnectionEndpoint *FindConnectionEndpoint( const std::string &endpoint_name); - XmppConnectionEndpoint *FindConnectionEndpoint(XmppServerConnection *conn); XmppConnectionEndpoint *LocateConnectionEndpoint( XmppServerConnection *connection, bool &created); void ReleaseConnectionEndpoint(XmppServerConnection *connection); diff --git a/src/xmpp/xmpp_state_machine.cc b/src/xmpp/xmpp_state_machine.cc index 918e0a9d5d3..52867c9123f 100644 --- a/src/xmpp/xmpp_state_machine.cc +++ b/src/xmpp/xmpp_state_machine.cc @@ -1357,35 +1357,35 @@ bool XmppStateMachine::ProcessStreamHeaderMessage(XmppSession *session, session->Connection()->SetTo(msg->from); XmppServer *xmpp_server = dynamic_cast(server_); - XmppConnectionEndpoint *endp = NULL; + XmppConnectionEndpoint *endpoint = NULL; // Look for an endpoint which may already exist if (xmpp_server) - endp = xmpp_server->FindConnectionEndpoint( - dynamic_cast(connection_)); + endpoint = xmpp_server->FindConnectionEndpoint(connection_->ToString()); // If older endpoint is present and is still associated with XmppConnection, // check if older connection is under graceful-restart. - if (endp && endp->connection()) { - XmppStateMachine *state_machine = endp->connection()->state_machine(); - - // Different state_machines imply that connections are different - if (state_machine && state_machine != this) { - xmsm::XmState state = state_machine->get_state(); + if (endpoint && endpoint->connection()) { + if (connection_ != endpoint->connection()) { + XmppChannel *channel = endpoint->connection()->ChannelMux(); // If GR is not supported, then close all new connections until old // one is completely deleted. Even if GR is supported, new // connection cannot be accepted until old one is fully cleaned up. - if (!xmpp_server->IsPeerCloseGraceful() || state != xmsm::ACTIVE) { + bool ready = channel->GetPeerState() == xmps::READY; + if (!xmpp_server->IsPeerCloseGraceful() || ready || + channel->IsCloseInProgress()) { // Bring down old session if it is still in ESTABLISHED state. // This is the scenario in which old session's TCP did not learn // the session down event, possibly due to compute cold reboot. // In that case, trigger closure (and possibly GR) process for // the old session. - if (state == xmsm::ESTABLISHED) - state_machine->Enqueue(xmsm::EvTcpClose( - state_machine->session())); + if (ready) { + XmppStateMachine *sm = + endpoint->connection()->state_machine(); + sm->Enqueue(xmsm::EvTcpClose(sm->session())); + } Enqueue(xmsm::EvTcpClose(session)); return false; } @@ -1666,31 +1666,28 @@ void XmppStateMachine::ResurrectOldConnection(XmppConnection *new_connection, if (created) return; - // There is no connection associated with older end point. Treat it as - // a new endpoint being created (XXX Should we assert here instead ?) - if (!connection_endpoint->connection()) { - connection_endpoint->set_connection(new_connection); - return; - } - // Retrieve old XmppConnection and XmppStateMachine (to reuse) XmppConnection *old_xmpp_connection = connection_endpoint->connection(); + assert(old_xmpp_connection); + XmppStateMachine *old_state_machine = old_xmpp_connection->state_machine(); + assert(old_state_machine); // Swap Old and New connections and state machines linkages new_connection->SwapXmppStateMachine(old_xmpp_connection); this->SwapXmppConnection(old_state_machine); - // Update XmppConnections from the sessions. + // Update XmppConnection in the old session. XmppSession *old_xmpp_session = old_state_machine->session(); if (old_xmpp_session) old_xmpp_session->SetConnection(new_connection); new_session->SetConnection(old_xmpp_connection); - // Swap old xmpp session with the new one. + // Set new session with the old connection as it would be the current active + // connection from now on. old_xmpp_connection->set_session(new_session); - // Trigger deletion of the new connection (which now is linked wth - // the old_state_machine and old_xmpp_session + // Trigger deletion of the new connection which now is associated wth the + // the old_state_machine new_connection->Shutdown(); }