Asp Forum - Synchronized attr_accessor

Nasir Khan

6/8/2007 7:35:00 PM

I have a fairly repetitive use case of having to define attributes and
have them lock protected. I was thinking of defining a attr_accessor
kind of directive but one that creates a lock protected attribute -

class Module
def attr_sync_accessor(*symbols)
symbols.each do |symbol|
str = <<-EOF
def #{symbol}()
if @#{symbol}
@#{symbol}.synchronize { return @#{symbol} }
else
nil
end
end
EOF
module_eval(str)
str = <<-EOF
def #{symbol}=(val)
if @#{symbol}
@#{symbol}.synchronize { @#{symbol} = val }
else
@#{symbol} = val
@#{symbol}.extend(MonitorMixin)
end
end
EOF
module_eval(str)
end
end
end

#--------------

Now to use it you need to require monitor so

require 'monitor'

class A
attr_sync_accessor :hello
def get
hello
end
def set(val)
self.hello = val
end
end

irb(main):190:0* d = A.new
=> #<D:0x27f77d4>
irb(main):191:0> d.set "h"
=> "h"
irb(main):192:0> d.get
=> "h"

This works but obviously has some flaws, like for example first time
creation of attribute is unprotected as is the nil check, also it does
not address scope [private, protected etc.].

I am looking for suggestions on improvements to this if anyone else
also finds it useful or if it is solved by someone in some other way
and I have overlooked something.

TIA
- Nasir

11 Answers

MenTaLguY

6/8/2007 7:48:00 PM

On Sat, 9 Jun 2007 04:34:42 +0900, Nasir Khan <rubylearner@gmail.com> wrote:
> This works but obviously has some flaws, like for example first time
> creation of attribute is unprotected as is the nil check, also it does
> not address scope [private, protected etc.].

Could you describe are you trying to accomplish using these attributes? There is usually a better way to communicate between threads than setting/getting attributes on an object.

-mental

Nasir Khan

6/8/2007 7:55:00 PM

This is about having a instance variable always accessed under a lock,
if object state (instance variables) are accessed by many threads at
the same time
for eg. a hash accessed by several threads for getting values but some
operation also adding deleting from the hash. You would want a
synchronized access to your hash in this case...

So this could save some repetitive lines of code, that is all.

- nasir

On Jun 8, 12:48 pm, MenTaLguY <men...@rydia.net> wrote:
> On Sat, 9 Jun 2007 04:34:42 +0900, Nasir Khan <rubylear...@gmail.com> wrote:
> > This works but obviously has some flaws, like for example first time
> > creation of attribute is unprotected as is the nil check, also it does
> > not address scope [private, protected etc.].
>
> Could you describe are you trying to accomplish using these attributes? There is usually a better way to communicate between threads than setting/getting attributes on an object.
>
> -mental

MenTaLguY

6/8/2007 9:13:00 PM

On Sat, 9 Jun 2007 04:55:24 +0900, Nasir Khan <rubylearner@gmail.com> wrote:
> This is about having a instance variable always accessed under a lock,
> if object state (instance variables) are accessed by many threads at
> the same time for eg. a hash accessed by several threads for getting values but some
> operation also adding deleting from the hash. You would want a
> synchronized access to your hash in this case...

Just because access to an instance variable is synchronized does not mean that access to the object it references is synchronized. Retrieving a hash object via a synchronized accessor does not make it safe against concurrent modification.

There are approaches which will work, but to know which ones to suggest, I will need to know more about the "bigger picture" that this code is part of.

-mental

Nasir Khan

6/8/2007 9:54:00 PM

There is no big picture.
The problem is simple - "create accessors for an instance variable
that access the variable under a lock".
Just as problem statement for "attr_accessor" could be - "create
accessors for instance variables"

- Nasir

PS: I understand that a synchronized access to hash is not the same as
synchronized access to hash values, but this problem is *not* about
it.

On Jun 8, 2:13 pm, MenTaLguY <men...@rydia.net> wrote:
> On Sat, 9 Jun 2007 04:55:24 +0900, Nasir Khan <rubylear...@gmail.com> wrote:
> > This is about having a instance variable always accessed under a lock,
> > if object state (instance variables) are accessed by many threads at
> > the same time for eg. a hash accessed by several threads for getting values but some
> > operation also adding deleting from the hash. You would want a
> > synchronized access to your hash in this case...
>
> Just because access to an instance variable is synchronized does not mean that access to the object it references is synchronized. Retrieving a hash object via a synchronized accessor does not make it safe against concurrent modification.
>
> There are approaches which will work, but to know which ones to suggest, I will need to know more about the "bigger picture" that this code is part of.
>
> -mental

Robert Dober

6/8/2007 10:24:00 PM

On 6/8/07, Nasir Khan <rubylearner@gmail.com> wrote:
> There is no big picture.
> The problem is simple - "create accessors for an instance variable
> that access the variable under a lock".
> Just as problem statement for "attr_accessor" could be - "create
> accessors for instance variables"

Hmm there is no such thing as a lock on an ivar.
By declaring a synchronized accessor there is just no way to know
anything about the behavior of the accessed object, it might be
threadsafe or immutable, therefore it does not make much sense to me
and if I have understood your code correctly neither to Ruby. Imagine
someone using your synchronized accessor to change the ivar to a
simple integer, your code would fail at the next access attempt.

Cheers
Robert

MenTaLguY

6/8/2007 10:40:00 PM

