kill and raise might be a bit of a bother if you add warnings, since they're used in several libraries without any good replacement other than reimpl (usually to interrupt stuck IO operations). But yeah, I'd be on board with an across-the-board Thread.critical= warning in both JRuby and IronRuby.

Tomas Matousek wrote:
I think we should at least report a warning when Thread#kill, Thread#raise or 
Thread#critical= is called if not eliminating them as Charlie proposed on his 
blog.

Tomas

-----Original Message-----
From: Curt Hagenlocher
Sent: Tuesday, December 30, 2008 10:05 PM
To: Shri Borde; IronRuby External Code Reviewers
Cc: ironruby-core@rubyforge.org
Subject: RE: Code Review: Thread#raise

Shouldn't the option "UseThreadAbortForSyncRaise" be called "...ForASyncRaise"?
I think that Thread.raise with no arguments should just inject a RuntimeError with no 
message as if $! were nil; this makes more sense than failing.  Trying to reference a 
"current exception" in another thread is a scary operation even if that's what 
MRI is doing.

Other than that, changes look really nice.  But anyone thinking of using this 
functionality should read Charlie's excellent piece from earlier in the year: 
http://blog.headius.com/2008/02/rubys-threadraise-threadkill-timeoutrb.html

-----Original Message-----
From: Shri Borde
Sent: Monday, December 29, 2008 3:00 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core@rubyforge.org
Subject: Code Review: Thread#raise

  tfpt review "/shelveset:raise;REDMOND\sborde"
  Comment  :
  Implements Thread#raise using Thread.Abort, and added tests for it
  Implemented stress mode (RubyOptions.UseThreadAbortForSyncRaise) which found 
more issues. Fixed most but not all
  Enabled test for timeout as well
  Remaining work (not high pri for now)
   - Thread#raise without Exception parameters is not supported as it needs to 
access the active exception of the target thread. This is stored as a 
thread-local static, and so cannot be accessed from other threads. Can be fixed 
by not using ThreadStaticAttribute.
   - Adding probes (in generated code, in C# library code, etc) will help to 
raise the exception quicker as Thread.Abort can be delayed indefinitely. 
Ideally, we need both approaches. For now, using Thread.Abort goes a long way.
   - Ideally, we would add a try-catch to the IDynamicObject/MetaObject code 
paths so that when other languages called into Ruby code, they would get the 
expected user exception rather than a ThreadAbortException

  RunRSpec: supports -ruby to run with MRI. Its much faster than doing "rake ruby 
why_regression". Added support for -e to run a specific example


_______________________________________________
Ironruby-core mailing list
Ironruby-core@rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core

_______________________________________________
Ironruby-core mailing list
Ironruby-core@rubyforge.org
http://rubyforge.org/mailman/listinfo/ironruby-core

Reply via email to