]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#964,!577] Added external socket ready handler to HAService
authorThomas Markwalder <tmark@isc.org>
Fri, 25 Oct 2019 19:49:34 +0000 (15:49 -0400)
committerThomas Markwalder <tmark@isc.org>
Tue, 5 Nov 2019 16:23:48 +0000 (11:23 -0500)
src/hooks/dhcp/high_availability/ha_service.*
    HAService::socketReadyHandler(int tcp_native_fd) - new handler for external socket ready
    callback. It detects out-of-transaction ready socket and unregisters it.

src/lib/http/client.*
    isTransactionOngoing(int socket_fd) - new method to return true if any of the clients
    connections are using the socket in an ongoing transaction

src/hooks/dhcp/high_availability/ha_service.cc
src/hooks/dhcp/high_availability/ha_service.h
src/lib/http/client.cc
src/lib/http/client.h

index 0cb08bf658c433bebf81a4cc4d5234b8afb3c2a2..ec78a5d74d0f59c199335e0f6ff8f8744d65bfe9 100644 (file)
@@ -1646,12 +1646,11 @@ HAService::clientConnectHandler(const boost::system::error_code& ec, int tcp_nat
 
 void
 HAService::socketReadyHandler(int tcp_native_fd) {
-    std::cout << "HAService::socketReadyHandler - ready socket:" << tcp_native_fd << std::endl;
-    // If the socket is not usable/or in a transaction
-    // we'll unregister it
-    // if (socket bad) /{
-    //      IfaceMgr::instance().deleteExternalSocket(tcp_native_fd, 0);
-    // }
+    // If the socket is ready but does not belong to one of our client's
+    // ongoing transactions we unregister it.
+    if (!client_.isTransactionOngoing(tcp_native_fd)) {
+        IfaceMgr::instance().deleteExternalSocket(tcp_native_fd);
+    }
 }
 
 void
index 48efda89fe178b4beecdd43dde318a85e83ac8cb..d364fb1ddd534914fc75e357993a755c2ca2b027 100644 (file)
@@ -744,9 +744,16 @@ protected:
     /// flagged as ready to read.   It is installed by the invocation to
     /// register the socket with IfaceMgr made in @ref clientConnectHandler.
     ///
-    /// @todo add logic to determine if the
-    /// socket is involved in an ongoing transcation or not. If not, it
-    /// will be unregistered from IfaceMgr.
+    /// The handler checks if the ready socket belongs to one our client's
+    /// ongoing transactions.  If it does, it simply returns.  If it does
+    /// not belong to an ongoing transactions, the socket descriptor is
+    /// unregistered from IfaceMgr.
+    ///
+    /// We do this in case the other peer closed the socket (e.g. idle timeout
+    /// for example), as this will cause the socket /appear ready to read to
+    /// the IfaceMgr::select(). If this happens in while no transcations are
+    /// in progess, we won't have anything to deal with the socket event.
+    /// This causes IfaceMgr::select() to endlessly interrupt on this socket.
     ///
     /// @param tcp_native_fd socket descriptor of the ready socket
     void socketReadyHandler(int tcp_native_fd);
index 50a581fcfbaf712ca5c00b6be428938b59cf0d90..e056170efd97a111ba62722bcb996ce89c416f1f 100644 (file)
@@ -139,6 +139,12 @@ public:
     /// @return true if transaction has been initiated, false otherwise.
     bool isTransactionOngoing() const;
 
+    /// @brief Checks if a transaction has been initiated over this connection and
+    /// socket descriptor.
+    ///
+    /// @return true if transaction has been initiated, false otherwise.
+    bool isTransactionOngoing(int socket_fd) const;
+
     /// @brief Checks and logs if premature transaction timeout is suspected.
     ///
     /// There are cases when the premature timeout occurs, e.g. as a result of
@@ -439,6 +445,23 @@ public:
         queue_.clear();
     }
 
+    /// @brief Deteremines if there is an ongoing transaction associated
+    /// with a given socket desecriptor.
+    ///
+    /// @param socket_fd socket descriptor to check
+    ///
+    /// @return True if the pool contains a connection which is using the
+    /// given socket descriptor for an ongoing transaction.
+    bool isTransactionOngoing(int socket_fd) {
+        for (auto conns_it = conns_.begin(); conns_it != conns_.end();
+             ++conns_it) {
+            if (conns_it->second->isTransactionOngoing(socket_fd)) {
+                return (true);
+            }
+        }
+        return (false);
+    }
+
 private:
 
     /// @brief Holds reference to the IO service.
@@ -607,6 +630,12 @@ Connection::isTransactionOngoing() const {
     return (static_cast<bool>(current_request_));
 }
 
+bool
+Connection::isTransactionOngoing(int socket_fd) const {
+    return ((static_cast<bool>(current_request_)) &&
+            (socket_.getNative() == socket_fd));
+}
+
 bool
 Connection::checkPrematureTimeout(const uint64_t transid) {
     // If there is no transaction but the handlers are invoked it means
@@ -923,6 +952,11 @@ HttpClient::asyncSendRequest(const Url& url, const HttpRequestPtr& request,
                                     request_callback, connect_callback, close_callback);
 }
 
+bool
+HttpClient::isTransactionOngoing(int socket_fd) const {
+    return (impl_->conn_pool_->isTransactionOngoing(socket_fd));
+}
+
 void
 HttpClient::stop() {
     impl_->conn_pool_->closeAll();
index b72605222adf4d97168806a67be432b91339da5d..95f1af977984515bca9cbf6f76ed50603281455e 100644 (file)
@@ -191,6 +191,12 @@ public:
     /// @brief Closes all connections.
     void stop();
 
+    /// @brief Deteremines if a socket is part of an ongoing transaction.
+    ///
+    /// @return True if the given socket descriptor is being used by an
+    /// ongoing transaction
+    bool isTransactionOngoing(int socket_fd) const;
+
 private:
 
     /// @brief Pointer to the HTTP client implementation.