[ruby-core:20999] Supporting Thread.critical=with native threads

[Brent Roman]

Hi Shri,

See comments below--->

Shri Borde wrote:

OK. I understand that MRI cannot make breaking changes lightly (though I do think the API is hard to use the way it is currently speced). We will treat this as an incompatibility in IronRuby and deal with it the best we can.

--->  What, precisely, will be the nature of the incompatibility?
        Do you intend to implement your proposed redefinition of Thread.critical=, or to mimic the all the existing Thread.critical= behaviors except for descheduling?

FWIW, Thread.pass and Kernel#sleep do not set Thread.critical to false. However, they do cause other threads to be scheduled.

--> This isn't news to me.  Have a look at my post titled "Thread misbehavior" dated 1/15/05.
      The patched Ruby 1.6.8 that we deploy in embedded systems sets Thread.critical=false whenever a the thread scheduler is invoked.  Our Kernel#doze extension clears Thread.critical immediately before blocking.  I agree that, without these changes, the current behavior is non-deterministic, as a thread can suddenly become "critical" at any random moment.  For now, all one can say is that threads in critical sections should not invoke Thread.pass or Kernel#sleep.  Changing this particular aspect of Thread.critical probably is a good idea.  The other changes you proposed seem designed to make Thread.critical work like a Mutex, which it, unfortunately, is not.


So in your scenario, when you hit a breakpoint, if you call some utility code that happens to do Thread.pass, it will "restart the world" which you will not be expecting.

--->  Why would one run a utility that does Thread.pass to examine system state for debugging?


Also, if you create a new thread expecting no one to see its uninitialized state, I think this is fragile design that will be prone to bugs. If any shared data is accessed from two threads, it should be done only if the threads have set Thread.critical=true (ie. use Thread.critical as a mutex without relying on descheduling of other threads).

