I replied before seeing this... Taking a sampling of a few gems, I think it should actually be the other way. Mongrel and webbrick do use Thread#kill to kill a background thread, but other gems do not. Thread#raise was also used in just a couple of places to raise a timeout error. And it is very hard, if not impossible, to use these APIs safely since you do not know what the other thread is doing.
OTOH, Thread.critical= can actually be used in a sane way for simple synchronization, like the lock keyword in C#. This is used way more widely, and a warning for this will cause lot of false alarms. Thanks, Shri -----Original Message----- From: charles.o.nut...@sun.com [mailto:charles.o.nut...@sun.com] On Behalf Of Charles Oliver Nutter Sent: Monday, January 05, 2009 6:26 AM To: ironruby-core@rubyforge.org Cc: Curt Hagenlocher; Shri Borde; IronRuby External Code Reviewers Subject: Re: [Ironruby-core] Code Review: Thread#raise 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
