Bug #9343: [PATCH] SizedQueue#max= wakes up waiters properly

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: 2.2.0
• ruby -v: -
• Backport: 1.9.3: DONTNEED, 2.0.0: DONTNEED, 2.1: DONE

[normalperson (Eric Wong)]

We were accessing the wrong array and trying to wake up
elements stored in the queue :x

$ git pull git://80x24.org/ruby.git szqmaxset-wakeup

The following changes since commit d1cc0ebb38a23feb37bb16ff2df3137c3cead069:

mkrunnable.rb: fix DLL path on Windows (2014-01-02 02:41:06 +0000)

are available in the git repository at:

git://80x24.org/ruby.git szqmaxset-wakeup

for you to fetch changes up to 9932853dec14a4f62606098d452da80a54bf9ecc:

SizedQueue#max= wakes up waiters properly (2014-01-02 07:22:02 +0000)

---

Eric Wong (1):
SizedQueue#max= wakes up waiters properly

ext/thread/thread.c | 2 +-
test/thread/test_queue.rb | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)

[ko1 (Koichi Sasada)]

• Assignee set to ko1 (Koichi Sasada)

[ko1 (Koichi Sasada)]

sorry for long absent.

I reviewed this patch and I have a question.
Only `diff' threads should wake-up?

Index: thread.c
===================================================================
--- thread.c	(revision 44833)
+++ thread.c	(working copy)
@@ -437,7 +437,7 @@
 	diff = max - GET_SZQUEUE_ULONGMAX(self);
     }
     RSTRUCT_SET(self, SZQUEUE_MAX, vmax);
-    while (diff > 0 && !NIL_P(t = rb_ary_shift(GET_QUEUE_QUE(self)))) {
+    while (diff-- > 0 && !NIL_P(t = rb_ary_shift(GET_SZQUEUE_WAITERS(self)))) {
 	rb_thread_wakeup_alive(t);
     }
     return vmax;

[normalperson (Eric Wong)]

[e-mail] wrote:

sorry for long absent.

No worries, I forgot about this, too :x

I reviewed this patch and I have a question.
Only `diff' threads should wake-up?

Yes, your patch looks correct. Old thread.rb had "diff.times do"

[ko1 (Koichi Sasada)]

• ruby -v changed from ruby 2.2.0dev (2014-01-02 trunk 44484) [x86_64-linux] to -

(2014/02/06 2:53), [e-mail] wrote:

Yes, your patch looks correct. Old thread.rb had "diff.times do"

All right.

Could you commit it?

--
// SASADA Koichi at atdot dot net

• Status changed from Open to Closed
• % Done changed from 0 to 100

Applied in changeset r44852.

---

ext/thread: SizedQueue#max= wakes up waiters properly

* ext/thread/thread.c (rb_szqueue_max_set): use correct queue and
  limit wakeups.  [Bug #9343][ruby-core:60517]
* test/thread/test_queue.rb (test_sized_queue_assign_max):
  test for bug

[normalperson (Eric Wong)]

SASADA Koichi [e-mail] wrote:

Could you commit it?

Done, r44852. This needs a backport to 2.1

[drbrain (Eric Hodel)]

• Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN, 2.1: UNKNOWN to 1.9.3: DONTNEED, 2.0.0: DONTNEED, 2.1: REQUIRED

Update backport status

[naruse (Yui NARUSE)]

• Backport changed from 1.9.3: DONTNEED, 2.0.0: DONTNEED, 2.1: REQUIRED to 1.9.3: DONTNEED, 2.0.0: DONTNEED, 2.1: DONE