Ruby Multithreaded producer-consumer problem

On Sun, Feb 24, 2013 at 4:51 PM, Abhijit S. [email protected]
wrote:

Sean O’halpin wrote in post #1098770:

  • the only shared state you need to protect access to is the
    @orange_count variable - it’s unnecessary to use synchronize around
    the whole loop.
    True. I thought then that @age also needs protection but on closer
    review now, it doesn’t.

Access to at least these variables needs proper synchronization:
Worker@next_run
OrangeTree@age
OrangeTree@orange_count

  • you don’t check whether the exit condition for the orange_picker
    thread is true before waiting on the CV. This will halt your thread
    indefinitely when the age_increaser thread exits before the
    orange_picker thread starts the wait.

No. If the age_increaser exits before the orange_picker starts to
wait it means that it will have set @next_run to :orange.
orange_picker won’t block then.

THIS was causing the deadlock.

No, the deadlock error was caused by the fact that one of the two
threads died silently because of Math.rand(). When you fix that by
deleting “Math.” your code works.

When I added just the check to my
program, it worked flawlessly. The missing check is the consequence of
(wrongly) thinking that @age is also protected by the sync block.

Can you please share the code which is working and the change you
believe made the difference?

Note, I was still using
http://www.ruby-forum.com/attachment/8132/orange_tree.rb as basis.

Kind regards

robert

Robert K. wrote in post #1098804:

Access to at least these variables needs proper synchronization:
Worker@next_run
OrangeTree@age
OrangeTree@orange_count

It seems that you are looking at the Monitor version of the code. I was
trying everything under the sun to get the code working so there exist a
couple of versions. The version Sean fixed does NOT use a Monitor, it
uses a Mutex.
I’ve uploaded my Mutex and Monitor versions and Sean’s Mutex version to
my GitHub
(ruby/learn_to_program at master · asarkar/ruby · GitHub).
The names are self-descriptive (orange_tree_*)
The Monitor version still has the deadlock issue but I have pushed it
off for later.

No. If the age_increaser exits before the orange_picker starts to
wait it means that it will have set @next_run to :orange.
orange_picker won’t block then.

Again, guess we are talking about 2 different versions.

No, the deadlock error was caused by the fact that one of the two
threads died silently because of Math.rand(). When you fix that by
deleting “Math.” your code works.

You are right that Math.rand was a culprit but it wasn’t alone. In Mutex
version, the deadlock happens without the following check even with
Math.rand fixed.
if @orange_tree.age < OrangeTree::AGE_TO_DIE
@cv.wait(@mutex)
end

Can you please share the code which is working and the change you
believe made the difference?

https://github.com/abhijitsarkar/ruby/tree/master/learn_to_program/orange_tree_*

I apologize if there was any confusion about different versions of the
program. It is not so confusing in the forum posts but I guess may not
be in the mailing list.

On Sun, Feb 24, 2013 at 4:23 PM, Robert K.
[email protected] wrote:

Access to at least these variables needs proper synchronization:
Worker@next_run
OrangeTree@age
OrangeTree@orange_count

Hi,

I think we must have been looking at different versions of the code -
there’s no @next_run in the version I responded to
(http://www.ruby-forum.com/attachment/8128/orange_tree.rb). I guess I
should have made it clear which version I was trying to fix (there
appear to be quite a few). Note, I did mistakenly point to the OP’s
first version in the description of the gist I wrote but that does not
use @next_run either. I take it you didn’t look at
Attempt to get http://www.ruby-forum.com/attachment/8128/orange_tree.rb working - see http://www.ruby-forum.com/topic/4410866 · GitHub.

Also, why would @age need synchronization?

  • you don’t check whether the exit condition for the orange_picker
    thread is true before waiting on the CV. This will halt your thread
    indefinitely when the age_increaser thread exits before the
    orange_picker thread starts the wait.

No. If the age_increaser exits before the orange_picker starts to
wait it means that it will have set @next_run to :orange.
orange_picker won’t block then.

THIS was causing the deadlock.

Like I said, there’s no @next_run in the version I responded to.
You’re right for the version of the code you’re referring to. Putting
the check on age in worked for the version I was looking at.

Regards,
Sean

On Sun, Feb 24, 2013 at 10:37 PM, Robert K.
[email protected] wrote:

On Sun, Feb 24, 2013 at 9:08 PM, Sean O’Halpin [email protected] wrote:

Also, why would @age need synchronization?

In http://www.ruby-forum.com/attachment/8132/orange_tree.rb there is
read access to @age from one thread (#pick_an_orange) and read write
access from the other (#one_year_passes) => synchronization needed.

Ah yes - thanks. I have a bad habit of thinking of integer read/writes
as atomic but a Fixnum isn’t that simple (nor is anything anyway with
multiprocessors in the mix).

Regards,
Sean

On Sun, Feb 24, 2013 at 9:08 PM, Sean O’Halpin [email protected]
wrote:

I think we must have been looking at different versions of the code -
there’s no @next_run in the version I responded to

Too bad.

(http://www.ruby-forum.com/attachment/8128/orange_tree.rb). I guess I
should have made it clear which version I was trying to fix (there
appear to be quite a few). Note, I did mistakenly point to the OP’s
first version in the description of the gist I wrote but that does not
use @next_run either. I take it you didn’t look at
Attempt to get http://www.ruby-forum.com/attachment/8128/orange_tree.rb working - see http://www.ruby-forum.com/topic/4410866 · GitHub.

Also, why would @age need synchronization?

In http://www.ruby-forum.com/attachment/8132/orange_tree.rb there is
read access to @age from one thread (#pick_an_orange) and read write
access from the other (#one_year_passes) => synchronization needed.

Cheers

robert