Bug #257

Thread#kill cannot break BLOCKING_REGION() on windows

Added by Usaku NAKAMURA over 6 years ago. Updated almost 4 years ago.

[ruby-dev:35446]
Status:Closed
Priority:Normal
Assignee:Usaku NAKAMURA
ruby -v: Backport:

Description

=begin
こんにちは、なかむら(う)です。

Windows(除cygwin)では、外部からBLOCKING_REGION()を中断させる
ことができないので、あるスレッドがBLOCKING_REGION()に入ってい
る場合、そのスレッドをThread#killで終了されることができません。

例:
require "socket"
s = TCPServer.new(0)
t = Thread.new{s.accept}
sleep 1
t.kill
t.join # <= acceptがBLOCKING_REGION()内で処理されるため、
# ここで永久に待ち状態となる

# 現時点で対策が思いついていないのですが、忘れないように記録
# に残します。

それでは。
--
U.Nakamura usa@garbagecollect.jp
=end

History

#1 Updated by Usaku NAKAMURA over 6 years ago

  • Assignee set to Usaku NAKAMURA

=begin

=end

#2 Updated by Usaku NAKAMURA over 6 years ago

  • Priority changed from Low to Normal

=begin

=end

#3 Updated by Usaku NAKAMURA over 6 years ago

=begin
こんにちは、なかむら(う)です。

In message " [Bug:trunk] Thread#kill cannot break BLOCKING_REGION() on windows"
on Jul.12,2008 01:56:54, usa@garbagecollect.jp wrote:
| Windows(除cygwin)では、外部からBLOCKING_REGION()を中断させる
| ことができないので、あるスレッドがBLOCKING_REGION()に入ってい
| る場合、そのスレッドをThread#killで終了されることができません。

この問題は解決してないんですが、

| 例:
| require "socket"
| s = TCPServer.new(0)
| t = Thread.new{s.accept}
| sleep 1
| t.kill
| t.join # <= acceptがBLOCKING_REGION()内で処理されるため、
| # ここで永久に待ち状態となる

この例に限定すると、以下のパッチで解決します。
# 例がacceptなのでacceptだけ対処しています。

思うに、socket.soからrb_thread_blocking_region()を呼び出すよ
り、このパッチのように、io.cでやってるのと同様、ブロックしう
る関数の呼び出しの前に読み書き可能のチェックを行うべきではな
いでしょうか。

Index: ext/socket/socket.c
===================================================================
--- ext/socket/socket.c (revision 18034)
+++ ext/socket/socket.c (working copy)
@@ -1525,32 +1525,16 @@ s_accept_nonblock(VALUE klass, rb_io_t *
return init_sock(rb_obj_alloc(klass), fd2);
}

