From: Nobuyoshi Nakada <nobu@...>
Date: 2008-05-29T16:14:21+09:00
Subject: [ruby-dev:34885] Re: Invalid read may occur when raising an exception to a thread that waits for mutex

なかだです。

At Wed, 28 May 2008 21:42:48 +0900,
Yusuke ENDOH wrote in [ruby-dev:34874]:
> unblock_function がクリアされないのは set_unblock_function 内で
> RUBY_VM_CHECK_INTS をしていて、そこから longjmp するせいです。
> この RUBY_VM_CHECK_INTS は必要なんでしょうか。

BLOCKING_REGIONに入るときにはcheckしても問題ないんじゃないでしょ
うか。問題が起きるのは抜けるときですよね。


Index: vm_core.h
===================================================================
--- vm_core.h	(revision 16676)
+++ vm_core.h	(working copy)
@@ -374,4 +374,9 @@ struct rb_vm_trap_tag {
 #define USE_VALUE_CACHE 0
 
+struct rb_unblock_callback {
+    rb_unblock_function_t *func;
+    void *arg;
+};
+
 typedef struct rb_thread_struct rb_thread_t;
 
@@ -419,7 +424,6 @@ struct rb_thread_struct
 
     int interrupt_flag;
-    rb_unblock_function_t *unblock_function;
-    void *unblock_function_arg;
     rb_thread_lock_t interrupt_lock;
+    struct rb_unblock_callback unblock;
 
     struct rb_vm_tag *tag;
Index: thread.c
===================================================================
--- thread.c	(revision 16676)
+++ thread.c	(working copy)
@@ -80,6 +80,7 @@ st_delete_wrap(st_table *table, st_data_
 #define THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION
 
-static void set_unblock_function(rb_thread_t *th, rb_unblock_function_t *func, void *ptr,
-				 rb_unblock_function_t **oldfunc, void **oldptr);
+static void set_unblock_function(rb_thread_t *th, rb_unblock_function_t *func, void *arg,
+				 struct rb_unblock_callback *old);
+static void reset_unblock_function(rb_thread_t *th, const struct rb_unblock_callback *old);
 
 #define GVL_UNLOCK_BEGIN() do { \
@@ -96,7 +97,6 @@ static void set_unblock_function(rb_thre
     rb_thread_t *__th = GET_THREAD(); \
     int __prev_status = __th->status; \
-    rb_unblock_function_t *__oldubf; \
-    void *__oldubfarg; \
-    set_unblock_function(__th, ubf, ubfarg, &__oldubf, &__oldubfarg); \
+    struct rb_unblock_callback __oldubf; \
+    set_unblock_function(__th, ubf, ubfarg, &__oldubf); \
     __th->status = THREAD_STOPPED; \
     thread_debug("enter blocking region (%p)\n", __th); \
@@ -107,5 +107,5 @@ static void set_unblock_function(rb_thre
     thread_debug("leave blocking region (%p)\n", __th); \
     remove_signal_thread_list(__th); \
-    set_unblock_function(__th, __oldubf, __oldubfarg, 0, 0); \
+    reset_unblock_function(__th, &__oldubf); \
     if (__th->status == THREAD_STOPPED) { \
 	__th->status = __prev_status; \
@@ -196,5 +196,5 @@ rb_thread_debug(const char *fmt, ...)
 static void
 set_unblock_function(rb_thread_t *th, rb_unblock_function_t *func, void *arg,
-		     rb_unblock_function_t **oldfunc, void **oldarg)
+		     struct rb_unblock_callback *old)
 {
   check_ints:
@@ -206,8 +206,7 @@ set_unblock_function(rb_thread_t *th, rb
     }
     else {
-	if (oldfunc) *oldfunc = th->unblock_function;
-	if (oldarg) *oldarg = th->unblock_function_arg;
-	th->unblock_function = func;
-	th->unblock_function_arg = arg;
+	if (old) *old = th->unblock;
+	th->unblock.func = func;
+	th->unblock.arg = arg;
     }
     native_mutex_unlock(&th->interrupt_lock);
@@ -215,10 +214,18 @@ set_unblock_function(rb_thread_t *th, rb
 
 static void
+reset_unblock_function(rb_thread_t *th, const struct rb_unblock_callback *old)
+{
+    native_mutex_lock(&th->interrupt_lock);
+    th->unblock = *old;
+    native_mutex_unlock(&th->interrupt_lock);
+}
+
+static void
 rb_thread_interrupt(rb_thread_t *th)
 {
     native_mutex_lock(&th->interrupt_lock);
     RUBY_VM_SET_INTERRUPT(th);
-    if (th->unblock_function) {
-	(th->unblock_function)(th->unblock_function_arg);
+    if (th->unblock.func) {
+	(th->unblock.func)(th->unblock.arg);
     }
     else {
Index: thread_pthread.c
===================================================================
--- thread_pthread.c	(revision 16676)
+++ thread_pthread.c	(working copy)
@@ -425,6 +425,6 @@ native_sleep(rb_thread_t *th, struct tim
     {
 	pthread_mutex_lock(&th->interrupt_lock);
-	th->unblock_function = ubf_pthread_cond_signal;
-	th->unblock_function_arg = th;
+	th->unblock.func = ubf_pthread_cond_signal;
+	th->unblock.arg = th;
 
 	if (RUBY_VM_INTERRUPTED(th)) {
@@ -452,6 +452,6 @@ native_sleep(rb_thread_t *th, struct tim
 	    }
 	}
-	th->unblock_function = 0;
-	th->unblock_function_arg = 0;
+	th->unblock.func = 0;
+	th->unblock.arg = 0;
 
 	pthread_mutex_unlock(&th->interrupt_lock);
Index: thread_win32.c
===================================================================
--- thread_win32.c	(revision 16676)
+++ thread_win32.c	(working copy)
@@ -221,6 +221,6 @@ native_sleep(rb_thread_t *th, struct tim
 
 	th->status = THREAD_STOPPED;
-	th->unblock_function = ubf_handle;
-	th->unblock_function_arg = th;
+	th->unblock.func = ubf_handle;
+	th->unblock.arg = th;
 
 	if (RUBY_VM_INTERRUPTED(th)) {
@@ -233,6 +233,6 @@ native_sleep(rb_thread_t *th, struct tim
 	}
 
-	th->unblock_function = 0;
-	th->unblock_function_arg = 0;
+	th->unblock.func = 0;
+	th->unblock.arg = 0;
 	th->status = status;
     }


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