I would be curious to see real-world patterns where shared data is accessed safely from two threads, and which relied on descheduling of other threads - to see if it was indeed done safely. (I don't mean to suggest that your particular code has bugs but ...) I think many people who try to use this pattern will end up with latent bugs, and I would like to see if this is the case or not.

--->  This code was written five years ago and is in hard use every day.  The Mutex pattern is safe, but  well designed Mutexless patterns perform much better in practice.  In any case, there's no point in arguing whether such code is good or bad.  It simply is.

You might want to consider dropping support for Thread.critical= from native threaded Ruby interpreters.
This is what MRI did with their version 1.9.  Why not make a clean break?
Or, support Thread.critical only in an optional lower-performing "compatibility mode" where only one thread is allowed to run at a time.

- brent

[Shri Borde]

> What, precisely, will be the nature of the incompatibility?

I am not sure where we will finally land as that will probably depend on user feedback, but we will need a note in some doc to point out that the semantics are not exactly the same as MRI. Currently, I have it implemented as a global mutex.

> I agree that, without these changes, the current behavior is
> non-deterministic, as a thread can suddenly become "critical" at any
> random moment.  For now, all one can say is that threads in critical
> sections should not invoke Thread.pass or Kernel#sleep.

Yup, this aspect is impossible to support without green threads.

> --->  This code was written five years ago and is in hard use every day.
> The Mutex pattern is safe, but  well designed Mutexless patterns perform
> much better in practice.  In any case, there's no point in arguing whether
> such code is good or bad.  It simply is.

I just wanted a canonical example of correct Mutexless code which relied on descheduling of Thread.critical=. Its easier to think about the scenario with a real example in mind. As mentioned in http://blogs.msdn.com/vancem/archive/2006/03/29/564854.aspx, one should be suspicious of lock-free code, and I have seen examples of lock-free production code which actually had latent bugs. That is not to say that lock-free code cannot be written. I just wanted to see a pattern in Ruby.

> You might want to consider dropping support for Thread.critical= from native
> threaded Ruby interpreters.
> This is what MRI did with their version 1.9.  Why not make a clean break?

I was not aware of this. I just tried 1.9 and the method is indeed gone! Note that the proposed change of the semantics I and Charles are suggesting were targeting some future version of MRI so that IronRuby and JRuby could get more compliant in the future. I did not expect 1.8.6 to be changed. With the method gone in 1.9, I couldn't have asked for a better resolution.

[Charles Oliver Nutter-2]

Brent Roman wrote:
> There should be no wriggle room in the spec about this.  The existing
> behavior may not be optimal, but it is simple and well established.  Old
> code depends on it, warts and all.  New code should avoid the use the
> Thread.critical entirely.

Given that 1.9 doesn't even have critical=, I'm not sure how backward
compatibility is part of this discussion. We're talking about specifying
critical= in such a way that the only *guaranteed* behaviors are
supportable by a single global mutex. These behaviors should be a subset
of behaviors promised by the current green-threaded critical= in MRI. So
on any parallel-threading implementation, with this "cross-impl
critical= spec" we will have an official set of behaviors users can
expect to work. I don't think anyone suggested changing 1.8.6; we just
want to specify critical= in a cross-impl way. 1.8.6's current superset
behavior would remain intact.

> For native threaded implementations, all of this can, and probably should,
> be implemented with a single, non-reentrant mutex.  Thread.critical=true
> when it was false would lock the global mutex and may block waiting for it.
> Thread.critical=false when it was true would unlock it.
>
> I think this approach is straightforward, allows native threaded
> interpreters to avoid expensive checkpointing and maintains very good
> compatibility with existing Ruby scripts.  I would suggest that JRuby keep
> its existing checkpointing available as a run-time option for maximum
> compatibility, analogous to its support of ObjectSpace.  Thus, JRuby could
> continue to support those few scripts that do depend on the fact that, in
> MRI and other green threaded implementations, Thread.critical=true can be
> used to keep all other threads from running.

Unfortunately the problem with the 1.8.6 behavior is that it's
impossible to guarantee even with vigorous checkpointing unless you
don't execute anything in parallel. Is it better to make a hard break
and support the mutex version only or pretend to support the 1.8.6
behavior but do it unreliably?

- Charlie

[Brent Roman]

Charlie & Shri,

Ruby 1.9 doesn't support Thread.critical=  However, when I last looked (about a year ago) it still had a "Giant VM Lock"  to prevent multiple Ruby threads from running in parallel.  MRI struck this compromise because it must to be able to run existing 'C' extensions unmodified.   In this threading environment it is quite easy to add this support for Thread.critical=.  I'm guessing the decision to delete Thread.critical= was to open the door for parallel threading in Ruby 2.x sometime in the future.

Ruby 1.9 *does* support Thread.exclusive {} by mapping this onto a global Mutex.  If you don't care about backward compatibility, then why do you need to blaze your own trail here?
Why not do exactly as v1.9 has already done?

The "cross-impl critical= spec" is not supported by any existing implementation.

Thread.critical is a flag that affects thread scheduling.  It is not a Mutex.
It commonly gets set when it is already true and cleared when it is already false.
Both these operations are NOPs in the current implementation!
One doesn't have to get deep into Threading Zen to see that the spec you
have posted will cause most existing code that uses Thread.critical= in any way
other than as a means of implementing a simple Mutex to fail.

The only way to support the existing Thread.critical= behavior in a meaningful way is in the context of a reduced performance threading model where only one Ruby thread runs at a time.  If there were another way, I think v1.9 would have retained it.

You've got two choices, as I see it.  Remove Thread.critical , or maintain an optional thread scheduling mode with a Giant VM Lock to ensure that only one thread runs at a time for backward compatibility with v1.8 and before.

Everything you've posted about the "vigorous checkpointing" JRuby does to support Thread.critical= indicates to me that even this valiant effort fails to implement Thread.critical in a parallel threading VM.  So, yes, in a nutshell, "It is better to make a hard break".  Drop support for Thread.critical= unless you intend to support the threading environment on which it depends.

- brent

[Charles Oliver Nutter-2]

I disagree on most points, since 1.8 is going to remain the most  
commonly used "spec" for quite a while, and the global mutex is a  
reasonable compromise that meets most use cases. Unless there are a  
large number of libraries depending on running on non-MRI impls and  
using critical= and relying on it's thread descheduling  
characteristics, we are penalizing the vast majority of JRuby or  
IronRuby users that don't for an extremely rare edge case. That's not  
very pragmatic. And if nothing else, we alternative implementers have  
learned to be very pragmatic.

Pragmatism would be doing exactly what Shri has done: propose a  
solution that appears to meet the 90+% case, actively search for  
libraries that solution would break, and evaluate the results. If  
there are such breaking cases, see if they can be fixed or weigh their  
relevance. If there are no such cases, proceed with said solution and  
be prepared to address undiscovered failures if any are reported. With  
JRuby spinning a new release every other month, the impact is minimal.

I suppose we can volunteer JRuby as a guinea pig for this new  
definition of critical=, and see how it goes. JRuby 1.1.7 will  
implement critical= as a global mutex. We shall see how it goes :)

