ruby-changes:40612
From: ko1 <ko1@a...>
Date: Sat, 21 Nov 2015 09:32:16 +0900 (JST)
Subject: [ruby-changes:40612] ko1:r52691 (trunk): * thread_sync.c: reduce the specification of Queue#close.
ko1 2015-11-21 09:32:09 +0900 (Sat, 21 Nov 2015) New Revision: 52691 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=52691 Log: * thread_sync.c: reduce the specification of Queue#close. * Queue#close accepts no arguments. * deq'ing on closed queue returns nil, always. [Feature #10600] * test/thread/test_queue.rb: catch up this fix. Modified files: trunk/ChangeLog trunk/test/thread/test_queue.rb trunk/thread_sync.c Index: thread_sync.c =================================================================== --- thread_sync.c (revision 52690) +++ thread_sync.c (revision 52691) @@ -523,7 +523,6 @@ enum { https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L523 }; #define QUEUE_CLOSED FL_USER5 -#define QUEUE_CLOSE_EXCEPTION FL_USER6 #define GET_QUEUE_QUE(q) get_array((q), QUEUE_QUE) #define GET_QUEUE_WAITERS(q) get_array((q), QUEUE_WAITERS) @@ -607,26 +606,15 @@ static VALUE https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L606 queue_closed_result(VALUE self) { assert(queue_length(self) == 0); - - if (FL_TEST(self, QUEUE_CLOSE_EXCEPTION)) { - raise_closed_queue_error(self); - } return Qnil; } static VALUE -queue_do_close(VALUE self, int argc, VALUE *argv, int is_szq) +queue_do_close(VALUE self, int is_szq) { - VALUE exception = Qfalse; - if (!queue_closed_p(self)) { - rb_scan_args(argc, argv, "01", &exception); FL_SET(self, QUEUE_CLOSED); - if (RTEST(exception)) { - FL_SET(self, QUEUE_CLOSE_EXCEPTION); - } - if (queue_num_waiting(self) > 0) { VALUE waiters = GET_QUEUE_WAITERS(self); wakeup_all_threads(waiters); @@ -697,7 +685,7 @@ queue_do_push(VALUE self, VALUE obj) https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L685 /* * Document-method: Queue#close * call-seq: - * close(exception=false) + * close * * Closes the queue. A closed queue cannot be re-opened. * @@ -707,15 +695,11 @@ queue_do_push(VALUE self, VALUE obj) https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L695 * * - +close+ will be ignored. * - * - calling enq/push/<< will raise ClosedQueueError('queue closed') + * - calling enq/push/<< will return nil. * * - when +empty?+ is false, calling deq/pop/shift will return an object * from the queue as usual. * - * - when +empty?+ is true, deq(non_block=false) will not suspend and - * will either return nil. If +exception+ parameter is true, raise ClosedQueueError error. - * deq(non_block=true) will ignore the parameter and raise a ThreadError('queue empty'). - * * ClosedQueueError is inherited from StopIteration, so that you can break loop block. * * Example: @@ -726,21 +710,13 @@ queue_do_push(VALUE self, VALUE obj) https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L710 * # ... * end * } - * q.close # equals to q.close(false) - * - * q = Queue.new - * Thread.new{ - * loop{ - * e = q.deq; ... # break with ClosedQueueError - * } - * } - * q.close(true) + * q.close */ static VALUE -rb_queue_close(int argc, VALUE *argv, VALUE self) +rb_queue_close(VALUE self) { - return queue_do_close(self, argc, argv, FALSE); + return queue_do_close(self, FALSE); } /* @@ -951,9 +927,9 @@ rb_szqueue_initialize(VALUE self, VALUE https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L927 * raising ClosedQueueError('queue closed'). */ static VALUE -rb_szqueue_close(int argc, VALUE *argv, VALUE self) +rb_szqueue_close(VALUE self) { - return queue_do_close(self, argc, argv, TRUE); + return queue_do_close(self, TRUE); } /* @@ -1273,7 +1249,7 @@ Init_thread_sync(void) https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L1249 rb_define_method(rb_cQueue, "initialize", rb_queue_initialize, 0); rb_undef_method(rb_cQueue, "initialize_copy"); rb_define_method(rb_cQueue, "marshal_dump", undumpable, 0); - rb_define_method(rb_cQueue, "close", rb_queue_close, -1); + rb_define_method(rb_cQueue, "close", rb_queue_close, 0); rb_define_method(rb_cQueue, "closed?", rb_queue_closed_p, 0); rb_define_method(rb_cQueue, "push", rb_queue_push, 1); rb_define_method(rb_cQueue, "pop", rb_queue_pop, -1); @@ -1294,7 +1270,7 @@ Init_thread_sync(void) https://github.com/ruby/ruby/blob/trunk/thread_sync.c#L1270 "que", "waiters", "queue_waiters", "size", NULL); rb_define_method(rb_cSizedQueue, "initialize", rb_szqueue_initialize, 1); - rb_define_method(rb_cSizedQueue, "close", rb_szqueue_close, -1); + rb_define_method(rb_cSizedQueue, "close", rb_szqueue_close, 0); rb_define_method(rb_cSizedQueue, "max", rb_szqueue_max_get, 0); rb_define_method(rb_cSizedQueue, "max=", rb_szqueue_max_set, 1); rb_define_method(rb_cSizedQueue, "push", rb_szqueue_push, -1); Index: ChangeLog =================================================================== --- ChangeLog (revision 52690) +++ ChangeLog (revision 52691) @@ -1,3 +1,12 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Sat Nov 21 09:18:10 2015 Koichi Sasada <ko1@a...> + + * thread_sync.c: reduce the specification of Queue#close. + * Queue#close accepts no arguments. + * deq'ing on closed queue returns nil, always. + [Feature #10600] + + * test/thread/test_queue.rb: catch up this fix. + Sat Nov 21 08:44:21 2015 Koichi Sasada <ko1@a...> * compile.c (iseq_compile_each): add debug information to NODE_STR Index: test/thread/test_queue.rb =================================================================== --- test/thread/test_queue.rb (revision 52690) +++ test/thread/test_queue.rb (revision 52691) @@ -289,6 +289,7 @@ class TestQueue < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/thread/test_queue.rb#L289 assert_raise_with_message(ClosedQueueError, /closed/){q << :nothing} assert_equal q.pop, :something assert_nil q.pop + assert_nil q.pop # non-blocking assert_raise_with_message(ThreadError, /queue empty/){q.pop(non_block=true)} end @@ -484,36 +485,11 @@ class TestQueue < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/thread/test_queue.rb#L485 assert_nil t.value end - def test_close_token_exception - [->{Queue.new}, ->{SizedQueue.new 3}].each do |qcreate| - q = qcreate[] - q.close true - assert_raise(ClosedQueueError){q.pop} - end - end - - def test_close_token_loop - [->{Queue.new}, ->{SizedQueue.new 3}].each do |qcreate| - q = qcreate[] - popped_items = [] - consumer_thread = Thread.new{loop{popped_items << q.pop}; :done} - 7.times{|i| q << i} - q.close true - sleep 0.1 unless q.empty? - assert_equal :done, consumer_thread.value - assert_equal 7.times.to_a, popped_items - end - end - def test_close_twice [->{Queue.new}, ->{SizedQueue.new 3}].each do |qcreate| q = qcreate[] q.close assert_nothing_raised(ClosedQueueError){q.close} - - q = qcreate[] - q.close(true) - assert_nothing_raised(ClosedQueueError){q.close(false)} end end @@ -533,8 +509,8 @@ class TestQueue < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/thread/test_queue.rb#L509 consumers = rand(7..12).times.map do Thread.new do count = 0 - loop do - i, st = q.pop + while e = q.pop + i, st = e count += 1 if i.is_a?(Fixnum) && st.is_a?(String) end count @@ -554,7 +530,7 @@ class TestQueue < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/thread/test_queue.rb#L530 end producers.each &:join - q.close true + q.close # results not randomly distributed. Not sure why. # consumers.map{|thr| thr.value}.each do |x| -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/