Custom Mutex methods undefined by fastthread

At the risk of asking an FAQ, I've run into the following problem:

irb(main):001:0> require 'thread'
=> true
irb(main):002:0> class Mutex; attr_accessor :owner; end
=> nil
irb(main):003:0> require 'rubygems'
=> true
irb(main):004:0> require 'fastthread'
=> true
irb(main):005:0> a = Mutex.new
=> #<Mutex:0xb788184c>
irb(main):006:0> a.owner = :foo
NoMethodError: undefined method `owner=' for #<Mutex:0xb788184c>
         from (irb):6

As you can see, the method definition prior to the fastthread require is lost. This wasn't a problem with 0.6.4, but is a problem with 1.0. Is this a genuine bug, or is there a good reason for this?

$ ruby -v
ruby 1.8.4 (2005-12-24) [i486-linux]

···

--
Alex

I don't know. But IMHO it's a bad idea to change class Mutex. Why do you think you need that?

Kind regards

  robert

···

On 11.06.2007 16:21, Alex Young wrote:

At the risk of asking an FAQ, I've run into the following problem:

irb(main):001:0> require 'thread'
=> true
irb(main):002:0> class Mutex; attr_accessor :owner; end
=> nil
irb(main):003:0> require 'rubygems'
=> true
irb(main):004:0> require 'fastthread'
=> true
irb(main):005:0> a = Mutex.new
=> #<Mutex:0xb788184c>
irb(main):006:0> a.owner = :foo
NoMethodError: undefined method `owner=' for #<Mutex:0xb788184c>
        from (irb):6

As you can see, the method definition prior to the fastthread require is lost. This wasn't a problem with 0.6.4, but is a problem with 1.0. Is this a genuine bug, or is there a good reason for this?

Hi,

At Mon, 11 Jun 2007 23:21:29 +0900,
Alex Young wrote in [ruby-talk:255142]:

irb(main):006:0> a.owner = :foo
NoMethodError: undefined method `owner=' for #<Mutex:0xb788184c>
         from (irb):6

Rather I consider it a bug if it is possible to change the
owner of a Mutex without locking.

As you can see, the method definition prior to the fastthread require is
lost. This wasn't a problem with 0.6.4, but is a problem with 1.0. Is
this a genuine bug, or is there a good reason for this?

It would be a genuine bugfix, I guess.

···

--
Nobu Nakada

In 1.0, the old Mutex class is swapped out and replaced with a fresh one when fastthread is included. This was necessary to permit requiring fastthread after thread had already been required (RubyGems, for example, needed this). If the classes weren't swapped, then any pre-existing mutexes would suddenly end up with new method definitions, breaking them.

-mental

···

On Mon, 11 Jun 2007 23:21:29 +0900, Alex Young <alex@blackkettle.org> wrote:

As you can see, the method definition prior to the fastthread require is
lost. This wasn't a problem with 0.6.4, but is a problem with 1.0. Is
this a genuine bug, or is there a good reason for this?

Robert Klemme wrote:

···

On 11.06.2007 16:21, Alex Young wrote:

At the risk of asking an FAQ, I've run into the following problem:

irb(main):001:0> require 'thread'
=> true
irb(main):002:0> class Mutex; attr_accessor :owner; end
=> nil
irb(main):003:0> require 'rubygems'
=> true
irb(main):004:0> require 'fastthread'
=> true
irb(main):005:0> a = Mutex.new
=> #<Mutex:0xb788184c>
irb(main):006:0> a.owner = :foo
NoMethodError: undefined method `owner=' for #<Mutex:0xb788184c>
        from (irb):6

As you can see, the method definition prior to the fastthread require is lost. This wasn't a problem with 0.6.4, but is a problem with 1.0. Is this a genuine bug, or is there a good reason for this?

I don't know. But IMHO it's a bad idea to change class Mutex. Why do you think you need that?

It's in some legacy code that broke on a gem update. It's used for logging which thread's currently got the lock from inside a #synchronize block. I've fixed it by changing the require order, which I don't like much. It could probably be refactored out, but that's not the point...

--
Alex

MenTaLguY wrote:

···

On Mon, 11 Jun 2007 23:21:29 +0900, Alex Young <alex@blackkettle.org> > wrote:

As you can see, the method definition prior to the fastthread
require is lost. This wasn't a problem with 0.6.4, but is a
problem with 1.0. Is this a genuine bug, or is there a good reason
for this?

In 1.0, the old Mutex class is swapped out and replaced with a fresh
one when fastthread is included. This was necessary to permit
requiring fastthread after thread had already been required
(RubyGems, for example, needed this). If the classes weren't
swapped, then any pre-existing mutexes would suddenly end up with new
method definitions, breaking them.

