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?
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?
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.
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.
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).