ruby-core

On Sun, 2007-02-11 at 07:00 +0900, Akinori MUSHA wrote:
> If you find any compatibility problems or unexpected behavior with the
> new implementation, please let me know.

The version of fastthread which was merged doesn't handle interrupted
waits correctly, which can result in an rb_bug().

Please see Ruby bug #8663:

http://rubyforge.org/tracker/index.php?func=3Ddetail&aid=3D8663&group_id=3D=
426&atid=3D1698

A patch to fix the issue is attached (also posted to the bug tracker).

-mental

Attachments (2)

fastthread-wait-fix.diff (3.05 KB, text/x-diff)

Index: ext/thread/thread.c
===================================================================
--- ext/thread/thread.c	(revision 11744)
+++ ext/thread/thread.c	(working copy)
@@ -132,6 +132,18 @@
     }
 }
 
+static void
+recycle_entries(List *list, Entry *first_entry, Entry *last_entry)
+{
+#ifdef USE_MEM_POOLS
+    last_entry->next = list->entry_pool;
+    list->entry_pool = first_entry;
+#else
+    last_entry->next = NULL;
+    free_entries(first_entry);
+#endif
+}
+
 static VALUE
 shift_list(List *list)
 {
@@ -149,26 +161,33 @@
     --list->size;
 
     value = entry->value;
-#ifdef USE_MEM_POOLS
-    entry->next = list->entry_pool;
-    list->entry_pool = entry;
-#else
-    free(entry);
-#endif
+    recycle_entries(list, entry, entry);
 
     return value;
 }
 
 static void
+remove_one(List *list, VALUE value)
+{
+    Entry **ref;
+    Entry *entry;
+    for (ref = &list->entries, entry = list->entries;
+              entry != NULL;
+              ref = &entry->next, entry = entry->next)
+    {
+        if (entry->value == value) {
+            *ref = entry->next;
+            recycle_entries(list, entry, entry);
+            break;
+        }
+    }
+}
+
+static void
 clear_list(List *list)
 {
     if (list->last_entry) {
-#ifdef USE_MEM_POOLS
-        list->last_entry->next = list->entry_pool;
-        list->entry_pool = list->entries;
-#else
-        free_entries(list->entries);
-#endif
+        recycle_entries(list, list->entries, list->last_entry);
         list->entries = NULL;
         list->last_entry = NULL;
         list->size = 0;
@@ -223,7 +242,31 @@
     return Qnil;
 }
 
+static VALUE
+wait_list_inner(List *list)
+{
+    push_list(list, rb_thread_current());
+    rb_thread_stop();
+    return Qnil;
+}
+
+static VALUE
+wait_list_cleanup(List *list)
+{
+    /* cleanup in case of spurious wakeups */
+    rb_thread_critical = 1;
+    remove_one(list, rb_thread_current());
+    rb_thread_critical = 0;
+    return Qnil;
+}
+
 static void
+wait_list(List *list)
+{
+    rb_ensure(wait_list_inner, (VALUE)list, wait_list_cleanup, (VALUE)list);
+}
+
+static void
 assert_no_survivors(List *waiting, const char *label, void *addr)
 {
     Entry *entry;
@@ -371,9 +414,7 @@
     rb_thread_critical = 1;
 
     while (RTEST(mutex->owner)) {
-        push_list(&mutex->waiting, current);
-        rb_thread_stop();
-
+        wait_list(&mutex->waiting);
         rb_thread_critical = 1;
     }
     mutex->owner = current; 
@@ -606,8 +647,7 @@
         rb_raise(rb_eThreadError, "Not owner");
     }
     mutex->owner = Qnil;
-    push_list(&condvar->waiting, rb_thread_current());
-    rb_thread_stop();
+    wait_list(&condvar->waiting);
 
     lock_mutex(mutex);
 }
@@ -626,8 +666,7 @@
 static VALUE
 legacy_wait(VALUE unused, legacy_wait_args *args)
 {
-    push_list(&args->condvar->waiting, rb_thread_current());
-    rb_thread_stop();
+    wait_list(&args->condvar->waiting);
     rb_funcall(args->mutex, rb_intern("lock"), 0);
     return Qnil;
 }

signature.asc (189 Bytes, application/pgp-signature)

Thread

Prev Next

In This Thread

Prev Next