-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added timeout to send for tcp_client and tcp_client-windows #3355
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
|
||
namespace spdlog { | ||
namespace details { | ||
|
||
class tcp_client { | ||
SOCKET socket_ = INVALID_SOCKET; | ||
|
||
|
@@ -37,42 +38,25 @@ class tcp_client { | |
::FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, NULL, | ||
last_error, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), buf, | ||
(sizeof(buf) / sizeof(char)), NULL); | ||
|
||
throw_spdlog_ex(fmt_lib::format("tcp_sink - {}: {}", msg, buf)); | ||
} | ||
|
||
public: | ||
tcp_client() { init_winsock_(); } | ||
|
||
~tcp_client() { | ||
close(); | ||
::WSACleanup(); | ||
} | ||
|
||
bool is_connected() const { return socket_ != INVALID_SOCKET; } | ||
|
||
void close() { | ||
::closesocket(socket_); | ||
socket_ = INVALID_SOCKET; | ||
} | ||
|
||
SOCKET fd() const { return socket_; } | ||
|
||
// try to connect or throw on failure | ||
void connect(const std::string &host, int port) { | ||
// Added timeout_ms parameter (in milliseconds) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please fix the comment. is it for connect only? for read and write? in seconds? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed the comments. |
||
void connect(const std::string &host, int port, int timeout_sec) { | ||
if (is_connected()) { | ||
close(); | ||
} | ||
struct addrinfo hints {}; | ||
struct addrinfo hints{}; | ||
ZeroMemory(&hints, sizeof(hints)); | ||
|
||
hints.ai_family = AF_UNSPEC; // To work with IPv4, IPv6, and so on | ||
hints.ai_family = AF_UNSPEC; // IPv4 or IPv6 | ||
hints.ai_socktype = SOCK_STREAM; // TCP | ||
hints.ai_flags = AI_NUMERICSERV; // port passed as as numeric value | ||
hints.ai_flags = AI_NUMERICSERV; // port as numeric value | ||
hints.ai_protocol = 0; | ||
int timeout_ms = timeout_sec * 1000; | ||
|
||
auto port_str = std::to_string(port); | ||
struct addrinfo *addrinfo_result; | ||
struct addrinfo *addrinfo_result = nullptr; | ||
auto rv = ::getaddrinfo(host.c_str(), port_str.c_str(), &hints, &addrinfo_result); | ||
int last_error = 0; | ||
if (rv != 0) { | ||
|
@@ -81,13 +65,11 @@ class tcp_client { | |
throw_winsock_error_("getaddrinfo failed", last_error); | ||
} | ||
|
||
// Try each address until we successfully connect(2). | ||
|
||
// Try each address until we successfully connect. | ||
for (auto *rp = addrinfo_result; rp != nullptr; rp = rp->ai_next) { | ||
socket_ = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol); | ||
if (socket_ == INVALID_SOCKET) { | ||
last_error = ::WSAGetLastError(); | ||
WSACleanup(); | ||
continue; | ||
} | ||
if (::connect(socket_, rp->ai_addr, (int)rp->ai_addrlen) == 0) { | ||
|
@@ -103,33 +85,66 @@ class tcp_client { | |
throw_winsock_error_("connect failed", last_error); | ||
} | ||
|
||
if (::setsockopt(socket_, SOL_SOCKET, SO_RCVTIMEO, | ||
reinterpret_cast<const char *>(&timeout_ms), | ||
sizeof(timeout_ms)) == SOCKET_ERROR) { | ||
last_error = ::WSAGetLastError(); | ||
WSACleanup(); | ||
throw_winsock_error_("setsockopt(SO_RCVTIMEO) failed", last_error); | ||
} | ||
if (::setsockopt(socket_, SOL_SOCKET, SO_SNDTIMEO, | ||
reinterpret_cast<const char *>(&timeout_ms), | ||
sizeof(timeout_ms)) == SOCKET_ERROR) { | ||
last_error = ::WSAGetLastError(); | ||
WSACleanup(); | ||
throw_winsock_error_("setsockopt(SO_SNDTIMEO) failed", last_error); | ||
} | ||
|
||
// set TCP_NODELAY | ||
int enable_flag = 1; | ||
::setsockopt(socket_, IPPROTO_TCP, TCP_NODELAY, reinterpret_cast<char *>(&enable_flag), | ||
sizeof(enable_flag)); | ||
} | ||
|
||
bool is_connected() const { return socket_ != INVALID_SOCKET; } | ||
|
||
void close() { | ||
if (is_connected()) { | ||
::closesocket(socket_); | ||
socket_ = INVALID_SOCKET; | ||
} | ||
} | ||
|
||
SOCKET fd() const { return socket_; } | ||
|
||
~tcp_client() { | ||
close(); | ||
::WSACleanup(); | ||
} | ||
|
||
// Send exactly n_bytes of the given data. | ||
// On error close the connection and throw. | ||
void send(const char *data, size_t n_bytes) { | ||
size_t bytes_sent = 0; | ||
while (bytes_sent < n_bytes) { | ||
const int send_flags = 0; | ||
auto write_result = | ||
::send(socket_, data + bytes_sent, (int)(n_bytes - bytes_sent), send_flags); | ||
auto write_result = ::send(socket_, data + bytes_sent, | ||
static_cast<int>(n_bytes - bytes_sent), send_flags); | ||
if (write_result == SOCKET_ERROR) { | ||
int last_error = ::WSAGetLastError(); | ||
close(); | ||
if (last_error == WSAETIMEDOUT) { | ||
throw_winsock_error_("Connection timed out", last_error); | ||
} | ||
throw_winsock_error_("send failed", last_error); | ||
} | ||
|
||
if (write_result == 0) // (probably should not happen but in any case..) | ||
{ | ||
if (write_result == 0) { // (probably should not happen but in any case..) | ||
break; | ||
} | ||
bytes_sent += static_cast<size_t>(write_result); | ||
} | ||
} | ||
}; | ||
|
||
} // namespace details | ||
} // namespace spdlog |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,9 +40,9 @@ class tcp_client { | |
~tcp_client() { close(); } | ||
|
||
// try to connect or throw on failure | ||
void connect(const std::string &host, int port) { | ||
void connect(const std::string &host, int port, int timeout_sec) { | ||
close(); | ||
struct addrinfo hints {}; | ||
struct addrinfo hints{}; | ||
memset(&hints, 0, sizeof(struct addrinfo)); | ||
hints.ai_family = AF_UNSPEC; // To work with IPv4, IPv6, and so on | ||
hints.ai_socktype = SOCK_STREAM; // TCP | ||
|
@@ -56,7 +56,6 @@ class tcp_client { | |
throw_spdlog_ex(fmt_lib::format("::getaddrinfo failed: {}", gai_strerror(rv))); | ||
} | ||
|
||
// Try each address until we successfully connect(2). | ||
int last_errno = 0; | ||
for (auto *rp = addrinfo_result; rp != nullptr; rp = rp->ai_next) { | ||
#if defined(SOCK_CLOEXEC) | ||
|
@@ -69,6 +68,14 @@ class tcp_client { | |
last_errno = errno; | ||
continue; | ||
} | ||
struct timeval timeout; | ||
timeout.tv_sec = timeout_sec; | ||
timeout.tv_usec = 0; | ||
::setsockopt(socket_, SOL_SOCKET, SO_RCVTIMEO, reinterpret_cast<char *>(&timeout), | ||
sizeof(timeout)); | ||
::setsockopt(socket_, SOL_SOCKET, SO_SNDTIMEO, reinterpret_cast<char *>(&timeout), | ||
sizeof(timeout)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i dont think it affects connect timeout. only recv and send There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it only send and recv timeout, I can do it for connect but it would be non blocking I guess. Can I proceed with that ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am confused. Isn't the connect timeout is the whole point of this pr? |
||
|
||
rv = ::connect(socket_, rp->ai_addr, rp->ai_addrlen); | ||
if (rv == 0) { | ||
break; | ||
|
@@ -111,7 +118,11 @@ class tcp_client { | |
auto write_result = | ||
::send(socket_, data + bytes_sent, n_bytes - bytes_sent, send_flags); | ||
if (write_result < 0) { | ||
int err = errno; | ||
close(); | ||
if (err == ETIMEDOUT) { | ||
throw_spdlog_ex("Connection timed out", ETIMEDOUT); | ||
} | ||
throw_spdlog_ex("write(2) failed", errno); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undo all the changes in example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, should I take the latest example from the repo.