From: Bill Kelly <billk@...>
Date: 2004-08-28T06:08:39+09:00
Subject: Re: Request For Comments: exception safe ConditionVariable#wait

Hi Paul,

> IMO, this isn't so much an exception-safety issue as it is a
> thread-safety issue.

With timeout, I'm thinking it's sort of a combination of
exceptions and threads.  Timeout can raise an exception at
"any point" within the timeout block; but it uses a secondary
thread to perform the raise.  Which means, things like
Thread.exclusive { } can be used to protect blocks of code
against the exception that would be raised by the timeout thread.

I hadn't thought about the more general exception safety
issues you bring up:

>  To write exception-safe code, it is necessary to
> guarantee that certain code will never raise an exception.  For example,
> suppose that I have implemented my own string class:
> 
>   class MyString
>     def initialize(characters, encoding)
>       @characters = characters.dup
>       @encoding = encoding
>     end
> 
>     attr_reader :characters, :encoding
>   end
> 
> now suppose I want to write MyString#replace.  I can easily do this if I
> can guarantee that swap() never raises an exception:
> 
>   class MyString
>     def replace(other)
>       copy = other.dup
>       swap(copy)
>     end
> 
>     def swap(other)
>       tmp_characters   = other.characters
>       tmp_encoding     = other.encoding
>       other.characters = self.characters
>       other.encoding   = self.encoding
>       self.characters  = tmp_characters
>       # better not get an exception here!
>       self.encoding    = tmp_encoding
>     end
>   end
> 
> Now if I write:
> 
>   s1 = MyString.new([?a, ?b, ?c], :ASCII)
>   s2 = MyString.new([?1, ?2, ?3], :ASCII)
>   timeout(x) do
>     s.replace(s2)
>   end
> 
> I'm in trouble, because suddenly I can get a TimeoutError from inside
> MyString#swap.
> 
> Your solution for ConditionVariable#wait works for that one case, but
> it's not generally applicable to other cases like this one.  Instead,
> for each case, I either have to set Thread.critical or I have to write
> code to rollback any changes I make after an exception anywhere I
> non-atomically change the state of an object.  What a pain (and a
> performance bottleneck!)

I agree - but this seems to be in the nature of timeout
itself.  My proposed changes to ConditionVariable#wait were
indeed intended to only ensure that that one method, #wait,
would function properly when used inside a timeout block.
(Because I happen to be in need of a #wait that I can time
out on. :)

I see what you mean now (exception safety vs. thread 
safety.)  I guess the email subject should more properly
be, "timeout-safe ConditionVariable#wait".  :-)

My sense is that currently, one has to be very careful
where one uses a timeout.  I don't use timeout very often,
so a lack of a general solution isn't necessarly giving me
too much grief.  But I do need to be sure that whatever I
do use timeout with, is safe.  (At least that's my current
mindset.)

> I'm not sure what a good general solution the problem is, but consider
> this:
> 
>   - a keyword (or method) is added to Ruby that indicates a method (or
>     block) never raises an exception
>   - if Thread#raise is called while the thread is executing that method
>     (or block), then set a flag and wait for the block to exit before
>     actually raising the exception
>   - If the user still wants to abort execution of that block, the user
>     can call Thread#force_raise or Thread#kill.
> 
> So now MyString#swap looks like this:
> 
>   def swap(other)
>     no_raise do
>       tmp_characters   = other.characters
>       tmp_encoding     = other.encoding
>       other.characters = self.characters
>       other.encoding   = self.encoding
>       self.characters  = tmp_characters
>       # better not get an exception here!
>       self.encoding    = tmp_encoding
>     end
>   end

Interesting.  Kind of like an interrupt mask for exceptions. :)

If you only wanted to protect against timeout, and not 
exceptions in general, I think (as you've already 
observed) you could:

   def swap(other)
     Thread.exclusive do
       tmp_characters   = other.characters
       tmp_encoding     = other.encoding
       other.characters = self.characters
       other.encoding   = self.encoding
       self.characters  = tmp_characters
       # better not get an exception here!
       self.encoding    = tmp_encoding
     end
   end

> A similar solution can be applied to your revised version of
> ConditionVariable#wait (consider what happens if you time out twice,
> e.g.):
> 
>   def foo(timeout_secs)
>     begin
>       timeout(timeout_secs) { @condition_var.wait(@mutex) }
>     rescue Timeout::Error
>     end
>   end
> 
>   timeout(5.01) do
>     foo(5)
>   end

I'm sorry, I'm not sure I'm following.  Does the nested
timeout here violate any safeguards inside the revised
ConditionVariable#wait implementation?  I realize the
rescue Timeout::Error there is not sufficient to 
squelch timeout exceptions from any nested/outer timeout
block.  Your outer timeout would need to rescue its own
exceptions as well. . . . Is that what you were referring
to?  Or am I not understanding your example?


Thanks for your feedback !!


Regards,

Bill