kep-808 Made changes requested by Isabel

bluzelle · Mar 2, 2019 · 9225e3d · 9225e3d
1 parent 24929e5
commit 9225e3d
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 12 deletions.
diff --git a/pbft/pbft.cpp b/pbft/pbft.cpp
@@ -295,7 +295,7 @@ pbft::handle_request(const bzn_envelope& request_env, const std::shared_ptr<sess
     {
         if (this->sessions_waiting_on_forwarded_requests.find(hash) == this->sessions_waiting_on_forwarded_requests.end())
         {
-            this->add_session_to_pool(hash, session);
+            this->add_session_to_sessions_waiting(hash, session);
         }
     }
 
@@ -444,7 +444,7 @@ pbft::handle_join_or_leave(const bzn_envelope& env, const pbft_membership_msg& m
             return;
         }
 
-        this->add_session_to_pool(msg_hash, session);
+        this->add_session_to_sessions_waiting(msg_hash, session);
 
         if (!this->is_primary())
         {
@@ -1901,16 +1901,17 @@ uint32_t pbft::generate_random_number(uint32_t min, uint32_t max)
     return dist(gen);
 }
 
-void pbft::add_session_to_pool(const std::string& msg_hash, std::shared_ptr<bzn::session_base> session)
+void pbft::add_session_to_sessions_waiting(const std::string &msg_hash, std::shared_ptr<bzn::session_base> session)
 {
     if (session)
     {
         this->sessions_waiting_on_forwarded_requests[msg_hash] = session;
-        session->add_shutdown_handler([msg_hash, this]()
+        session->add_shutdown_handler([msg_hash, this, session]()
                                         {
                                             std::lock_guard<std::mutex> lock(this->pbft_lock);
                                             auto it = this->sessions_waiting_on_forwarded_requests.find(msg_hash);
-                                            if (it != this->sessions_waiting_on_forwarded_requests.end() && !it->second->is_open())
+                                            if (it != this->sessions_waiting_on_forwarded_requests.end()
+                                            && (it->second->get_session_id() == session->get_session_id()))
                                             {
                                                 this->sessions_waiting_on_forwarded_requests.erase(it);
                                             }

diff --git a/pbft/pbft.hpp b/pbft/pbft.hpp
@@ -50,7 +50,7 @@ namespace bzn
     {
         // fwd declare test as it's not in the same namespace...
         class pbft_test_database_response_is_forwarded_to_session_Test;
-        class pbft_test_add_session_to_pool_can_add_a_session_and_shutdown_handler_removes_session_from_pool_Test;
+        class pbft_test_add_session_to_sessions_waiting_can_add_a_session_and_shutdown_handler_removes_session_from_sessions_waiting_Test;
     }
 
     using request_hash_t = std::string;
@@ -217,7 +217,7 @@ namespace bzn
         bool is_peer(const bzn::uuid_t& peer) const;
         bool get_sequences_and_request_hashes_from_proofs( const pbft_msg& viewchange_msg, std::set<std::pair<uint64_t, std::string>>& sequence_request_pairs) const;
 
-        void add_session_to_pool(const std::string& msg_hash, std::shared_ptr<bzn::session_base> session);
+        void add_session_to_sessions_waiting(const std::string &msg_hash, std::shared_ptr<bzn::session_base> session);
 
         // Using 1 as first value here to distinguish from default value of 0 in protobuf
         uint64_t view = 1;
@@ -299,7 +299,7 @@ namespace bzn
         FRIEND_TEST(pbft_newview_test, get_sequences_and_request_hashes_from_proofs);
         FRIEND_TEST(pbft_newview_test, test_last_sequence_in_newview_prepared_proofs);
         FRIEND_TEST(bzn::test::pbft_test, database_response_is_forwarded_to_session);
-        FRIEND_TEST(bzn::test::pbft_test, add_session_to_pool_can_add_a_session_and_shutdown_handler_removes_session_from_pool);
+        FRIEND_TEST(bzn::test::pbft_test, add_session_to_sessions_waiting_can_add_a_session_and_shutdown_handler_removes_session_from_sessions_waiting);
 
         friend class pbft_proto_test;
         friend class pbft_join_leave_test;

diff --git a/pbft/test/pbft_test.cpp b/pbft/test/pbft_test.cpp
@@ -200,7 +200,7 @@ namespace bzn::test
         this->database_response_handler(this->request_msg, mock_session);
     }
 
-    TEST_F(pbft_test, add_session_to_pool_can_add_a_session_and_shutdown_handler_removes_session_from_pool)
+    TEST_F(pbft_test, add_session_to_sessions_waiting_can_add_a_session_and_shutdown_handler_removes_session_from_sessions_waiting)
     {
         this->build_pbft();
 
@@ -214,9 +214,6 @@ namespace bzn::test
                 shutdown_handler = handler;
             }));
 
-        EXPECT_CALL(*mock_session, is_open())
-            .WillOnce(Return(false));
-
         pbft->handle_database_message(this->request_msg, this->mock_session);
 
         EXPECT_EQ(size_t(1), this->pbft->sessions_waiting_on_forwarded_requests.size());