From: Roger Pack <rogerpack2005@...>
Date: 2008-03-21T16:08:47+09:00
Subject: Re: Thread#raise, Thread#kill, and timeout.rb are unsafe

I still like Ara's idea :)

However--is something like this already possible if we redefine the 
Thread#raise method?  For example define it to be something like this.

class Thread
 def raise_safe *args
       # sorry, couldn't think of a better way than a mutex to prohibit 
weirdness from occurring.  So that's where a problem would occur--in the 
ensure block of the synchronize function.  Ahh well.  If you re-aliased 
raise to be this method, maybe avoided.

  @raise_mutex.synchronize {
    unless @currently_non_interruptible_section
       raise *args
    else
          self.queued_interrupts << *args
    end
  }
 end

 def uninterruptible_block
    return if @currently_non_interruptible_section # already within one. 
Maybe should keep a count of how deep you are. Not sure
    @currently_non_interruptible_section = true
    begin
      yield # might want to handle queued exceptions at the end of this 
block
    ensure
      @raise_mutex.synchronize {@currently_non_interruptible_section = 
false}
    end
    # after this point the thread is subject to interruptions again
 end

end

Then you could have a thread do something like

uninterruptible_block do
  # some stuff
  # handle the queued interrupts if desired.  Note that you could loop 
and 'check if you've been interupted yet' once per loop or what not, if 
desired

end

# handle queued interrupts, if desired



timeout would possibly become

def timeout(sec, exception=Error)
  return yield if sec == nil or sec.zero?
  raise ThreadError, "timeout within critical session"\
                                      if Thread.critical
  raise ThreadError, "called nested functions that want to use 
uninterruptible and raise on it" if 
Thread.current.currently_non_interruptible_section
  # this is necessary since we do use 'real' raise this time
  # to interrupt our block's yielding
  # so if you had nested--ever--the raise of an uppermost one
  # could interrupt a lower one in it ensure. Same problem would occur.
  # we can only have one at a time.
  # this is a hack that assumes we 'only' use uninterruptible_block for 
calls to timeout.

  death_mutex = Mutex.new

  uninterruptible_block do
   begin
     x = Thread.current
     y = Thread.start {
       sleep sec
       death_mutex.synchronize {
         x.raise exception, "execution expired" if x.alive? # this is 
not raise_safe, but real raise.  Still unsafe as per some previous 
comments...but is it safer than it used to be now?
       }
     }
     yield sec # would this code be dampered somehow?
   ensure
     death_mutex.synchronize {
       y.kill if y and y.alive?
     }
   end
  end
  for interrupt in Thread.current.queued_interrupts do; raise interrupt; 
end # or something like that
end

now you could "replace" the use of raise with raise_safe.  Would this 
work?
This would prohibit 'other threads' from interrupting a thread that is 
within a timeout block (obviously bad) until it timed out, but would it 
be thread safe?

Thoughts?
Thank you for reading :)

Overall I'd say I'm in agreeance with the OP's call to deprecate 
Thread#kill, Thread#raise, Thread#critical= as they can too easily hurt, 
though something like this might help.  This suggestion also seems far 
less complete than Tanaka's somehow.

Still wishing for an ensure block that was somehow 'guaranteed' to run 
all the way through, and delay interruptions or ignore them till it 
ended.
Thanks.

-R


Tanaka Akira wrote:
> In article <E1JaM9H-0000jj-AB@x61.netlab.jp>,
>   Yukihiro Matsumoto <matz@ruby-lang.org> writes:
> 
>> What should safe point declaration mechanism look like?
> 
> Basically, asynchronous events should be queued.
> 
> * make a queue for each thread.
> * Thread#raise(exc) enqueue exc to the queue.
> * Thread.check_interrupt(klass) checks an exception which is
>   a kind of klass in the queue of the current thread.  If it
>   exists, the exception is raised.
> * other queue examining mechanism, such as sigpending, etc,
>   may be useful.
> 
> Thread.check_interrupt is a safe point.
> 
> However safe points is not only Thread.check_interrupt.
> Blocking operations, such as I/O, may or may not be safe
> points.  It is because Thread.check_interrupt with blocking
> operations causes race conditions.  So application must
> choose that make a blocking operation interruption safe or
> uninterruptible.
> 
> * Thread.blocking_interruptible(klass) { ... }
> * Thread.blocking_uninterruptible(klass) { ... }
> 
> Blocking operations in Thread.blocking_interruptible are
> safe points.
> 
> If a thread has no resources to release, it is safe to kill
> asynchronously.  Other than that, safe points should be
> declared explicitly.  When a resource is acquired,
> application should declare it.
> 
> * Thread.delay_interrupt(klass) { ... }
> 
> It is safe points that out of outermost
> Thread.delay_interrupt.
> 
> Another idea is about ensure clause.  Since an ensure
> clause is used to release a resource, it may delay
> asynchronous events as in Thread.delay_interrupt.

-- 
Posted via http://www.ruby-forum.com/.