From: Nobuyoshi Nakada <nobu@...>
Date: 2007-03-20T01:52:54+09:00
Subject: [ruby-dev:30645] Re: possible bug in rb_detach_process()

なかだです。

At Sat, 17 Mar 2007 18:36:07 +0900,
KUBO Takehiro wrote in [ruby-dev:30618]:
> detach_process_watcher() に渡される引数は、rb_detach_process のスレッ
> ドのスタック上の値になっています。rb_waitpid() が実行される前に
> rb_detach_process のスレッドが動いてスタック上の値が書き変わったら、
> rb_waitpid() に渡される pid はわけのわからない値になります。

たしかに。

> 正しくはこう？
> 
> static VALUE
> detach_process_watcher(void *arg)  <- int *pid_p を void *arg に
> {
>     rb_pid_t pid = (int)arg;       <- void* を rb_pid_t に戻す。
>     rb_pid_t cpid;
>     int status;
> 
>     for (;;) {
>         cpid = rb_waitpid(pid, &status, WNOHANG);  <- *pid_p を pid に
>         if (cpid != 0) return rb_last_status_get();
>         rb_thread_sleep(1);

1.9ではpollingは不要になったはずです。

この際なので、1.8でもwatchスレッドからexit statusをかえすように
しませんか。

* process.c (detach_process_watcher): should not pass the pointer
  to an auto variable to the thread to be created.  pointed and
  fix by KUBO Takehiro <kubo at jiubao.org>  [ruby-dev:30618]

* process.c (detach_process_watcher): now returns the exit status
  of the detached process.


Index: trunk/process.c
===================================================================
--- trunk/process.c	(revision 12102)
+++ trunk/process.c	(working copy)
@@ -861,14 +861,13 @@ proc_waitall(void)
 
 static VALUE
-detach_process_watcher(int *pid_p)
+detach_process_watcher(void *arg)
 {
-    rb_pid_t cpid;
+    rb_pid_t cpid, pid = (rb_pid_t)arg;
     int status;
 
-    for (;;) {
-	cpid = rb_waitpid(*pid_p, &status, WNOHANG);
-	if (cpid != 0) return rb_last_status_get();
-	rb_thread_sleep(1);
+    while ((cpid = rb_waitpid(pid, &status, 0)) == 0) {
+	/* wait while alive */
     }
+    return rb_last_status_get();
 }
 
@@ -876,5 +875,5 @@ VALUE
 rb_detach_process(rb_pid_t pid)
 {
-    return rb_thread_create(detach_process_watcher, (void*)&pid);
+    return rb_thread_create(detach_process_watcher, (void*)pid);
 }
 
@@ -892,6 +891,5 @@ rb_detach_process(rb_pid_t pid)
  *  process _pid_ when it terminates. Use <code>detach</code>
  *  only when you do not intent to explicitly wait for the child to
- *  terminate.  <code>detach</code> only checks the status
- *  periodically (currently once each second).
+ *  terminate.
  *
  *  The waiting thread returns the exit status of the detached process
Index: stable/process.c
===================================================================
--- stable/process.c	(revision 12102)
+++ stable/process.c	(working copy)
@@ -839,14 +839,13 @@ proc_waitall()
 
 static VALUE
-detach_process_watcher(pid_p)
-    int *pid_p;
+detach_process_watcher(arg)
+    void *arg;
 {
-    int cpid, status;
+    int pid = (int)arg, status;
 
-    for (;;) {
-	cpid = rb_waitpid(*pid_p, &status, WNOHANG);
-	if (cpid != 0) return Qnil;
+    while (rb_waitpid(pid, &status, WNOHANG) == 0) {
 	rb_thread_sleep(1);
     }
+    return rb_last_status;
 }
 
@@ -855,5 +854,5 @@ rb_detach_process(pid)
     int pid;
 {
-    return rb_thread_create(detach_process_watcher, (void*)&pid);
+    return rb_thread_create(detach_process_watcher, (void*)pid);
 }
 
@@ -874,4 +873,9 @@ rb_detach_process(pid)
  *  periodically (currently once each second).
  *
+ *  The waiting thread returns the exit status of the detached process
+ *  when it terminates, so you can use <code>Thread#join</code> to
+ *  know the result.  If specified _pid_ is not a valid child process
+ *  ID, the thread returns +nil+ immediately.
+ *
  *  In this first example, we don't reap the first child process, so
  *  it appears as a zombie in the process status display.


-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