-struct accept_arg {
- int fd;
- struct sockaddr *sockaddr;
- socklen_t *len;
-};
-
-static VALUE
-accept_blocking(void *data)
-{
- struct accept_arg *arg = data;
- return (VALUE)accept(arg->fd, arg->sockaddr, arg->len);
-}
-
static VALUE
s_accept(VALUE klass, int fd, struct sockaddr *sockaddr, socklen_t *len)
{
int fd2;
int retry = 0;
- struct accept_arg arg;

  rb_secure(3);
  • arg.fd = fd;
  • arg.sockaddr = sockaddr;
  • arg.len = len; retry:
  • fd2 = (int)BLOCKING_REGION(accept_blocking, &arg);
  • rb_thread_wait_fd(fd);
  • fd2 = accept(fd, sockaddr, len);
    if (fd2 < 0) {
    switch (errno) {
    case EMFILE:

    以下は、以上の話と直接関係はないなかださん宛の私信:

    #

    だいぶ前にちらっと話したことがあるように、defファイルで言う

    select@20=_rb_w32_select@20

    みたいな奴が拡張ライブラリから正しく参照されず、直接ws2_32.dll

    向きになってしまうことがあるようです。

    msvcrt-ruby190.dllが更新されずにsocket.soだけ更新されるよう

    なパターンで再現したのですが、理由はさっぱりわかりません...

    なんか心当たりはありますか?

    それでは。

    U.Nakamura usa@garbagecollect.jp

=end

#4 Updated by Usaku NAKAMURA over 6 years ago

=begin
こんにちは、なかむら(う)です。

In message " Re: [Bug:trunk] Thread#kill cannot break BLOCKING_REGION() on windows"
on Jul.12,2008 04:32:40, usa@garbagecollect.jp wrote:

思うに、socket.soからrb_thread_blocking_region()を呼び出すよ
り、このパッチのように、io.cでやってるのと同様、ブロックしう
る関数の呼び出しの前に読み書き可能のチェックを行うべきではな
いでしょうか。

いろいろ考えるに、sendやrecvはいずれにせよブロックする可能性
があるのでrb_thread_blocking_region()でくくっておく必要はある
のではないかと考え直しました。
それはそれとして事前に読み書き可能のチェックを行うのには意味
はあると思うので、そういった点を全体に適用したパッチを作成し
ました。

commitしちゃいたいのですがどうでしょうか?

Index: ext/socket/socket.c
===================================================================
--- ext/socket/socket.c (revision 18067)
+++ ext/socket/socket.c (working copy)
@@ -551,7 +551,8 @@ bsock_send(int argc, VALUE *argv, VALUE
GetOpenFile(sock, fptr);
arg.fd = fptr->fd;
arg.flags = NUM2INT(flags);
- while ((n = (int)BLOCKING_REGION(func, &arg)) < 0) {
+ while (rb_thread_fd_writable(arg.fd),
+ (n = (int)BLOCKING_REGION(func, &arg)) < 0) {
if (rb_io_wait_writable(arg.fd)) {
continue;
}
@@ -640,6 +641,7 @@ s_recvfrom(VALUE sock, int argc, VALUE *
RBASIC(str)->klass = 0;

  while (rb_io_check_closed(fptr),
  • rb_thread_wait_fd(arg.fd),
    

    (slen = BLOCKING_REGION(recvfrom_blocking, &arg)) < 0) {
    if (RBASIC(str)->klass || RSTRING_LEN(str) != buflen) {
    rb_raise(rb_eRuntimeError, "buffer string modified");
    @@ -1140,19 +1142,17 @@ struct connect_arg {
    socklen_t len;
    };

    -static VALUE
    -connect_blocking(void *data)
    +static int
    +connect0(struct connect_arg *arg)
    {

  • struct connect_arg *arg = data;

  • return (VALUE)connect(arg->fd, arg->sockaddr, arg->len);

  • return connect(arg->fd, arg->sockaddr, arg->len);
    }

    #if defined(SOCKS) && !defined(SOCKS5)
    -static VALUE
    -socks_connect_blocking(void *data)
    +static int
    +socks_connect0(struct connect_arg *arg)
    {

  • struct connect_arg *arg = data;

  • return (VALUE)Rconnect(arg->fd, arg->sockaddr, arg->len);

  • return Rconnect(arg->fd, arg->sockaddr, arg->len);
    }
    #endif

    @@ -1160,7 +1160,7 @@ static int
    ruby_connect(int fd, const struct sockaddr *sockaddr, int len, int socks)
    {
    int status;

  • rb_blocking_function_t *func = connect_blocking;

  • int (*func)(struct connect_arg *) = connect0;
    struct connect_arg arg;
    #if WAIT_IN_PROGRESS > 0
    int wait_in_progress = -1;
    @@ -1172,10 +1172,11 @@ ruby_connect(int fd, const struct sockad
    arg.sockaddr = sockaddr;
    arg.len = len;
    #if defined(SOCKS) && !defined(SOCKS5)

  • if (socks) func = socks_connect_blocking;

  • if (socks) func = socks_connect0;
    #endif
    for (;;) {

  • status = (int)BLOCKING_REGION(func, &arg);

  • rb_thread_fd_writable(fd);

  • status = func(&arg);
    if (status < 0) {
    switch (errno) {
    case EAGAIN:
    @@ -1525,32 +1526,16 @@ s_accept_nonblock(VALUE klass, rb_io_t *
    return init_sock(rb_obj_alloc(klass), fd2);
    }

    -struct accept_arg {

  • int fd;

  • struct sockaddr *sockaddr;

  • socklen_t *len;

    -};

    -static VALUE
    -accept_blocking(void *data)
    -{

  • struct accept_arg *arg = data;

  • return (VALUE)accept(arg->fd, arg->sockaddr, arg->len);

    -}

    static VALUE
    s_accept(VALUE klass, int fd, struct sockaddr *sockaddr, socklen_t *len)
    {
    int fd2;
    int retry = 0;

  • struct accept_arg arg;

    rb_secure(3);

  • arg.fd = fd;

  • arg.sockaddr = sockaddr;

  • arg.len = len;
    retry:

  • fd2 = (int)BLOCKING_REGION(accept_blocking, &arg);

  • rb_thread_wait_fd(fd);

  • fd2 = accept(fd, sockaddr, len);
    if (fd2 < 0) {
    switch (errno) {
    case EMFILE:
    @@ -1852,6 +1837,7 @@ udp_send(int argc, VALUE *argv, VALUE so
    retry:
    arg.to = res->ai_addr;
    arg.tolen = res->ai_addrlen;

  • rb_thread_fd_writable(arg.fd);
    n = (int)BLOCKING_REGION(sendto_blocking, &arg);
    if (n >= 0) {
    freeaddrinfo(res0);
    @@ -2036,6 +2022,7 @@ unix_send_io(VALUE sock, VALUE val)
    #endif

    arg.fd = fptr->fd;

  • rb_thread_fd_writable(arg.fd);
    if ((int)BLOCKING_REGION(sendmsg_blocking, &arg) == -1)
    rb_sys_fail("sendmsg(2)");

    @@ -2102,6 +2089,7 @@ unix_recv_io(int argc, VALUE *argv, VALU
    #endif

    arg.fd = fptr->fd;

  • rb_thread_wait_fd(arg.fd);
    if ((int)BLOCKING_REGION(recvmsg_blocking, &arg) == -1)
    rb_sys_fail("recvmsg(2)");

    それでは。

    U.Nakamura usa@garbagecollect.jp

=end

#5 Updated by Yukihiro Matsumoto over 6 years ago

=begin
まつもと ゆきひろです

In message "Re: Re: [Bug:trunk] Thread#kill cannot break BLOCKING_REGION() on windows"
on Tue, 15 Jul 2008 17:51:36 +0900, "U.Nakamura" usa@garbagecollect.jp writes:

|いろいろ考えるに、sendやrecvはいずれにせよブロックする可能性
|があるのでrb_thread_blocking_region()でくくっておく必要はある
|のではないかと考え直しました。
|それはそれとして事前に読み書き可能のチェックを行うのには意味
|はあると思うので、そういった点を全体に適用したパッチを作成し
|ました。
|
|commitしちゃいたいのですがどうでしょうか?

どうぞ。ダメだったらリバートするということで。

=end

#6 Updated by Nobuyoshi Nakada over 6 years ago

  • Status changed from Open to Closed

=begin

=end

Also available in: Atom PDF