I think the implications of critical= in MRI are more complicated than
just a local lock. It actually stops normal thread scheduling, so other
threads freeze what they're doing. That's vastly more intrusive than
just a lock or critical section:
◆ ruby -e "Thread.new { sleep 1; puts Time.now; redo }; sleep 3;
Thread.critical = true; puts 'critical'; sleep 3; puts 'leaving
critical'; Thread.critical = false; sleep 3"
Tue Jan 06 18:50:34 +0000 2009
Tue Jan 06 18:50:35 +0000 2009
critical
leaving critical
Tue Jan 06 18:50:39 +0000 2009
Tue Jan 06 18:50:40 +0000 2009
Tue Jan 06 18:50:41 +0000 2009
If those other threads are holding locks or resources, deadlock is
extremely easy, and of course it's nearly impossible to emulate right
with real parallel threads since you can't easily stop them whenever you
want.
#kill and #raise are also very tricky and dangerous, but they're at
least localized. They can be defined in terms of a message passed from
one thread to another plus a periodic message checkpoint.
I still believe critical= is much more in need of a warning.
Shri Borde wrote:
Warning sounds reasonable for Thread#kill and Thread#raise. FWIW, Mongrel and
webbrick do use Thread#kill to kill a background thread.
Thread.critical= can actually be used in a sane way for simple synchronization,
like the lock keyword in C#. This is used much more widely, and a warning for
this will cause lot of false alarms.
Thanks,
Shri
-----Original Message-----
From: Tomas Matousek
Sent: Saturday, January 03, 2009 11:52 AM
To: Curt Hagenlocher; Shri Borde; IronRuby External Code Reviewers
Cc: ironruby-core@rubyforge.org
Subject: RE: Code Review: Thread#raise
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
