Skip to content

Commit 47244b0

Browse files
shioimmshioimm
authored
Fix: Specifying 0 should cause an immediate timeout (ruby#15641)
This change fixes a bug in which specifying 0 for timeout-related options (such as the `timeout` option of `Addrinfo.getaddrinfo`) incorrectly results in an infinite wait. (This change overwrites ruby#15626 .)
1 parent f81c62b commit 47244b0

File tree

7 files changed

+32
-47
lines changed

7 files changed

+32
-47
lines changed

‎ext/socket/ipsocket.c‎

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -83,23 +83,21 @@ init_inetsock_internal(VALUE v)
8383
VALUE open_timeout = arg->open_timeout;
8484
VALUE timeout;
8585
VALUE starts_at;
86-
unsigned int timeout_msec;
8786

8887
timeout = NIL_P(open_timeout) ? resolv_timeout : open_timeout;
89-
timeout_msec = NIL_P(timeout) ? 0 : rsock_value_timeout_to_msec(timeout);
9088
starts_at = current_clocktime();
9189

9290
arg->remote.res = rsock_addrinfo(arg->remote.host, arg->remote.serv,
9391
family, SOCK_STREAM,
94-
(type == INET_SERVER) ? AI_PASSIVE : 0, timeout_msec);
92+
(type == INET_SERVER) ? AI_PASSIVE : 0, timeout);
9593

9694
/*
9795
* Maybe also accept a local address
9896
*/
9997

10098
if (type != INET_SERVER && (!NIL_P(arg->local.host) || !NIL_P(arg->local.serv))) {
10199
arg->local.res = rsock_addrinfo(arg->local.host, arg->local.serv,
102-
family, SOCK_STREAM, 0, 0);
100+
family, SOCK_STREAM, 0, timeout);
103101
}
104102

105103
VALUE io = Qnil;
@@ -630,14 +628,7 @@ init_fast_fallback_inetsock_internal(VALUE v)
630628
arg->getaddrinfo_shared = NULL;
631629

632630
int family = arg->families[0];
633-
unsigned int t;
634-
if (!NIL_P(open_timeout)) {
635-
t = rsock_value_timeout_to_msec(open_timeout);
636-
} else if (!NIL_P(resolv_timeout)) {
637-
t = rsock_value_timeout_to_msec(resolv_timeout);
638-
} else {
639-
t = 0;
640-
}
631+
VALUE t = NIL_P(open_timeout) ? resolv_timeout : open_timeout;
641632

642633
arg->remote.res = rsock_addrinfo(
643634
arg->remote.host,
@@ -1337,15 +1328,7 @@ rsock_init_inetsock(
13371328
* Maybe also accept a local address
13381329
*/
13391330
if (!NIL_P(local_host) || !NIL_P(local_serv)) {
1340-
unsigned int t;
1341-
if (!NIL_P(open_timeout)) {
1342-
t = rsock_value_timeout_to_msec(open_timeout);
1343-
} else if (!NIL_P(resolv_timeout)) {
1344-
t = rsock_value_timeout_to_msec(resolv_timeout);
1345-
} else {
1346-
t = 0;
1347-
}
1348-
1331+
VALUE t = NIL_P(open_timeout) ? resolv_timeout : open_timeout;
13491332
local_res = rsock_addrinfo(
13501333
local_host,
13511334
local_serv,
@@ -1609,7 +1592,7 @@ static VALUE
16091592
ip_s_getaddress(VALUE obj, VALUE host)
16101593
{
16111594
union_sockaddr addr;
1612-
struct rb_addrinfo *res = rsock_addrinfo(host, Qnil, AF_UNSPEC, SOCK_STREAM, 0, 0);
1595+
struct rb_addrinfo *res = rsock_addrinfo(host, Qnil, AF_UNSPEC, SOCK_STREAM, 0, Qnil);
16131596
socklen_t len = res->ai->ai_addrlen;
16141597

16151598
/* just take the first one */

‎ext/socket/raddrinfo.c‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ rb_freeaddrinfo(struct rb_addrinfo *ai)
293293
xfree(ai);
294294
}
295295

296-
unsigned int
296+
static int
297297
rsock_value_timeout_to_msec(VALUE timeout)
298298
{
299299
double seconds = NUM2DBL(timeout);
@@ -308,7 +308,7 @@ rsock_value_timeout_to_msec(VALUE timeout)
308308
#if GETADDRINFO_IMPL == 0
309309

310310
static int
311-
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai, unsigned int _timeout)
311+
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai, int _timeout)
312312
{
313313
return getaddrinfo(hostp, portp, hints, ai);
314314
}
@@ -346,7 +346,7 @@ fork_safe_getaddrinfo(void *arg)
346346
}
347347

348348
static int
349-
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai, unsigned int _timeout)
349+
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai, int _timeout)
350350
{
351351
struct getaddrinfo_arg arg;
352352
MEMZERO(&arg, struct getaddrinfo_arg, 1);
@@ -367,11 +367,11 @@ struct getaddrinfo_arg
367367
int err, gai_errno, refcount, done, cancelled, timedout;
368368
rb_nativethread_lock_t lock;
369369
rb_nativethread_cond_t cond;
370-
unsigned int timeout;
370+
int timeout;
371371
};
372372

