Commit ed13fb5c authored by Thomas Markwalder's avatar Thomas Markwalder

[#964,!577] Revamped to detect and close OOB Connections

Rather than just unregistering the socket, we now actually close
the Connection. This ensures we never end up with an unregistered
but open connection.

src/hooks/dhcp/high_availability/ha_service.*
    HAService::clientConnectHandler() - modified to call
    HttpClient::closeIfOutOfBandwidth().

src/lib/http/client.*
    Connection - replaced isTransaction(int socket_fd) with
    isMySocket(int socket_fd)

    ConnectionPool - replaced isTransaction(int socket_fd) with
    closeIfOutOfBandwidth(int socket_fd)

    HttpClient - replaced isTransaction(int socket_fd) with
    closeIfOutOfBandwidth(int socket_fd)
parent 16f37ca0
......@@ -1647,10 +1647,10 @@ HAService::clientConnectHandler(const boost::system::error_code& ec, int tcp_nat
void
HAService::socketReadyHandler(int tcp_native_fd) {
// 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);
}
// ongoing transactions, we close it. This will unregister it from
// IfaceMgr and ensure the client starts over with a fresh connection
// if it needs to do so.
client_.closeIfOutOfBandwidth(tcp_native_fd);
}
void
......
......@@ -744,16 +744,14 @@ protected:
/// flagged as ready to read. It is installed by the invocation to
/// register the socket with IfaceMgr made in @ref clientConnectHandler.
///
/// 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.
/// The handler calls @ref HttpClient::closeIfOutOfBandwidth() to catch
/// and close any sockets that have gone ready outside of transactions.
///
/// 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
/// the IfaceMgr::select(). If this happens 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.
/// This causes IfaceMgr::select() to endlessly interrupt on the socket.
///
/// @param tcp_native_fd socket descriptor of the ready socket
void socketReadyHandler(int tcp_native_fd);
......
......@@ -139,11 +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.
/// @brief Checks if a socket descriptor belongs to this connection.
///
/// @return true if transaction has been initiated, false otherwise.
bool isTransactionOngoing(int socket_fd) const;
/// @param socket_fd socket descriptor to check
///
/// @return True if the socket fd belongs to this connection.
bool isMySocket(int socket_fd) const;
/// @brief Checks and logs if premature transaction timeout is suspected.
///
......@@ -445,21 +446,41 @@ public:
queue_.clear();
}
/// @brief Deteremines if there is an ongoing transaction associated
/// with a given socket desecriptor.
/// @brief Closes a connection if it has an out-of-bandwidth socket event
///
/// @param socket_fd socket descriptor to check
/// If the pool contains a connection using the given socket and that
/// connection is currently in a transaction the method returns as this
/// indicates a normal ready event. If the connection is not in an
/// ongoing transaction, then the connection is closed.
///
/// @return True if the pool contains a connection which is using the
/// given socket descriptor for an ongoing transaction.
bool isTransactionOngoing(int socket_fd) {
/// This is method is intended to be used to detect and clean up then
/// sockets that are marked ready outside of transactions. The most comman
/// case is the other end of the socket being closed.
///
/// @param socket_fd socket descriptor to check
void closeIfOutOfBandwidth(int socket_fd) {
// First we look for a connection with the socket.
for (auto conns_it = conns_.begin(); conns_it != conns_.end();
++conns_it) {
if (conns_it->second->isTransactionOngoing(socket_fd)) {
return (true);
if (!conns_it->second->isMySocket(socket_fd)) {
// Not this connection.
continue;
}
if (conns_it->second->isTransactionOngoing()) {
// Matches but is in a transaction, all is well.
return;
}
// Socket has no transaction, so any ready event is
// out-of-bandwidth (other end probably closed), so
// let's close it. Note we do not remove any queued
// requests, as this might somehow be occurring in
// between them.
conns_it->second->close();
conns_.erase(conns_it);
}
return (false);
}
private:
......@@ -631,9 +652,8 @@ Connection::isTransactionOngoing() const {
}
bool
Connection::isTransactionOngoing(int socket_fd) const {
return ((static_cast<bool>(current_request_)) &&
(socket_.getNative() == socket_fd));
Connection::isMySocket(int socket_fd) const {
return (socket_.getNative() == socket_fd);
}
bool
......@@ -952,9 +972,9 @@ 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::closeIfOutOfBandwidth(int socket_fd) {
return (impl_->conn_pool_->closeIfOutOfBandwidth(socket_fd));
}
void
......
......@@ -191,11 +191,19 @@ public:
/// @brief Closes all connections.
void stop();
/// @brief Deteremines if a socket is part of an ongoing transaction.
/// @brief Closes a connection if it has an out-of-bandwidth socket event
///
/// @return True if the given socket descriptor is being used by an
/// ongoing transaction
bool isTransactionOngoing(int socket_fd) const;
/// If the client owns a connection using the given socket and that
/// connection is currently in a transaction the method returns as this
/// indicates a normal ready event. If the connection is not in an
/// ongoing transaction, then the connection is closed.
///
/// This is method is intended to be used to detect and clean up then
/// sockets that are marked ready outside of transactions. The most comman
/// case is the other end of the socket being closed.
///
/// @param socket_fd socket descriptor to check
void closeIfOutOfBandwidth(int socket_fd);
private:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment