ruby-dev

>>> ということは危ないのは RUBY_VM_SET_INTERRUPT() がロストしたときに、タイムアウトなしの
>>> スリープをしていて、vm->running_thread にならないから、タイマースレッドに
>>> 起こして貰えないケースでしょうか？
>>
>>　 ちょっとその例が具体的にぱっとわからないのですが，th->interrupt_flag
>> を監視している，前のメールで出してもらった例は危険そうです．危険そう，と
>> 思うのだけど，本当に危険なのかは実は自身が無いのですが．
>
> じゃあパッチつくるから、パッチが汚くなかったらacceptしてくださいよ。危険の
> ありなしをあれこれ悩むよりも建設的です。きっと

しばらく考えたところ、書き込み側で atomic ops 使えば十分という気がしてきました。
それなら、readerは速度低下０なのでパフォーマンス問題がおこりようがないし。
Linuxでしかテストしてませんが、こんなもんでどうでしょうか？

Attachments (1)

interrupt_flag.patch (5.28 KB, text/x-diff)

commit 8e1fd07eab070215ec52bdde3cf3d77a1d00413e
Author: KOSAKI Motohiro <[email protected]>
Date:   Wed May 11 22:19:51 2011 +0900

    interrupt flag
    
    Signed-off-by: KOSAKI Motohiro <[email protected]>
---
 atomic.h  |   35 +++++++++++++++++++++++++++++++++++
 signal.c  |   36 +++++++++---------------------------
 thread.c  |    9 +++++----
 vm_core.h |    9 +++++----
 4 files changed, 54 insertions(+), 35 deletions(-)

diff --git a/atomic.h b/atomic.h
new file mode 100644
index 0000000..8de38a5
--- /dev/null
+++ b/atomic.h
@@ -0,0 +1,35 @@
+#ifndef RUBY_ATOMIC_H
+#define RUBY_ATOMIC_H
+
+#ifdef _WIN32
+typedef LONG rb_atomic_t;
+
+# define ATOMIC_SET(var, val) InterlockedExchange(&(var), (val))
+# define ATOMIC_INC(var) InterlockedIncrement(&(var))
+# define ATOMIC_DEC(var) InterlockedDecrement(&(var))
+# define ATOMIC_OR InterlockedOr(&(var), (val))
+# define ATOMIC_EXCHANGE(var, val) InterlockedExchange(&(var), (val))
+
+#elif defined HAVE_GCC_ATOMIC_BUILTINS
+/* @shyouhei hack to support atomic operations in case of gcc. Gcc
+ * has its own pseudo-insns to support them.  See info, or
+ * http://gcc.gnu.org/onlinedocs/gcc/Atomic-Builtins.html */
+
+typedef unsigned int rb_atomic_t; /* Anything OK */
+# define ATOMIC_SET(var, val)  __sync_lock_test_and_set(&(var), (val))
+# define ATOMIC_INC(var) __sync_fetch_and_add(&(var), 1)
+# define ATOMIC_DEC(var) __sync_fetch_and_sub(&(var), 1)
+# define ATOMIC_OR(var, val) __sync_or_and_fetch(&(var), (val))
+# define ATOMIC_EXCHANGE(var, val) __sync_lock_test_and_set(&(var), (val))
+
+#else
+typedef int rb_atomic_t;
+
+# define ATOMIC_SET(var, val) ((var) = (val))
+# define ATOMIC_INC(var) (++(var))
+# define ATOMIC_DEC(var) (--(var))
+# define ATOMIC_OR(var, val) ((var) |= (val))
+# define ATOMIC_EXCHANGE(var, val) ruby_atomic_exchange(&(var), val)
+#endif
+
+#endif /* RUBY_ATOMIC_H */
diff --git a/signal.c b/signal.c
index d53c564..1a1d1d5 100644
--- a/signal.c
+++ b/signal.c
@@ -16,33 +16,15 @@
 #include <signal.h>
 #include <stdio.h>
 #include <errno.h>
+#include "atomic.h"
 
