From: Eric Hodel <drbrain@...7.net>
Date: 2006-09-19T15:47:30+09:00
Subject: Re: timeout and ensure

On Sep 17, 2006, at 12:36 PM, Joel VanderWerf wrote:

> Jan Svitok wrote:
> ...
>> FYI: Eric Hodel has written a post about nested timeouts:
>> http://blog.segment7.net/articles/2006/04/11/care-and-feeding-of- 
>> timeout-timeout
>
> The second part of the article points out a potential danger of the  
> interaction between timeouts and rescue/ensure. But the proposed  
> solution may not be good advice.
>
> The danger is that a timeout exception may fire during an ensure  
> clause, preventing the ensure clause (and any necessary cleanup  
> code within it) from finishing, which could cause resource leaks or  
> other problems.
>
> The solution proposed in the article is to wrap code within the  
> ensure clause in a begin..end block to handle timeout exceptions,  
> like this:
>
>   require 'timeout'
>
>   Timeout.timeout 2 do
>     begin
>       puts "Allocating the thingy..."
>       sleep 1
>       raise RuntimeError, 'Oh no! Something went wrong!'
>     ensure
>       # Since we might time out, hold onto the timeout we caught
>       # so we can re-raise it when we're done cleaning up.
>       timeout = nil
>       begin # we really need to clean up
>         puts "Cleaning up after the thingy..."
>         sleep 2
>         puts "Cleaned up after the thingy!"
>       rescue Timeout::Error => e
>         puts "Timed out! Trying again!"
>         timeout = e # save that timeout then retry
>         retry
>       end
>       # Raise the timeout so we time out all the way to the top.
>       raise timeout unless timeout.nil?
>     end
>   end
>
> However, that solution only reduces the chance of the timeout  
> interfering with the ensure clause. Suppose the timeout fires while  
> the main thread is executing the line "timeout = nil". (It's  
> impossible in this example, but you can get it to happen by putting  
> a "sleep 5" just after this line.) Then the inner begin..end clause  
> doesn't catch the timeout, and the cleanup doesn't happen. In  
> general, unless you are very sure about the timings of your code,  
> you cannot guarantee that the timeout won't fire at the wrong time.  
> So it's a race condition.

 From reading rb_eval, it might be possible to fix the race condition  
by exploiting implementation details, but I don't have the  
opportunity to examine it in depth.

rb_eval looks something like this:

rb_eval(VALUE self, NODE *node) {
   switch (nd_type(node)) {
   case NODE_ENSURE:
     rb_eval(self, node->begin_body);
     rb_eval(self, node->ensure_body);
     break;
   case NODE_RESCUE:
     /* ... */
   }
   CHECK_INTS; /* check for a thread to switch to or a signal to  
process */
}

So threads may be switched (and Timeout::Error raised) only when is  
finished evaluating a node.

So if the first item of an ensure is begin (which creates a  
NODE_RESCUE) we might be "safe":

$ parse_tree_show -f
begin
   perform some work
ensure
   begin
     clean up after ourselves
   rescue Timeout::Error
     retry
   end
end
(eval):2: warning: parenthesize argument(s) for future version
(eval):5: warning: parenthesize argument(s) for future version
(eval):5: warning: parenthesize argument(s) for future version
[[:ensure,
   # this is the begin body.
   [:fcall, :perform, [:array, [:fcall, :some, [:array,  
[:vcall, :work]]]]],
   # this is the ensure body
   [:rescue,
    # this is the inner begin body, we will always reach here after  
ensure.
    [:fcall,
     :clean,
     [:array,
      [:fcall,
       :up,
       [:array, [:fcall, :after, [:array, [:vcall, :ourselves]]]]]]],
    # this is the rescue statement
    [:resbody, [:array, [:colon2, [:const, :Timeout], :Error]],
     # this is the rescue body
     [:retry]]]]]

I am on vacation starting tomorrow, so I'll probably look up this  
thread when I return.

-- 
Eric Hodel - drbrain@segment7.net - http://blog.segment7.net
This implementation is HODEL-HASH-9600 compliant

http://trackmap.robotcoop.com