From c053208c42664088fbec18c95bdebf983ff4009d Mon Sep 17 00:00:00 2001 From: Prabhjot Singh Sethi Date: Fri, 10 Jul 2015 11:24:32 +0530 Subject: [PATCH] Handle Session Close before established processing Issue: ------ Tor agent tries to set socket option on closed socket resulting in EBADF and assertion, this session was already closed by time connection establish was getting processed Fix: ---- Skip connection establish for session, if session is already closed. Added test case to simulate the event. Closes-Bug: 1472423 Change-Id: I49b2bf873cf5a36f27eee1c9f22cdfa2a9d48835 (cherry picked from commit 3e66e25913f4fbb129d1c0b9b4834d2daaa26c3b) --- .../ovsdb_client/ovsdb_client_session.cc | 3 ++ .../ovsdb_client/ovsdb_client_tcp.cc | 24 +++++++++++---- .../ovsdb_client/ovsdb_client_tcp.h | 2 ++ .../ovsdb_client/test/test_ovs_event.cc | 30 ++++++++++++++++--- 4 files changed, 49 insertions(+), 10 deletions(-) diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_session.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_session.cc index 94e6d117031..bf4e30396c2 100644 --- a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_session.cc +++ b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_session.cc @@ -127,6 +127,9 @@ void OvsdbClientSession::OnEstablish() { void OvsdbClientSession::OnClose() { OVSDB_SESSION_TRACE(Trace, this, "Connection to client Closed"); + if (!idl_inited_) { + return; + } client_idl_->TriggerDeletion(); } diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_tcp.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_tcp.cc index 9a726fe2b3b..60f17cf5dc2 100644 --- a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_tcp.cc +++ b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_tcp.cc @@ -78,6 +78,10 @@ void OvsdbClientTcp::set_connect_complete_cb(SessionEventCb cb) { connect_complete_cb_ = cb; } +void OvsdbClientTcp::set_pre_connect_complete_cb(SessionEventCb cb) { + pre_connect_complete_cb_ = cb; +} + OvsdbClientSession *OvsdbClientTcp::FindSession(Ip4Address ip, uint16_t port) { // match both ip and port with available session // if port is not provided match only ip @@ -222,12 +226,20 @@ bool OvsdbClientTcpSession::ProcessSessionEvent(OvsdbSessionEvent ovs_event) { } break; case TcpSession::CONNECT_COMPLETE: - ec = SetSocketOptions(); - assert(ec.value() == 0); - set_status("Established"); - OnEstablish(); - if (!ovs_server->connect_complete_cb_.empty()) { - ovs_server->connect_complete_cb_(this); + if (!ovs_server->pre_connect_complete_cb_.empty()) { + ovs_server->pre_connect_complete_cb_(this); + } + if (!IsClosed()) { + ec = SetSocketOptions(); + assert(ec.value() == 0); + set_status("Established"); + OnEstablish(); + if (!ovs_server->connect_complete_cb_.empty()) { + ovs_server->connect_complete_cb_(this); + } + } else { + OVSDB_SESSION_TRACE(Trace, this, "Skipping connection complete on" + " closed session"); } break; default: diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_tcp.h b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_tcp.h index 3dfd86fcd2d..5d37a588f15 100644 --- a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_tcp.h +++ b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_tcp.h @@ -118,6 +118,7 @@ class OvsdbClientTcp : public TcpServer, public OvsdbClient { // Used by Test Code to trigger events in specific order void set_connect_complete_cb(SessionEventCb cb); + void set_pre_connect_complete_cb(SessionEventCb cb); // API to shutdown the TCP server void shutdown(); @@ -136,6 +137,7 @@ class OvsdbClientTcp : public TcpServer, public OvsdbClient { Ip4Address tsn_ip_; bool shutdown_; SessionEventCb connect_complete_cb_; + SessionEventCb pre_connect_complete_cb_; DISALLOW_COPY_AND_ASSIGN(OvsdbClientTcp); }; }; diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/test/test_ovs_event.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/test/test_ovs_event.cc index e6ec782080b..5657b933613 100644 --- a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/test/test_ovs_event.cc +++ b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/test/test_ovs_event.cc @@ -144,14 +144,15 @@ class OvsdbEventTest : public ::testing::Test { } void ImmediateCloseSessionEvent(OvsdbClientTcpSession *session) { - if (session->client_idl()->deleted()) { - return; + if (session->client_idl() && !session->client_idl()->deleted()) { + // take reference for idl to validate + immediate_close_idl_ = session->client_idl(); } - // take reference for idl to validate - immediate_close_idl_ = session->client_idl(); session->TriggerClose(); OvsdbClientTcp *ovs_server = static_cast(init_->ovsdb_client()); + ovs_server->set_pre_connect_complete_cb( + boost::bind(&OvsdbEventTest::NoOpSessionEvent, this, _1)); ovs_server->set_connect_complete_cb( boost::bind(&OvsdbEventTest::ImmediateCloseSessionEventDone, this, _1)); tcp_session_ = NULL; @@ -190,6 +191,27 @@ TEST_F(OvsdbEventTest, ImmediateConnectionDown) { client->WaitForIdle(); } +TEST_F(OvsdbEventTest, ImmediateConnectionDownBeforeEstablish) { + TestTaskHold *hold = + new TestTaskHold(TaskScheduler::GetInstance()->GetTaskId("db::DBTableStop"), 0); + + OvsdbClientTcp *ovs_server = + static_cast(init_->ovsdb_client()); + // Set callback to close the session before processing connect complete event + ovs_server->set_pre_connect_complete_cb( + boost::bind(&OvsdbEventTest::ImmediateCloseSessionEvent, this, _1)); + + immediate_close_done_ = false; + tcp_session_->TriggerClose(); + // wait for completion of immediate_close + WAIT_FOR(500, 10000, (immediate_close_done_ == true)); + + WAIT_FOR(100, 10000, tcp_session_ != NULL && + tcp_session_->status() == string("Established")); + delete hold; + client->WaitForIdle(); +} + int main(int argc, char *argv[]) { GETUSERARGS(); // override with true to initialize ovsdb server and client