-#ifdef _WIN32
-typedef LONG rb_atomic_t;
-
-# define ATOMIC_TEST(var) InterlockedExchange(&(var), 0)
-# define ATOMIC_SET(var, val) InterlockedExchange(&(var), (val))
-# define ATOMIC_INC(var) InterlockedIncrement(&(var))
-# define ATOMIC_DEC(var) InterlockedDecrement(&(var))
-
-#elif defined HAVE_GCC_ATOMIC_BUILTINS
-/* @shyouhei hack to support atomic operations in case of gcc. Gcc
- * has its own pseudo-insns to support them.  See info, or
- * http://gcc.gnu.org/onlinedocs/gcc/Atomic-Builtins.html */
-
-typedef unsigned int rb_atomic_t; /* Anything OK */
-# define ATOMIC_TEST(var) __sync_lock_test_and_set(&(var), 0)
-# define ATOMIC_SET(var, val)  __sync_lock_test_and_set(&(var), (val))
-# define ATOMIC_INC(var) __sync_fetch_and_add(&(var), 1)
-# define ATOMIC_DEC(var) __sync_fetch_and_sub(&(var), 1)
-
-#else
-typedef int rb_atomic_t;
-
-# define ATOMIC_TEST(var) ((var) ? ((var) = 0, 1) : 0)
-# define ATOMIC_SET(var, val) ((var) = (val))
-# define ATOMIC_INC(var) (++(var))
-# define ATOMIC_DEC(var) (--(var))
+#if !defined(_WIN32) && !defined(HAVE_GCC_ATOMIC_BUILTINS)
+rb_atomic_t ruby_atomic_exchange(rb_atomic_t *ptr, rb_atomic_t val)
+{
+    rb_atomic_t old = *ptr;
+    *ptr = val;
+    return ptr;
+}
 #endif
 
 #if defined(__BEOS__) || defined(__HAIKU__)
diff --git a/thread.c b/thread.c
index 182feb3..124b4e4 100644
--- a/thread.c
+++ b/thread.c
@@ -1289,19 +1289,20 @@ thread_s_pass(VALUE klass)
 static void
 rb_threadptr_execute_interrupts_rec(rb_thread_t *th, int sched_depth)
 {
+    rb_atomic_t interrupt;
+
     if (GET_VM()->main_thread == th) {
 	while (rb_signal_buff_size() && !th->exec_signal) native_thread_yield();
     }
 
     if (th->raised_flag) return;
 
-    while (th->interrupt_flag) {
+    while ((interrupt = ATOMIC_EXCHANGE(th->interrupt_flag, 0)) != 0) {
 	enum rb_thread_status status = th->status;
-	int timer_interrupt = th->interrupt_flag & 0x01;
-	int finalizer_interrupt = th->interrupt_flag & 0x04;
+	int timer_interrupt = interrupt & 0x01;
+	int finalizer_interrupt = interrupt & 0x04;
 
 	th->status = THREAD_RUNNABLE;
-	th->interrupt_flag = 0;
 
 	/* signal handling */
 	if (th->exec_signal) {
diff --git a/vm_core.h b/vm_core.h
index c5417d9..640b205 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -22,6 +22,7 @@
 #include "vm_opts.h"
 #include "id.h"
 #include "method.h"
+#include "atomic.h"
 
 #if   defined(_WIN32)
 #include "thread_win32.h"
@@ -430,7 +431,7 @@ typedef struct rb_thread_struct {
     VALUE thrown_errinfo;
     int exec_signal;
 
-    int interrupt_flag;
+    rb_atomic_t interrupt_flag;
     rb_thread_lock_t interrupt_lock;
     struct rb_unblock_callback unblock;
     VALUE locking_mutex;
@@ -687,9 +688,9 @@ extern rb_vm_t *ruby_current_vm;
 #error "unsupported thread model"
 #endif
 
-#define RUBY_VM_SET_INTERRUPT(th) ((th)->interrupt_flag |= 0x02)
-#define RUBY_VM_SET_TIMER_INTERRUPT(th) ((th)->interrupt_flag |= 0x01)
-#define RUBY_VM_SET_FINALIZER_INTERRUPT(th) ((th)->interrupt_flag |= 0x04)
+#define RUBY_VM_SET_TIMER_INTERRUPT(th)		ATOMIC_OR((th)->interrupt_flag, 0x01)
+#define RUBY_VM_SET_INTERRUPT(th)		ATOMIC_OR((th)->interrupt_flag, 0x02)
+#define RUBY_VM_SET_FINALIZER_INTERRUPT(th)	ATOMIC_OR((th)->interrupt_flag, 0x04)
 #define RUBY_VM_INTERRUPTED(th) ((th)->interrupt_flag & 0x02)
 
 int rb_signal_buff_size(void);

Thread

Prev Next

In This Thread

Prev Next