Commit cb11b477 authored by Thomas Markwalder's avatar Thomas Markwalder
Browse files

[#691,!395] Review comments 2

src/hooks/dhcp/high_availability/ha_messages.mes
    HA_SERVICE_CONNECT_INVALID_SOCKET - new message

src/hooks/dhcp/high_availability/ha_service.cc
 HAService::clientConnectHandler() - added negative fd logic

src/lib/http/client.cc
    Connection::closeCallback() - new method that wraps invocation
    of close callback in try-catch.

src/lib/http/http_messages.mes
    HTTP_CONNECTION_CLOSE_CALLBACK_FAILED - new message
parent de3af2b8
// File created from ../../../../src/hooks/dhcp/high_availability/ha_messages.mes on Mon Jun 24 2019 17:05
// File created from ../../../../src/hooks/dhcp/high_availability/ha_messages.mes on Wed Jun 26 2019 09:25
#include <cstddef>
#include <log/message_types.h>
......@@ -60,6 +60,7 @@ extern const isc::log::MessageID HA_LOCAL_DHCP_DISABLE = "HA_LOCAL_DHCP_DISABLE"
extern const isc::log::MessageID HA_LOCAL_DHCP_ENABLE = "HA_LOCAL_DHCP_ENABLE";
extern const isc::log::MessageID HA_MISSING_CONFIGURATION = "HA_MISSING_CONFIGURATION";
extern const isc::log::MessageID HA_SCOPES_HANDLER_FAILED = "HA_SCOPES_HANDLER_FAILED";
extern const isc::log::MessageID HA_SERVICE_CONNECT_INVALID_SOCKET = "HA_SERVICE_CONNECT_INVALID_SOCKET";
extern const isc::log::MessageID HA_SERVICE_STARTED = "HA_SERVICE_STARTED";
extern const isc::log::MessageID HA_STATE_MACHINE_CONTINUED = "HA_STATE_MACHINE_CONTINUED";
extern const isc::log::MessageID HA_STATE_MACHINE_PAUSED = "HA_STATE_MACHINE_PAUSED";
......@@ -129,6 +130,7 @@ const char* values[] = {
"HA_LOCAL_DHCP_ENABLE", "local DHCP service is enabled while the %1 is in the %2 state",
"HA_MISSING_CONFIGURATION", "high-availability parameter not specified for High Availability hooks library",
"HA_SCOPES_HANDLER_FAILED", "ha-scopes command failed: %1",
"HA_SERVICE_CONNECT_INVALID_SOCKET", "Attempted to register an invalid socket, error code: %1",
"HA_SERVICE_STARTED", "started high availability service in %1 mode as %2 server",
"HA_STATE_MACHINE_CONTINUED", "state machine is un-paused",
"HA_STATE_MACHINE_PAUSED", "state machine paused in state %1",
......
// File created from ../../../../src/hooks/dhcp/high_availability/ha_messages.mes on Mon Jun 24 2019 17:05
// File created from ../../../../src/hooks/dhcp/high_availability/ha_messages.mes on Wed Jun 26 2019 09:25
#ifndef HA_MESSAGES_H
#define HA_MESSAGES_H
......@@ -61,6 +61,7 @@ extern const isc::log::MessageID HA_LOCAL_DHCP_DISABLE;
extern const isc::log::MessageID HA_LOCAL_DHCP_ENABLE;
extern const isc::log::MessageID HA_MISSING_CONFIGURATION;
extern const isc::log::MessageID HA_SCOPES_HANDLER_FAILED;
extern const isc::log::MessageID HA_SERVICE_CONNECT_INVALID_SOCKET;
extern const isc::log::MessageID HA_SERVICE_STARTED;
extern const isc::log::MessageID HA_STATE_MACHINE_CONTINUED;
extern const isc::log::MessageID HA_STATE_MACHINE_PAUSED;
......
......@@ -116,6 +116,11 @@ failure.
This informational message indicates that the High Availability hooks library
has been unloaded successfully.
% HA_SERVICE_CONNECT_INVALID_SOCKET Attempted to register an invalid socket, error code: %1
This is an error message that indicates that the server attempted to
register an invalid socket with the Interface Manager. This is an internal
server error and a bug report should be created.
% HA_DHCP4_START_SERVICE_FAILED failed to start DHCPv4 HA service in dhcp4_srv_configured callout: %1
This error message is issued when an attempt to start High Availability service
for the DHCPv4 server failed in the dhcp4_srv_configured callout. This
......
......@@ -1621,7 +1621,19 @@ HAService::verifyAsyncResponse(const HttpResponsePtr& response) {
bool
HAService::clientConnectHandler(const boost::system::error_code& ec, int tcp_native_fd) {
if (!ec || ec.value() == boost::asio::error::in_progress) {
if (!ec || (ec.value() == boost::asio::error::in_progress)) {
if (ec && (ec.value() == boost::asio::error::in_progress)) {
std::cout << "connect in_progress : " << tcp_native_fd << std::endl;
}
if (tcp_native_fd < 0) {
// This really should not be possible, but just in case.
LOG_ERROR(ha_logger, HA_SERVICE_CONNECT_INVALID_SOCKET)
.arg(ec.value());
return (false);
}
// Register the socket with Interface Manager.
// External socket callback is a NOP. Ready events handlers are run by an
// explicit call IOService ready in kea-dhcp<n> code. We are registering
// the socket only to interrupt main-thread select().
......@@ -1629,7 +1641,7 @@ HAService::clientConnectHandler(const boost::system::error_code& ec, int tcp_nat
}
// If ec.value() == boost::asio::error::already_connected, we should already
// be registered, so nothing to do. If it is any other value, than connect
// be registered, so nothing to do. If it is any other value, then connect
// failed and Connection logic should handle that, not us, so no matter
// what happens we're returning true.
return (true);
......
......@@ -233,6 +233,17 @@ private:
/// @brief Local callback invoked when request timeout occurs.
void timerCallback();
/// @brief Local callback invoked when the connection is closed.
///
/// Invokes the close callback (if one), passing in the socket's
/// descriptor, when the connection's socket about to be closed.
/// The callback invocation is wrapped in a try-catch to ensure
/// exception safety.
///
/// @param clear dictates whether or not the callback is discarded
/// after invocation. Defaults to false.
void closeCallback(const bool clear = false);
/// @brief Pointer to the connection pool owning this connection.
///
/// This is a weak pointer to avoid circular dependency between the
......@@ -502,6 +513,23 @@ Connection::resetState() {
current_callback_ = HttpClient::RequestHandler();
}
void
Connection::closeCallback(const bool clear) {
if (close_callback_) {
try {
close_callback_(socket_.getNative());
} catch (...) {
LOG_ERROR(http_logger, HTTP_CONNECTION_CLOSE_CALLBACK_FAILED);
}
}
if (clear) {
close_callback_ = HttpClient::CloseHandler();
}
}
void
Connection::doTransaction(const HttpRequestPtr& request,
const HttpResponsePtr& response,
......@@ -529,9 +557,7 @@ Connection::doTransaction(const HttpRequestPtr& request,
// data over this socket, when the peer may close the connection. In this
// case we'll need to re-transmit but we don't handle it here.
if (socket_.getASIOSocket().is_open() && !socket_.isUsable()) {
if (close_callback_) {
close_callback_(socket_.getNative());
}
closeCallback();
socket_.close();
}
......@@ -568,11 +594,8 @@ Connection::doTransaction(const HttpRequestPtr& request,
void
Connection::close() {
if (close_callback_) {
close_callback_(socket_.getNative());
close_callback_ = HttpClient::CloseHandler();
}
closeCallback(true);
timer_.cancel();
socket_.close();
resetState();
......@@ -667,7 +690,7 @@ Connection::terminate(const boost::system::error_code& ec,
ConnectionPoolPtr conn_pool = conn_pool_.lock();
if (conn_pool && conn_pool->getNextRequest(url_, request, response, request_timeout,
callback, connect_callback, close_callback)) {
doTransaction(request, response, request_timeout, callback,
doTransaction(request, response, request_timeout, callback,
connect_callback, close_callback);
}
}
......
// File created from ../../../src/lib/http/http_messages.mes on Mon May 13 2019 17:08
// File created from ../../../src/lib/http/http_messages.mes on Wed Jun 26 2019 10:16
#include <cstddef>
#include <log/message_types.h>
......@@ -16,6 +16,7 @@ extern const isc::log::MessageID HTTP_CLIENT_REQUEST_RECEIVED_DETAILS = "HTTP_CL
extern const isc::log::MessageID HTTP_CLIENT_REQUEST_SEND = "HTTP_CLIENT_REQUEST_SEND";
extern const isc::log::MessageID HTTP_CLIENT_REQUEST_SEND_DETAILS = "HTTP_CLIENT_REQUEST_SEND_DETAILS";
extern const isc::log::MessageID HTTP_CLIENT_REQUEST_TIMEOUT_OCCURRED = "HTTP_CLIENT_REQUEST_TIMEOUT_OCCURRED";
extern const isc::log::MessageID HTTP_CONNECTION_CLOSE_CALLBACK_FAILED = "HTTP_CONNECTION_CLOSE_CALLBACK_FAILED";
extern const isc::log::MessageID HTTP_CONNECTION_STOP = "HTTP_CONNECTION_STOP";
extern const isc::log::MessageID HTTP_CONNECTION_STOP_FAILED = "HTTP_CONNECTION_STOP_FAILED";
extern const isc::log::MessageID HTTP_DATA_RECEIVED = "HTTP_DATA_RECEIVED";
......@@ -42,6 +43,7 @@ const char* values[] = {
"HTTP_CLIENT_REQUEST_SEND", "sending HTTP request %1 to %2",
"HTTP_CLIENT_REQUEST_SEND_DETAILS", "detailed information about request sent to %1:\n%2",
"HTTP_CLIENT_REQUEST_TIMEOUT_OCCURRED", "HTTP request timeout occurred when communicating with %1",
"HTTP_CONNECTION_CLOSE_CALLBACK_FAILED", "Connection close callback threw an exception",
"HTTP_CONNECTION_STOP", "stopping HTTP connection from %1",
"HTTP_CONNECTION_STOP_FAILED", "stopping HTTP connection failed",
"HTTP_DATA_RECEIVED", "received %1 bytes from %2",
......
// File created from ../../../src/lib/http/http_messages.mes on Mon May 13 2019 17:08
// File created from ../../../src/lib/http/http_messages.mes on Wed Jun 26 2019 10:16
#ifndef HTTP_MESSAGES_H
#define HTTP_MESSAGES_H
......@@ -17,6 +17,7 @@ extern const isc::log::MessageID HTTP_CLIENT_REQUEST_RECEIVED_DETAILS;
extern const isc::log::MessageID HTTP_CLIENT_REQUEST_SEND;
extern const isc::log::MessageID HTTP_CLIENT_REQUEST_SEND_DETAILS;
extern const isc::log::MessageID HTTP_CLIENT_REQUEST_TIMEOUT_OCCURRED;
extern const isc::log::MessageID HTTP_CONNECTION_CLOSE_CALLBACK_FAILED;
extern const isc::log::MessageID HTTP_CONNECTION_STOP;
extern const isc::log::MessageID HTTP_CONNECTION_STOP_FAILED;
extern const isc::log::MessageID HTTP_DATA_RECEIVED;
......
......@@ -63,6 +63,11 @@ This debug message is issued when the HTTP request timeout has occurred and
the server is going to send a response with Http Request timeout status
code.
% HTTP_CONNECTION_CLOSE_CALLBACK_FAILED Connection close callback threw an exception
This is an error message emitted when the close connection callback
registered on the connection failed unexpectedly. This is a programmatic
error that should be submitted as a bug.
% HTTP_CONNECTION_STOP stopping HTTP connection from %1
This debug message is issued when one of the HTTP connections is stopped.
The connection can be stopped as a result of an error or after the
......
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