On Sat, 9 Jun 2007 06:54:04 +0900, Nasir Khan <rubylearner@gmail.com> wrote:
> There is no big picture.

Since you had asked for feedback on anything you'd missed, I was trying to find out if was some mitigating factor in the specific way your program worked (the one for which you originally wrote this code), before telling you that it won't work.

But -- unless the objects the attributes were set to were never modified, even if they didn't have the thread safety problems you already noted, the accessors generated still couldn't ensure thread safety. If thread 1 calls obj.some_accessor.foo, and thread 2 calls obj.some_accessor.bar (where #bar is some mutating method), some_accessor being synchronized simply _will not_ protect you.

-mental

Nasir Khan

6/9/2007 2:35:00 AM

Yeah I see what you guys mean...I was a little delusional. Now I
realize that I was looking for basically something equivalent to
Java's -
Collections.synchronizedMap(new HashMap(...))
...
Thanks

On 6/8/07, MenTaLguY <mental@rydia.net> wrote:
> On Sat, 9 Jun 2007 06:54:04 +0900, Nasir Khan <rubylearner@gmail.com> wrote:
> > There is no big picture.
>
> Since you had asked for feedback on anything you'd missed, I was trying to find out if was some mitigating factor in the specific way your program worked (the one for which you originally wrote this code), before telling you that it won't work.
>
> But -- unless the objects the attributes were set to were never modified, even if they didn't have the thread safety problems you already noted, the accessors generated still couldn't ensure thread safety. If thread 1 calls obj.some_accessor.foo, and thread 2 calls obj.some_accessor.bar (where #bar is some mutating method), some_accessor being synchronized simply _will not_ protect you.
>
> -mental
>
>
>

Nasir Khan

6/9/2007 5:37:00 AM

Actually facets/more/synchash.rb does what I was looking for hash.

Thanks

On 6/8/07, Nasir Khan <rubylearner@gmail.com> wrote:
> Yeah I see what you guys mean...I was a little delusional. Now I
> realize that I was looking for basically something equivalent to
> Java's -
> Collections.synchronizedMap(new HashMap(...))
> ...
> Thanks
>
>
>
>
> On 6/8/07, MenTaLguY <mental@rydia.net> wrote:
> > On Sat, 9 Jun 2007 06:54:04 +0900, Nasir Khan <rubylearner@gmail.com> wrote:
> > > There is no big picture.
> >
> > Since you had asked for feedback on anything you'd missed, I was trying to find out if was some mitigating factor in the specific way your program worked (the one for which you originally wrote this code), before telling you that it won't work.
> >
> > But -- unless the objects the attributes were set to were never modified, even if they didn't have the thread safety problems you already noted, the accessors generated still couldn't ensure thread safety. If thread 1 calls obj.some_accessor.foo, and thread 2 calls obj.some_accessor.bar (where #bar is some mutating method), some_accessor being synchronized simply _will not_ protect you.
> >
> > -mental
> >
> >
> >
>

Caleb Clausen

6/10/2007 8:13:00 PM

Nasir Khan wrote:
> define_method(meth) do |*args|
> @__ms_lock = Mutex.new unless @__ms_lock
> @__ms_lock.synchronize do
> self.send("__nonsync_#{meth}",*args)
> end
> end

This still isn't completely thread-safe, I'm afraid. If the lock
doesn't exist yet and two threads both call some synchronized method
on the same object, you've got a race condition. I'd suggest creating
the lock upfront, before the synchronization wrappers are defined....
unfortunately, that makes implementing this idea a bit more
complicated.

Caleb Clausen

6/11/2007 6:32:00 AM

On 6/10/07, Nasir Khan <rubylearner@gmail.com> wrote:
> Thanks for the feedback. Here is a refinement -

I think there's still a race condition... you need to change this:

@@__ms_c_lock.synchronize { @__ms_lock = Mutex.new unless @__ms_lock
} unless @__ms_lock

To this:

@@__ms_c_lock.synchronize { @__ms_lock = Mutex.new unless @__ms_lock
unless @__ms_lock }

Which is somewhat uglier, since now the class level mutex has to be
checked on every synchronized method call.

Now, what I had in mind was a little more like this (INCOMPLETELY TESTED):

module MethodSynchronizer

def MethodSynchronizer.included(into)
into.sync_methods.each do |m|
MethodSynchronizer.wrap_method(into, m)
end
end

def MethodSynchronizer.wrap_method(klass, meth)
klass.class_eval do
alias_method "__nonsync_#{meth}", "#{meth}"
require 'thread'
define_method(:initialize_synchronizer) do
@__ms_lock=Mutex.new
return self
end
define_method(meth) do |*args|
@__ms_lock.synchronize do
self.send("__nonsync_#{meth}",*args)
end
end
end
end
end

And now you have to make sure that #initialize_synchronizer is called
on the object before any of the synchronized methods.

(What I really was thinking of was hacking into the regular
#initialize to get it to do the extra initialization automatically...
That can be hairy; among other things, you have to get it into the
#initialize of the class being mixed-in to, rather than the module's
#initialize. It's not impossible, but it requires more code than I
want to write right now.)

comp.lang.ruby

Synchronized attr_accessor

Nasir Khan

MenTaLguY

Nasir Khan

MenTaLguY

Nasir Khan

Robert Dober

MenTaLguY

Nasir Khan

Nasir Khan

Caleb Clausen

Caleb Clausen

x Login to ForumsZone