Skip to content

Commit

Permalink
Fix issue of unexpected response timeouts when an HTTP client with lo…
Browse files Browse the repository at this point in the history 
…ng time keep-alive (#5588)
  • Loading branch information
@NathanFreeman
NathanFreeman authored Nov 26, 2024
1 parent 3c7d3f5 commit e4add24
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 24 deletions.
19 changes: 11 additions & 8 deletions ext-src/swoole_http_client_coro.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ class Client {
#ifdef SW_USE_OPENSSL
uint8_t ssl;
#endif
double connect_timeout = network::Socket::default_connect_timeout;
double connect_timeout = 0;
double response_timeout = 0;
bool defer = false;
bool lowercase_header = true;
bool use_default_port;
Expand Down Expand Up @@ -706,10 +707,12 @@ void Client::apply_setting(zval *zset, const bool check_all) {
zval *ztmp;
HashTable *vht = Z_ARRVAL_P(zset);

if (php_swoole_array_get_value(vht, "connect_timeout", ztmp) ||
php_swoole_array_get_value(vht, "timeout", ztmp) /* backward compatibility */) {
if (php_swoole_array_get_value(vht, "connect_timeout", ztmp)) {
connect_timeout = zval_get_double(ztmp);
}
if (php_swoole_array_get_value(vht, "timeout", ztmp)) {
response_timeout = zval_get_double(ztmp);
}
if (php_swoole_array_get_value(vht, "max_retries", ztmp)) {
max_retries = (uint8_t) SW_MIN(zval_get_long(ztmp), UINT8_MAX);
}
Expand Down Expand Up @@ -806,11 +809,11 @@ bool Client::connect() {
accept_websocket_compression = false;
#endif

// socket->set_buffer_allocator(&SWOOLE_G(zend_string_allocator));
// connect
socket->set_timeout(connect_timeout, Socket::TIMEOUT_CONNECT);
double _timeout = connect_timeout == 0 ? network::Socket::default_connect_timeout : connect_timeout;
socket->set_timeout(_timeout, Socket::TIMEOUT_CONNECT);
socket->set_resolve_context(&resolve_context_);
socket->set_dtor([this](Socket *_socket) { socket_dtor(); });
// socket->set_buffer_allocator(&SWOOLE_G(zend_string_allocator));

if (!socket->connect(host, port)) {
set_error(socket->errCode, socket->errMsg, ESTATUS_CONNECT_FAILED);
Expand Down Expand Up @@ -1368,9 +1371,9 @@ bool Client::recv_response(double timeout) {
parser.data = this;

if (timeout == 0) {
timeout = socket->get_timeout(Socket::TIMEOUT_READ);
timeout = response_timeout == 0 ? network::Socket::default_read_timeout : response_timeout;
}
Socket::timeout_controller tc(socket, timeout, Socket::TIMEOUT_READ);
Socket::TimeoutController tc(socket, timeout, Socket::TIMEOUT_READ);
bool success = false;
while (true) {
if (sw_unlikely(tc.has_timedout(Socket::TIMEOUT_READ))) {
Expand Down
4 changes: 2 additions & 2 deletions ext-src/swoole_mysql_coro.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class MysqlClient {
Socket *socket = nullptr;
zval zsocket;
zval zobject;
Socket::timeout_controller *tc = nullptr;
Socket::TimeoutController *tc = nullptr;

enum sw_mysql_state state = SW_MYSQL_STATE_CLOSED;
bool quit = false;
Expand Down Expand Up @@ -174,7 +174,7 @@ class MysqlClient {
// Notice: `timeout > 0` is wrong, maybe -1
if (timeout != 0) {
SW_ASSERT(!tc);
tc = new Socket::timeout_controller(socket, timeout, type);
tc = new Socket::TimeoutController(socket, timeout, type);
}
}

Expand Down
29 changes: 15 additions & 14 deletions include/swoole_coroutine_socket.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -512,8 +512,8 @@ class Socket {

class TimerController {
public:
TimerController(TimerNode **timer_pp, double timeout, Socket *sock, TimerCallback callback)
: timer_pp(timer_pp), timeout(timeout), socket_(sock), callback(callback) {}
TimerController(TimerNode **_timer_pp, double _timeout, Socket *_socket, TimerCallback _callback)
: timer_pp(_timer_pp), timeout(_timeout), socket_(_socket), callback(std::move(_callback)) {}
bool start() {
if (timeout != 0 && !*timer_pp) {
enabled = true;
Expand Down Expand Up @@ -545,16 +545,16 @@ class Socket {
public:
class TimeoutSetter {
public:
TimeoutSetter(Socket *socket, double timeout, const enum TimeoutType type)
: socket_(socket), timeout(timeout), type(type) {
if (timeout == 0) {
TimeoutSetter(Socket *socket, double _timeout, const enum TimeoutType _type)
: socket_(socket), timeout(_timeout), type(_type) {
if (_timeout == 0) {
return;
}
for (uint8_t i = 0; i < SW_ARRAY_SIZE(timeout_type_list); i++) {
if (type & timeout_type_list[i]) {
if (_type & timeout_type_list[i]) {
original_timeout[i] = socket->get_timeout(timeout_type_list[i]);
if (timeout != original_timeout[i]) {
socket->set_timeout(timeout, timeout_type_list[i]);
if (_timeout != original_timeout[i]) {
socket->set_timeout(_timeout, timeout_type_list[i]);
}
}
}
Expand All @@ -579,12 +579,13 @@ class Socket {
double original_timeout[sizeof(timeout_type_list)] = {};
};

class timeout_controller : public TimeoutSetter {
class TimeoutController : public TimeoutSetter {
public:
timeout_controller(Socket *socket, double timeout, const enum TimeoutType type)
: TimeoutSetter(socket, timeout, type) {}
bool has_timedout(const enum TimeoutType type) {
SW_ASSERT_1BYTE(type);
TimeoutController(Socket *_socket, double _timeout, const enum TimeoutType _type)
: TimeoutSetter(_socket, _timeout, _type) {}

bool has_timedout(const enum TimeoutType _type) {
SW_ASSERT_1BYTE(_type);
if (timeout > 0) {
if (sw_unlikely(startup_time == 0)) {
startup_time = microtime();
Expand All @@ -594,7 +595,7 @@ class Socket {
socket_->set_err(ETIMEDOUT);
return true;
}
socket_->set_timeout(timeout - used_time, type);
socket_->set_timeout(timeout - used_time, _type);
}
}
return false;
Expand Down

0 comments on commit e4add24

Please sign in to comment.