- Charlie (mobile)

On Jan 19, 2009, at 2:28, Brent Roman <brent@...> wrote:

>
> Charlie & Shri,
>
> Ruby 1.9 doesn't support Thread.critical=  However, when I last looked
> (about a year ago) it still had a "Giant VM Lock"  to prevent  
> multiple Ruby
> threads from running in parallel.  MRI struck this compromise  
> because it
> must to be able to run existing 'C' extensions unmodified.   In this
> threading environment it is quite easy to add this support for
> Thread.critical=.  I'm guessing the decision to delete  
> Thread.critical= was
> to open the door for parallel threading in Ruby 2.x sometime in the  
> future.
>
> Ruby 1.9 *does* support Thread.exclusive {} by mapping this onto a  
> global
> Mutex.  If you don't care about backward compatibility, then why do  
> you need
> to blaze your own trail here?
> Why not do exactly as v1.9 has already done?
>
> The "cross-impl critical= spec" is not supported by any existing
> implementation.
>
> Thread.critical is a flag that affects thread scheduling.  It is not a
> Mutex.
> It commonly gets set when it is already true and cleared when it is  
> already
> false.
> Both these operations are NOPs in the current implementation!
> One doesn't have to get deep into Threading Zen to see that the spec  
> you
> have posted will cause most existing code that uses Thread.critical=  
> in any
> way
> other than as a means of implementing a simple Mutex to fail.
>
> The only way to support the existing Thread.critical= behavior in a
> meaningful way is in the context of a reduced performance threading  
> model
> where only one Ruby thread runs at a time.  If there were another  
> way, I
> think v1.9 would have retained it.
>
> You've got two choices, as I see it.  Remove Thread.critical , or  
> maintain
> an optional thread scheduling mode with a Giant VM Lock to ensure  
> that only
> one thread runs at a time for backward compatibility with v1.8 and  
> before.
>
> Everything you've posted about the "vigorous checkpointing" JRuby  
> does to
> support Thread.critical= indicates to me that even this valiant  
> effort fails
> to implement Thread.critical in a parallel threading VM.  So, yes,  
> in a
> nutshell, "It is better to make a hard break".  Drop support for
> Thread.critical= unless you intend to support the threading  
> environment on
> which it depends.
>
> - brent
>
> --
> View this message in context: http://www.nabble.com/-ruby-core%3A20999--Supporting-Thread.critical%3Dwith-native-threads-tp21224464p21539447.html
> Sent from the ruby-core mailing list archive at Nabble.com.
>
>

[Paul Brannan]

On Tue, Jan 20, 2009 at 02:52:46AM +0900, Charles Oliver Nutter wrote:
> I suppose we can volunteer JRuby as a guinea pig for this new definition
> of critical=, and see how it goes. JRuby 1.1.7 will implement critical=
> as a global mutex. We shall see how it goes :)

Will this be on or off by default?  It sounds dangerous, in that it
could silently break code.  Seems like it would be better to disable
critical= altogether and require users to adapt.

Paul

[Charles Oliver Nutter-2]

Paul Brannan wrote:
> On Tue, Jan 20, 2009 at 02:52:46AM +0900, Charles Oliver Nutter wrote:
>> I suppose we can volunteer JRuby as a guinea pig for this new definition
>> of critical=, and see how it goes. JRuby 1.1.7 will implement critical=
>> as a global mutex. We shall see how it goes :)
>
> Will this be on or off by default?  It sounds dangerous, in that it
> could silently break code.  Seems like it would be better to disable
> critical= altogether and require users to adapt.

It would be on by default; as with many other features, I think
implementing the 90% case and documenting how critical= is supposed to
officially work in JRuby would be better for now than a hard break. And
remember that even currently it's not 100% since it doesn't guarantee
other threads won't run for at least a little while before stopping. So
we're just making that behavior official and standard in JRuby, since
it's the best we can do.

- Charlie