From: Sylvain Joyeux Date: 2007-09-10T00:40:51+09:00 Subject: Re: Next 1.8.6 on Sept. 22 --Boundary_(ID_Xc2FQrwlO1Wh7L7E+baqZg) Content-type: text/plain; charset=iso-2022-jp Content-transfer-encoding: 7BIT Content-disposition: inline On Sunday 09 September 2007, Urabe Shyouhei wrote: > Hi all. > > We're planning to release 1.8.6 fixes on Sept. 22(Sat). If you find > something that should be included (but not yet) to it, please let us > know. The following patches are all patches that can be found in RubyForge and are related to acknowledged bugreports also on rubyforge. They may have an equivalent in ruby_1_8 though Best regards, Sylvain Joyeux commit cbc21281c133031305a0cbbe9b7f419dbe9d46e7 Author: Sylvain Joyeux Date: Sat Aug 18 09:28:36 2007 +0200 [ext/dl] fix free-function for DLPtrData objects: should not re-allocate This is related to [#11859] GC will SIGSEGV if the free function of a data object The conclusion of the discussion in this bug is that it is not allowed to allocate objects in free-functions. DL has therefore a bug in this regard. Shyouhei submitted the bug: [#11882] recursive invokation of garbage_collect() in dlptr_free() Which is fixed by the patch: [#13151] fix allocation of objects in dlptr_free commit 5656b1448110c4e79c25d9071eb46c29f737018e Author: Sylvain Joyeux Date: Sat Aug 18 09:24:50 2007 +0200 [thread] fix exception-safety in Mutex#lock This fixes [#11901] mutex_lock is not exception-safe The problem lies in the fact that mutex_lock has *two* ways to be released: either by mutex_unlock *or* because of an exception. wait_list ensures that the thread gets removed from the list in both cases. Note that it is completely harmless in the "normal" case, as remove_one silently ignores if the object is not present in the list. commit 0c5e8d7a481fab0451075fdd3202993e22abe74e Author: Sylvain Joyeux Date: Sat Aug 18 09:22:29 2007 +0200 [thread] mutex_relock is useless, remove it This is related to [#11507] SVN 1.8.6 has thread support broken spurious "not owner" errors during ConditionVariable#wait in the following context mutex.synchronize do condition_variable.wait(mutex) end commit efbe5a5fe5bb84852c12bf525892229ab8c46ea3 Author: Sylvain Joyeux Date: Sat Aug 18 09:06:38 2007 +0200 [gc] add the rb_bug mentioned in [bug#11859] --Boundary_(ID_Xc2FQrwlO1Wh7L7E+baqZg) Content-type: text/x-diff; charset=iso-2022-jp; name=0001-gc-add-the-rb_bug-mentioned-in-bug-11859.patch Content-transfer-encoding: 7BIT Content-disposition: attachment; filename=0001-gc-add-the-rb_bug-mentioned-in-bug-11859.patch From efbe5a5fe5bb84852c12bf525892229ab8c46ea3 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Sat, 18 Aug 2007 09:06:38 +0200 Subject: [PATCH] [gc] add the rb_bug mentioned in [bug#11859] --- gc.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/gc.c b/gc.c index 9ac3bfb..e7e7804 100644 --- a/gc.c +++ b/gc.c @@ -378,6 +378,10 @@ rb_newobj() { VALUE obj; + if (during_gc) + rb_bug("object allocation during garbage collection phase"); + + if (!freelist) garbage_collect(); obj = (VALUE)freelist; -- 1.5.3.1 --Boundary_(ID_Xc2FQrwlO1Wh7L7E+baqZg) Content-type: text/x-diff; charset=iso-2022-jp; name=0003-thread-fix-exception-safety-in-Mutex-lock.patch Content-transfer-encoding: 7BIT Content-disposition: attachment; filename=0003-thread-fix-exception-safety-in-Mutex-lock.patch From 5656b1448110c4e79c25d9071eb46c29f737018e Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Sat, 18 Aug 2007 09:24:50 +0200 Subject: [PATCH] [thread] fix exception-safety in Mutex#lock This fixes [#11901] mutex_lock is not exception-safe The problem lies in the fact that mutex_lock has *two* ways to be released: either by mutex_unlock *or* because of an exception. wait_list ensures that the thread gets removed from the list in both cases. Note that it is completely harmless in the "normal" case, as remove_one silently ignores if the object is not present in the list. --- ext/thread/thread.c | 3 +-- test/thread/test_thread.rb | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/ext/thread/thread.c b/ext/thread/thread.c index d0894e8..f1d7ef2 100644 --- a/ext/thread/thread.c +++ b/ext/thread/thread.c @@ -419,9 +419,8 @@ lock_mutex(Mutex *mutex) mutex->owner = current; } else { - push_list(&mutex->waiting, current); do { - rb_thread_stop(); + wait_list(&mutex->waiting); rb_thread_critical = 1; if (!MUTEX_LOCKED_P(mutex)) { mutex->owner = current; diff --git a/test/thread/test_thread.rb b/test/thread/test_thread.rb index abdcebf..f5a0f9c 100644 --- a/test/thread/test_thread.rb +++ b/test/thread/test_thread.rb @@ -63,5 +63,43 @@ class TC_Thread < Test::Unit::TestCase assert_raises(Interrupt) { thread.value } assert(locked) end + + # This test checks that a thread in Mutex#lock which is raised is + # completely removed from the wait_list of the mutex + def test_mutex_exception_handling + m = Mutex.new + m.lock + + sleeping = false + t = Thread.new do + begin + m.lock + rescue + end + + sleeping = true + # Keep that thread alive: if the thread returns, the test method + # won't be able to check that +m+ has not been taken (dead mutex + # owners are ignored) + sleep + end + + # Wait for t to wait for the mutex and raise it + while true + sleep 0.1 + break if t.stop? + end + t.raise ArgumentError + assert(t.alive? || sleeping) + + # Wait for +t+ to reach the sleep + while true + sleep 0.1 + break if t.stop? + end + + # Now unlock. The mutex should be free, so Mutex#unlock should return nil + assert(! m.unlock) + end end -- 1.5.3.1 --Boundary_(ID_Xc2FQrwlO1Wh7L7E+baqZg) Content-type: text/x-diff; charset=iso-2022-jp; name=0002-thread-mutex_relock-is-useless-remove-it.patch Content-transfer-encoding: 7BIT Content-disposition: attachment; filename=0002-thread-mutex_relock-is-useless-remove-it.patch From 0c5e8d7a481fab0451075fdd3202993e22abe74e Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Sat, 18 Aug 2007 09:22:29 +0200 Subject: [PATCH] [thread] mutex_relock is useless, remove it This is related to [#11507] SVN 1.8.6 has thread support broken spurious "not owner" errors during ConditionVariable#wait in the following context mutex.synchronize do condition_variable.wait(mutex) end --- ext/thread/thread.c | 25 +++---------------------- 1 files changed, 3 insertions(+), 22 deletions(-) diff --git a/ext/thread/thread.c b/ext/thread/thread.c index 43dfc34..d0894e8 100644 --- a/ext/thread/thread.c +++ b/ext/thread/thread.c @@ -12,9 +12,6 @@ #include #include #include -#include - -enum rb_thread_status rb_thread_status _((VALUE)); static VALUE rb_cMutex; static VALUE rb_cConditionVariable; @@ -262,10 +259,10 @@ wait_list_cleanup(List *list) return Qnil; } -static void +static VALUE wait_list(List *list) { - rb_ensure(wait_list_inner, (VALUE)list, wait_list_cleanup, (VALUE)list); + return rb_ensure(wait_list_inner, (VALUE)list, wait_list_cleanup, (VALUE)list); } static void @@ -446,22 +443,6 @@ rb_mutex_lock(VALUE self) return self; } -static VALUE -relock_mutex(Mutex *mutex) -{ - VALUE current = rb_thread_current(); - - switch (rb_thread_status(current)) { - case THREAD_RUNNABLE: - case THREAD_STOPPED: - lock_mutex(mutex); - break; - default: - break; - } - return Qundef; -} - /* * Document-method: unlock * @@ -675,7 +656,7 @@ wait_condvar(ConditionVariable *condvar, Mutex *mutex) if (RTEST(waking)) { wake_thread(waking); } - rb_ensure(wait_list, (VALUE)&condvar->waiting, relock_mutex, (VALUE)mutex); + rb_ensure(wait_list, (VALUE)&condvar->waiting, lock_mutex, (VALUE)mutex); } static VALUE -- 1.5.3.1 --Boundary_(ID_Xc2FQrwlO1Wh7L7E+baqZg) Content-type: text/x-diff; charset=iso-2022-jp; name=0004-ext-dl-fix-free-function-for-DLPtrData-objects-sh.patch Content-transfer-encoding: 7BIT Content-disposition: attachment; filename=0004-ext-dl-fix-free-function-for-DLPtrData-objects-sh.patch From cbc21281c133031305a0cbbe9b7f419dbe9d46e7 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Sat, 18 Aug 2007 09:28:36 +0200 Subject: [PATCH] [ext/dl] fix free-function for DLPtrData objects: should not re-allocate This is related to [#11859] GC will SIGSEGV if the free function of a data object The conclusion of the discussion in this bug is that it is not allowed to allocate objects in free-functions. DL has therefore a bug in this regard. Shyouhei submitted the bug: [#11882] recursive invokation of garbage_collect() in dlptr_free() Which is fixed by the patch: [#13151] fix allocation of objects in dlptr_free --- ext/dl/ptr.c | 73 +++++++++++++++++++++++++++++++++++++++++++++------------ 1 files changed, 57 insertions(+), 16 deletions(-) diff --git a/ext/dl/ptr.c b/ext/dl/ptr.c index 5112151..bbcb82e 100644 --- a/ext/dl/ptr.c +++ b/ext/dl/ptr.c @@ -6,10 +6,46 @@ #include #include /* for ruby version code */ #include "dl.h" +#include "st.h" VALUE rb_cDLPtrData; VALUE rb_mDLMemorySpace; -static VALUE DLMemoryTable; + +/* For those who are wondering, st_data_t is always the size of an void* + * (see the first lines of st.h + */ +int memory_table_compare(a, b) + void* a, *b; +{ + return (a == b); +} +int memory_table_hash(a) + void* a; +{ + /* Use the low-order bits as hash value, as they are the most likely to + * change */ + return (int)a; +} + +static struct st_hash_type memory_table_type = { + memory_table_compare, + memory_table_hash +}; +static st_table* DLMemoryTable; + +static int +memory_table_mark_i(void* key, VALUE value, void* arg) +{ + rb_gc_mark(value); + return ST_CONTINUE; +} +static void memory_table_mark(VALUE table) { + st_foreach(DLMemoryTable, memory_table_mark_i, 0); +} + +static void memory_table_free(VALUE table) { + //st_free_table(DLMemoryTable); +} #ifndef T_SYMBOL # define T_SYMBOL T_FIXNUM @@ -26,8 +62,7 @@ rb_hash_delete(VALUE hash, VALUE key) static void rb_dlmem_delete(void *ptr) { - rb_secure(4); - rb_hash_delete(DLMemoryTable, DLLONG2NUM(ptr)); + st_delete(DLMemoryTable, (st_data_t*)&ptr, 0); } static void @@ -37,7 +72,7 @@ rb_dlmem_aset(void *ptr, VALUE obj) rb_dlmem_delete(ptr); } else{ - rb_hash_aset(DLMemoryTable, DLLONG2NUM(ptr), DLLONG2NUM(obj)); + st_insert(DLMemoryTable, (st_data_t)ptr, obj); } } @@ -46,8 +81,12 @@ rb_dlmem_aref(void *ptr) { VALUE val; - val = rb_hash_aref(DLMemoryTable, DLLONG2NUM(ptr)); - return val == Qnil ? Qnil : (VALUE)DLNUM2LONG(val); + if (!st_lookup(DLMemoryTable, (st_data_t)ptr, &val)) { + return Qnil; + } + if (val == Qundef) + return Qnil; + return val; } void @@ -1010,26 +1049,26 @@ rb_dlptr_size(int argc, VALUE argv[], VALUE self) } } -static VALUE -dlmem_each_i(VALUE assoc, void *data) +static int +dlmem_each_i(void* key, VALUE value, void* arg) { - VALUE key, val; - key = rb_ary_entry(assoc, 0); - val = rb_ary_entry(assoc, 1); - rb_yield(rb_assoc_new(key,(VALUE)DLNUM2LONG(val))); - return Qnil; + VALUE vkey = DLLONG2NUM(key); + rb_yield(rb_assoc_new(vkey, value)); + return ST_CONTINUE; } VALUE rb_dlmem_each(VALUE self) { - rb_iterate(rb_each, DLMemoryTable, dlmem_each_i, 0); + st_foreach(DLMemoryTable, dlmem_each_i, 0); return Qnil; } void Init_dlptr() { + VALUE rb_DLMemoryTable; + rb_cDLPtrData = rb_define_class_under(rb_mDL, "PtrData", rb_cObject); rb_define_alloc_func(rb_cDLPtrData, rb_dlptr_s_allocate); rb_define_singleton_method(rb_cDLPtrData, "malloc", rb_dlptr_s_malloc, -1); @@ -1062,7 +1101,9 @@ Init_dlptr() rb_define_method(rb_cDLPtrData, "size=", rb_dlptr_size, -1); rb_mDLMemorySpace = rb_define_module_under(rb_mDL, "MemorySpace"); - DLMemoryTable = rb_hash_new(); - rb_define_const(rb_mDLMemorySpace, "MemoryTable", DLMemoryTable); + DLMemoryTable = st_init_table(&memory_table_type); + rb_DLMemoryTable = Data_Wrap_Struct(rb_cObject, memory_table_mark, memory_table_free, DLMemoryTable); + rb_define_const(rb_mDLMemorySpace, "MemoryTable", rb_DLMemoryTable); rb_define_module_function(rb_mDLMemorySpace, "each", rb_dlmem_each, 0); } + -- 1.5.3.1 --Boundary_(ID_Xc2FQrwlO1Wh7L7E+baqZg)--