--Boundary-00 cT+FsmeN+gR+I/ Content-Type: text/plain; charset tf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline > Which set of patches do you think should be committed? The former to > ruby_1_8 and the latter to ruby_1_8_6? The latter patches must be applied to 1.8.6, since for now 1.8.6 with the 'new' thread support is unusable for multi-threaded programs. The former patches may not be applied at all. According to mental, it breaks compatibility (well, in my POV and mental's, it breaks poorly written programs ...). If you want to apply these changes, tell me so that I rewrite the patches, the changes I made are quite ugly ... Moreover, you should add fastthread's test suite to ruby's own, and merge in it the tests I wrote for the particular issues fixed by these patches (test_thread.rb) For the sake of simplicity, I join all the files in this mail. I slightly modified the test for ConditionVariable#wait in exception context, so please consider only the files in this mail. Regards Sylvain Joyeux --Boundary-00 cT+FsmeN+gR+I/ Content-Type: text/x-diff; charset tf-8"; name hread-condvar_wait.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename hread-condvar_wait.diff" --- /home/sjoyeux/system/ruby-1.8.6/ext/thread/thread.c 2007-03-03 11:08:06.000000000 +0100 +++ thread.c 2007-03-14 17:39:31.000000000 +0100 @@ -391,7 +410,7 @@ rb_mutex_try_lock(VALUE self) * */ -static void +static VALUE lock_mutex(Mutex *mutex) { VALUE current; @@ -406,6 +425,7 @@ lock_mutex(Mutex *mutex) mutex->owner urrent; rb_thread_critical ; + return Qnil; } static VALUE @@ -624,18 +645,12 @@ static void wait_condvar(ConditionVariable *condvar, Mutex *mutex) { rb_thread_critical ; - if (!RTEST(mutex->owner)) { + if (rb_thread_current() ! utex->owner) { rb_thread_critical ; - return; + rb_raise(rb_eThreadError, "not owner of the synchronization mutex"); } - if (mutex->owner ! b_thread_current()) { - rb_thread_critical ; - rb_raise(rb_eThreadError, "Not owner"); - } - mutex->owner nil; - wait_list(&condvar->waiting); - - lock_mutex(mutex); + unlock_mutex_inner(mutex); + rb_ensure(wait_list, (VALUE)&condvar->waiting, lock_mutex, (VALUE)mutex); } static VALUE --Boundary-00 cT+FsmeN+gR+I/ Content-Type: text/x-diff; charset tf-8"; name hread-mutex-not_owner.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename hread-mutex-not_owner.diff" --- /home/sjoyeux/system/ruby-1.8.6/ext/thread/thread.c 2007-03-03 11:08:06.000000000 +0100 +++ thread.c 2007-03-14 15:36:58.000000000 +0100 @@ -429,8 +443,13 @@ unlock_mutex_inner(Mutex *mutex) VALUE waking; if (!RTEST(mutex->owner)) { - return Qundef; + rb_raise(rb_eThreadError, "not owner"); + } + + if (mutex->owner ! b_thread_current()) { + rb_raise(rb_eThreadError, "not owner"); } + mutex->owner nil; waking ake_one(&mutex->waiting); --Boundary-00 cT+FsmeN+gR+I/ Content-Type: text/x-diff; charset tf-8"; name hread-mutex-remove_one.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename hread-mutex-remove_one.diff" --- /home/sjoyeux/system/ruby-1.8.6/ext/thread/thread.c 2007-03-03 11:08:06.000000000 +0100 +++ thread.c 2007-03-14 17:12:38.000000000 +0100 @@ -165,12 +165,23 @@ remove_one(List *list, VALUE value) Entry **ref; Entry *entry; - for (ref list->entries, entry ist->entries; - entry ! ULL; - ref entry->next, entry ntry->next) { - if (entry->value value) { - *ref ntry->next; - recycle_entries(list, entry, entry); + entry ist->entries; + if (!entry) return; + if (entry->value value) + { + shift_list(list); + return; + } + + for (entry ist->entries; entry->next ! ULL; entry ntry->next) { + Entry* next_entry ntry->next; + if (next_entry->value value) { + entry->next ext_entry->next; + if (!entry->next) + list->last_entry ntry; + + --list->size; + recycle_entries(list, next_entry, next_entry); break; } } --Boundary-00 cT+FsmeN+gR+I/ Content-Type: text/x-diff; charset tf-8"; name hread-mutex-recursive_lock.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename hread-mutex-recursive_lock.diff" --- /home/sjoyeux/system/ruby-1.8.6/ext/thread/thread.c 2007-03-03 11:08:06.000000000 +0100 +++ thread.c 2007-03-14 15:45:24.000000000 +0100 @@ -399,6 +410,9 @@ lock_mutex(Mutex *mutex) rb_thread_critical ; while (RTEST(mutex->owner)) { + if (mutex->owner current) + rb_raise(rb_eThreadError, "recursive mutex lock"); + wait_list(&mutex->waiting); rb_thread_critical ; } --Boundary-00 cT+FsmeN+gR+I/ Content-Type: application/x-ruby; name est_thread.rb" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename est_thread.rb" require 'thread' require 'test/unit' class TC_Thread < Test::Unit::TestCase def setup Thread.abort_on_exception rue end def teardown Thread.abort_on_exception alse end def test_condvar mutex utex.new condvar onditionVariable.new result ] mutex.synchronize do t hread.new do mutex.synchronize do result << 1 condvar.signal end end result << 0 condvar.wait(mutex) result << 2 t.join end assert_equal([0, 1, 2], result) end def test_condvar_wait_not_owner mutex utex.new condvar onditionVariable.new assert_raises(ThreadError) { condvar.wait(mutex) } end def test_condvar_wait_exception_handling # Calling wait in the only thread running should raise a ThreadError of # 'stopping only thread' mutex utex.new condvar onditionVariable.new Thread.abort_on_exception alse locked alse thread hread.new do mutex.synchronize do begin condvar.wait(mutex) rescue Exception locked utex.locked? raise end end end while !thread.stop? sleep(0.1) end thread.raise Interrupt, "interrupt a dead condition variable" assert_raises(Interrupt) { thread.value } assert(locked) end end --Boundary-00 cT+FsmeN+gR+I/--