I see... And there's no easy way to tell if Mutex has been modified in user code before that, so you couldn't even raise a warning in that case... Hum. Still, at least Google knows about it now, for posterity :slight_smile:

--
Alex

Well, it made it more obvious that any customizations depending on Mutex internals were broken (they didn't work in 0.6.4 either; it was just less obvious).

On the other hand, I don't want to penalize customizations which are threadsafe and don't depend on Mutex internals. So, if folks can provide examples of customizations which are threadsafe and which don't depend on Mutex internals, I'll see what I can do for them.

-mental

···

On Tue, 12 Jun 2007 01:08:21 +0900, Nobuyoshi Nakada <nobu@ruby-lang.org> wrote:

As you can see, the method definition prior to the fastthread require is
lost. This wasn't a problem with 0.6.4, but is a problem with 1.0. Is
this a genuine bug, or is there a good reason for this?

It would be a genuine bugfix, I guess.

irb(main):006:0> a.owner = :foo
NoMethodError: undefined method `owner=' for #<Mutex:0xb788184c>
        from (irb):6

Out of curiosity, how did it work with earlier versions of fastthread? fastthread never stored the current owner in @owner.

It's in some legacy code that broke on a gem update. It's used for
logging which thread's currently got the lock from inside a #synchronize
block. I've fixed it by changing the require order, which I don't like
much. It could probably be refactored out, but that's not the point...

Modifying Mutex isn't a very safe way to accomplish that, particularly as you can't rely on implementation details (because the implementation of Mutex is different for each Ruby implementation). If the owner information is important to record, you'd be much better off writing a wrapper around Mutex and using that instead.

-mental

···

On Tue, 12 Jun 2007 00:50:36 +0900, Alex Young <alex@blackkettle.org> wrote:

MenTaLguY wrote:

irb(main):006:0> a.owner = :foo NoMethodError: undefined method
`owner=' for #<Mutex:0xb788184c> from (irb):6

Out of curiosity, how did it work with earlier versions of
fastthread? fastthread never stored the current owner in @owner.

No, that was mine. I was (for various reasons) assigning owner inside a synchronized block.

It's in some legacy code that broke on a gem update. It's used for
logging which thread's currently got the lock from inside a
#synchronize block. I've fixed it by changing the require order,
which I don't like much. It could probably be refactored out, but
that's not the point...

Modifying Mutex isn't a very safe way to accomplish that,
particularly as you can't rely on implementation details (because the
implementation of Mutex is different for each Ruby implementation).
If the owner information is important to record, you'd be much better
off writing a wrapper around Mutex and using that instead.

To be honest, I'll probably just get rid of the code in this case, as it's just logging execution data at this point, and it's more of a security blanket than essential information. However, I'm not really relying on an implementation detail here - I was adding to the implementation by reopening the class. I should have subclassed, but isn't the convention that we can modify classes to our own desires? I admit that modifying a synchronisation primitive is playing with fire, but I was purposefully not affecting its synchronisation behaviour.

Still, I understand the reason it's stopped working - it just came at a rather inconvenient moment :slight_smile:

···

On Tue, 12 Jun 2007 00:50:36 +0900, Alex Young <alex@blackkettle.org> > wrote:

--
Alex

I should have subclassed, but isn't the convention that we can modify classes
to our own desires?

Yeah -- I'm not uninterested in supporting that somehow, but on the other hand all the Mutex customizations I've seen so far have either lacked thread safety, or they depended on implementation details which would have meant they wouldn't work under e.g. JRuby either. So I'm a little reluctant.

I admit that modifying a synchronisation primitive is playing with fire,
but I was purposefully not affecting its synchronisation behaviour.

Out of curiosity, could you post the code in question? It's difficult for me to see how modifying @owner could not affect the behavior of a "classic" thread.rb mutex, but then I've not seen the code in question.

-mental

···

On Tue, 12 Jun 2007 01:58:06 +0900, Alex Young <alex@blackkettle.org> wrote:

Since your modification does sound safe (though you're going to need to be very careful about @owner's readers...), I'll see if I can think of a solution for the customization problem (suggestions welcome!).

-mental

···

On Tue, 12 Jun 2007 01:58:06 +0900, Alex Young <alex@blackkettle.org> wrote:

I should have subclassed, but isn't the convention that we can modify classes
to our own desires?

Whoops, "classic" thread.rb uses @locked; never mind!

-mental

···

On Tue, 12 Jun 2007 02:10:00 +0900, MenTaLguY <mental@rydia.net> wrote:

Out of curiosity, could you post the code in question? It's difficult for
me to see how modifying @owner could not affect the behavior of a "classic"
thread.rb mutex, but then I've not seen the code in question.