373373
static struct getaddrinfo_arg *
374-
allocate_getaddrinfo_arg(const char *hostp, const char *portp, const struct addrinfo *hints, unsigned int timeout)
374+
allocate_getaddrinfo_arg(const char *hostp, const char *portp, const struct addrinfo *hints, int timeout)
375375
{
376376
size_t hostp_offset = sizeof(struct getaddrinfo_arg);
377377
size_t portp_offset = hostp_offset + (hostp ? strlen(hostp) + 1 : 0);
@@ -465,8 +465,11 @@ wait_getaddrinfo(void *ptr)
465465
struct getaddrinfo_arg *arg = (struct getaddrinfo_arg *)ptr;
466466
rb_nativethread_lock_lock(&arg->lock);
467467
while (!arg->done && !arg->cancelled) {
468-
unsigned long msec = arg->timeout;
469-
if (msec > 0) {
468+
long msec = arg->timeout;
469+
if (msec == 0) {
470+
arg->cancelled = 1;
471+
arg->timedout = 1;
472+
} else if (msec > 0) {
470473
rb_native_cond_timedwait(&arg->cond, &arg->lock, msec);
471474
if (!arg->done) {
472475
arg->cancelled = 1;
@@ -549,7 +552,7 @@ fork_safe_do_getaddrinfo(void *ptr)
549552
}
550553

551554
static int
552-
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai, unsigned int timeout)
555+
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai, int timeout)
553556
{
554557
int retry;
555558
struct getaddrinfo_arg *arg;
@@ -1021,7 +1024,7 @@ rb_scheduler_getaddrinfo(VALUE scheduler, VALUE host, const char *service,
10211024
}
10221025

10231026
struct rb_addrinfo*
1024-
rsock_getaddrinfo(VALUE host, VALUE port, struct addrinfo *hints, int socktype_hack, unsigned int timeout)
1027+
rsock_getaddrinfo(VALUE host, VALUE port, struct addrinfo *hints, int socktype_hack, VALUE timeout)
10251028
{
10261029
struct rb_addrinfo* res = NULL;
10271030
struct addrinfo *ai;
@@ -1056,7 +1059,8 @@ rsock_getaddrinfo(VALUE host, VALUE port, struct addrinfo *hints, int socktype_h
10561059
}
10571060

10581061
if (!resolved) {
1059-
error = rb_getaddrinfo(hostp, portp, hints, &ai, timeout);
1062+
int t = NIL_P(timeout) ? -1 : rsock_value_timeout_to_msec(timeout);
1063+
error = rb_getaddrinfo(hostp, portp, hints, &ai, t);
10601064
if (error == 0) {
10611065
res = (struct rb_addrinfo *)xmalloc(sizeof(struct rb_addrinfo));
10621066
res->allocated_by_malloc = 0;
@@ -1089,7 +1093,7 @@ rsock_fd_family(int fd)
10891093
}
10901094

10911095
struct rb_addrinfo*
1092-
rsock_addrinfo(VALUE host, VALUE port, int family, int socktype, int flags, unsigned int timeout)
1096+
rsock_addrinfo(VALUE host, VALUE port, int family, int socktype, int flags, VALUE timeout)
10931097
{
10941098
struct addrinfo hints;
10951099

@@ -1380,8 +1384,7 @@ call_getaddrinfo(VALUE node, VALUE service,
13801384
hints.ai_flags = NUM2INT(flags);
13811385
}
13821386

1383-
unsigned int t = NIL_P(timeout) ? 0 : rsock_value_timeout_to_msec(timeout);
1384-
res = rsock_getaddrinfo(node, service, &hints, socktype_hack, t);
1387+
res = rsock_getaddrinfo(node, service, &hints, socktype_hack, timeout);
13851388

13861389
if (res == NULL)
13871390
rb_raise(rb_eSocket, "host not found");

‎ext/socket/rubysocket.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,8 @@ void rb_freeaddrinfo(struct rb_addrinfo *ai);
327327
VALUE rsock_freeaddrinfo(VALUE arg);
328328
int rb_getnameinfo(const struct sockaddr *sa, socklen_t salen, char *host, size_t hostlen, char *serv, size_t servlen, int flags);
329329
int rsock_fd_family(int fd);
330-
struct rb_addrinfo *rsock_addrinfo(VALUE host, VALUE port, int family, int socktype, int flags, unsigned int timeout);
331-
struct rb_addrinfo *rsock_getaddrinfo(VALUE host, VALUE port, struct addrinfo *hints, int socktype_hack, unsigned int timeout);
330+
struct rb_addrinfo *rsock_addrinfo(VALUE host, VALUE port, int family, int socktype, int flags, VALUE timeout);
331+
struct rb_addrinfo *rsock_getaddrinfo(VALUE host, VALUE port, struct addrinfo *hints, int socktype_hack, VALUE timeout);
332332

333333
VALUE rsock_fd_socket_addrinfo(int fd, struct sockaddr *addr, socklen_t len);
334334
VALUE rsock_io_socket_addrinfo(VALUE io, struct sockaddr *addr, socklen_t len);
@@ -453,7 +453,6 @@ void free_fast_fallback_getaddrinfo_shared(struct fast_fallback_getaddrinfo_shar
453453
# endif
454454
#endif
455455

456-
unsigned int rsock_value_timeout_to_msec(VALUE);
457456
NORETURN(void rsock_raise_user_specified_timeout(struct addrinfo *ai, VALUE host, VALUE port));
458457

459458
void rsock_init_basicsocket(void);

‎ext/socket/socket.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -965,7 +965,7 @@ sock_s_gethostbyname(VALUE obj, VALUE host)
965965
{
966966
rb_warn("Socket.gethostbyname is deprecated; use Addrinfo.getaddrinfo instead.");
967967
struct rb_addrinfo *res =
968-
rsock_addrinfo(host, Qnil, AF_UNSPEC, SOCK_STREAM, AI_CANONNAME, 0);
968+
rsock_addrinfo(host, Qnil, AF_UNSPEC, SOCK_STREAM, AI_CANONNAME, Qnil);
969969
return rsock_make_hostent(host, res, sock_sockaddr);
970970
}
971971

@@ -1183,7 +1183,7 @@ sock_s_getaddrinfo(int argc, VALUE *argv, VALUE _)
11831183
norevlookup = rsock_do_not_reverse_lookup;
11841184
}
11851185

1186-
res = rsock_getaddrinfo(host, port, &hints, 0, 0);
1186+
res = rsock_getaddrinfo(host, port, &hints, 0, Qnil);
11871187

11881188
ret = make_addrinfo(res, norevlookup);
11891189
rb_freeaddrinfo(res);
@@ -1279,7 +1279,7 @@ sock_s_getnameinfo(int argc, VALUE *argv, VALUE _)
12791279
hints.ai_socktype = (fl & NI_DGRAM) ? SOCK_DGRAM : SOCK_STREAM;
12801280
/* af */
12811281
hints.ai_family = NIL_P(af) ? PF_UNSPEC : rsock_family_arg(af);
1282-
res = rsock_getaddrinfo(host, port, &hints, 0, 0);
1282+
res = rsock_getaddrinfo(host, port, &hints, 0, Qnil);
12831283
sap = res->ai->ai_addr;
12841284
salen = res->ai->ai_addrlen;
12851285
}
@@ -1335,7 +1335,7 @@ sock_s_getnameinfo(int argc, VALUE *argv, VALUE _)
13351335
static VALUE
13361336
sock_s_pack_sockaddr_in(VALUE self, VALUE port, VALUE host)
13371337
{
1338-
struct rb_addrinfo *res = rsock_addrinfo(host, port, AF_UNSPEC, 0, 0, 0);
1338+
struct rb_addrinfo *res = rsock_addrinfo(host, port, AF_UNSPEC, 0, 0, Qnil);
13391339
VALUE addr = rb_str_new((char*)res->ai->ai_addr, res->ai->ai_addrlen);
13401340

13411341
rb_freeaddrinfo(res);

‎ext/socket/tcpsocket.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ tcp_s_gethostbyname(VALUE obj, VALUE host)
113113
{
114114
rb_warn("TCPSocket.gethostbyname is deprecated; use Addrinfo.getaddrinfo instead.");
115115
struct rb_addrinfo *res =
116-
rsock_addrinfo(host, Qnil, AF_UNSPEC, SOCK_STREAM, AI_CANONNAME, 0);
116+
rsock_addrinfo(host, Qnil, AF_UNSPEC, SOCK_STREAM, AI_CANONNAME, Qnil);
117117
return rsock_make_hostent(host, res, tcp_sockaddr);
118118
}
119119

‎ext/socket/udpsocket.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ udp_connect(VALUE self, VALUE host, VALUE port)
8484
{
8585
struct udp_arg arg = {.io = self};
8686

87-
arg.res = rsock_addrinfo(host, port, rsock_fd_family(rb_io_descriptor(self)), SOCK_DGRAM, 0, 0);
87+
arg.res = rsock_addrinfo(host, port, rsock_fd_family(rb_io_descriptor(self)), SOCK_DGRAM, 0, Qnil);
8888

8989
int result = (int)rb_ensure(udp_connect_internal, (VALUE)&arg, rsock_freeaddrinfo, (VALUE)arg.res);
9090
if (!result) {
@@ -129,7 +129,7 @@ udp_bind(VALUE self, VALUE host, VALUE port)
129129
{
130130
struct udp_arg arg = {.io = self};
131131

132-
arg.res = rsock_addrinfo(host, port, rsock_fd_family(rb_io_descriptor(self)), SOCK_DGRAM, 0, 0);
132+
arg.res = rsock_addrinfo(host, port, rsock_fd_family(rb_io_descriptor(self)), SOCK_DGRAM, 0, Qnil);
133133

134134
VALUE result = rb_ensure(udp_bind_internal, (VALUE)&arg, rsock_freeaddrinfo, (VALUE)arg.res);
135135
if (!result) {
@@ -212,7 +212,7 @@ udp_send(int argc, VALUE *argv, VALUE sock)
212212
GetOpenFile(sock, arg.fptr);
213213
arg.sarg.fd = arg.fptr->fd;
214214
arg.sarg.flags = NUM2INT(flags);
215-
arg.res = rsock_addrinfo(host, port, rsock_fd_family(arg.fptr->fd), SOCK_DGRAM, 0, 0);
215+
arg.res = rsock_addrinfo(host, port, rsock_fd_family(arg.fptr->fd), SOCK_DGRAM, 0, Qnil);
216216
ret = rb_ensure(udp_send_internal, (VALUE)&arg,
217217
rsock_freeaddrinfo, (VALUE)arg.res);
218218
if (!ret) rsock_sys_fail_host_port("sendto(2)", host, port);

‎test/socket/test_socket.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1011,7 +1011,7 @@ def test_tcp_socket_all_hostname_resolution_failed
10111011
Addrinfo.define_singleton_method(:getaddrinfo) do |_, _, family, *_|
10121012
case family
10131013
when Socket::AF_INET6 then raise SocketError
1014-
when Socket::AF_INET then sleep(0.001); raise SocketError, "Last hostname resolution error"
1014+
when Socket::AF_INET then sleep(0.01); raise SocketError, "Last hostname resolution error"
10151015
end
10161016
end
10171017

0 commit comments

Comments
 (0)