[Ironruby-core] Code Review: Issues with Thread#kill and raising exception from ensure clause

Shri Borde Shri.Borde at microsoft.com
Sat Jan 10 00:54:48 EST 2009


Inline...

-----Original Message-----
From: Tomas Matousek 
Sent: Friday, January 09, 2009 4:34 PM
To: Shri Borde; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: RE: Code Review: Issues with Thread#kill and raising exception from ensure clause

Are any methods in RubyOps Exceptions #region (including those added by this shelveset) emitted to IL? If not they should rather be in RubyUtils class.
[Shri Borde] They are not emitted to IL. Moved it to RubyUtils...

On the other hand, SourceUnitTree.CheckForAsyncRaiseViaThreadAbortshould should be marked as [Emitted], be in RubyOps.cs and SourceUnitTree.GenerateCheckForAsyncException should use Methods.CheckForAsyncRaiseViaThreadAbort to get the method.
[Shri Borde] Done

Could we add a public helper that calls thread.Abort(new ThreadExitMarker()); move the private ThreadExitMarker class and IsRubyThreadExit to RubyUtils? Then we won't need to duplicate the marker class in ThreadOps?
[Shri Borde] Its not duplicated. I already moved it out of ThreadOps.

Other than that, looks good.

Tomas

-----Original Message-----
From: Shri Borde 
Sent: Friday, January 09, 2009 3:33 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: Code Review: Issues with Thread#kill and raising exception from ensure clause

  tfpt review "/shelveset:kill;REDMOND\sborde"

  Comment  : 
  Added more tests for Thread#kill, and fixed bugs that were exposed. Throwing an exception from the ensure clause exposes new code paths.
  
  Ruby allows Thread#kill to be completely subverted (not just deferred as an infinite loop in an ensure clause would do) by raising an exception from a ensure clause. I have not matched that behavior, and left it as a IronRuby bug for now.




More information about the Ironruby-core mailing list