Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.java.programmer > #10728 > unrolled thread

Question whether a problem with race conditions exists in this case

Back to article view | Back to comp.lang.java.programmer

Contents

  Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-14 09:07 -0800
    Re: Question whether a problem with race conditions exists in this case Andreas Leitgeb <avl@gamma.logic.tuwien.ac.at> - 2011-12-14 17:53 +0000
      Re: Question whether a problem with race conditions exists in this case Lew <lewbloch@gmail.com> - 2011-12-14 10:44 -0800
        Re: Question whether a problem with race conditions exists in this case Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2011-12-14 10:54 -0800
        Re: Question whether a problem with race conditions exists in this case Andreas Leitgeb <avl@gamma.logic.tuwien.ac.at> - 2011-12-14 21:15 +0000
          Re: Question whether a problem with race conditions exists in this case Tom Anderson <twic@urchin.earth.li> - 2011-12-15 14:58 +0000
            Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-15 08:40 -0800
            Re: Question whether a problem with race conditions exists in this case Gene Wirchenko <genew@ocis.net> - 2011-12-15 11:55 -0800
              Re: Question whether a problem with race conditions exists in this case Tom Anderson <twic@urchin.earth.li> - 2011-12-16 14:06 +0000
                Re: Question whether a problem with race conditions exists in this case Gene Wirchenko <genew@ocis.net> - 2011-12-16 10:09 -0800
      Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-14 11:47 -0800
    Re: Question whether a problem with race conditions exists in this case Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2011-12-14 10:53 -0800
      Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-14 11:54 -0800
        Re: Question whether a problem with race conditions exists in this case Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2011-12-15 16:38 -0800
          Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-15 23:01 -0800
            Re: Question whether a problem with race conditions exists in this case Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2011-12-16 09:34 -0800
              Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-17 11:55 -0800
                Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-19 05:33 -0800
    Re: Question whether a problem with race conditions exists in this case Lew <lewbloch@gmail.com> - 2011-12-14 11:04 -0800
      Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-14 12:32 -0800
        Re: Question whether a problem with race conditions exists in this case markspace <-@.> - 2011-12-14 14:13 -0800
          Re: Question whether a problem with race conditions exists in this case Eric Sosman <esosman@ieee-dot-org.invalid> - 2011-12-14 17:44 -0500
            Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-14 14:50 -0800
              Re: Question whether a problem with race conditions exists in this case markspace <-@.> - 2011-12-14 15:26 -0800
        Re: Question whether a problem with race conditions exists in this case Lew <lewbloch@gmail.com> - 2011-12-15 01:34 -0800
      Re: Question whether a problem with race conditions exists in this case Andreas Leitgeb <avl@gamma.logic.tuwien.ac.at> - 2011-12-14 21:38 +0000
    Re: Question whether a problem with race conditions exists in this case Eric Sosman <esosman@ieee-dot-org.invalid> - 2011-12-14 16:26 -0500
      Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-14 13:57 -0800
        Re: Question whether a problem with race conditions exists in this case Eric Sosman <esosman@ieee-dot-org.invalid> - 2011-12-14 18:05 -0500
          Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-14 23:25 -0800
            Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-14 23:28 -0800
        Re: Question whether a problem with race conditions exists in this case Tom Anderson <twic@urchin.earth.li> - 2011-12-15 14:44 +0000
          Re: Question whether a problem with race conditions exists in this case Lew <lewbloch@gmail.com> - 2011-12-16 08:27 -0800
            Re: Question whether a problem with race conditions exists in this case Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2011-12-16 09:41 -0800
              Re: Question whether a problem with race conditions exists in this case Tom Anderson <twic@urchin.earth.li> - 2011-12-16 18:51 +0000
            Re: Question whether a problem with race conditions exists in this case Tom Anderson <twic@urchin.earth.li> - 2011-12-16 18:50 +0000
      Re: Question whether a problem with race conditions exists in this case Saxo <saxo123@gmx.de> - 2011-12-14 14:13 -0800
        Re: Question whether a problem with race conditions exists in this case Lew <lewbloch@gmail.com> - 2011-12-14 16:55 -0800

Page 2 of 2 — ← Prev page 1 [2]

#10750

On 12/14/2011 12:32 PM, Saxo wrote:

> This way the single AtomicBoolean object is passed that will be
> changed from false to true by reference, so that all new values
> become visible for any thread calling aNode.get() "at once".
>

But there isn't a single AtomicBoolean, and it isn't passed in.  It's 
assigned as an instance variable initialization.  There's one 
AtomicBoolean per Node object.

This is really important, because it makes your whole code wrong.  If 
you want to pass something in, then pass it in.

public class Node {
   private final AtomicBoolean useNewValue;

   public Node( AtomicBoolean commonFlag ) {
     useNewValue = commonFlag;
   }

  ...

}

[toc] | [prev] | [next] | [standalone]

#10754

On 12/14/2011 5:13 PM, markspace wrote:
> On 12/14/2011 12:32 PM, Saxo wrote:
>
>> This way the single AtomicBoolean object is passed that will be
>> changed from false to true by reference, so that all new values
>> become visible for any thread calling aNode.get() "at once".
>>
>
> But there isn't a single AtomicBoolean, and it isn't passed in. It's
> assigned as an instance variable initialization. There's one
> AtomicBoolean per Node object.

     The instance field gets changed in the setNewValue() call.
The original value is only there, as far as I can see, to cough
up a "false" until it's replaced.

     I still wish I knew what he's trying to do, though.

-- 
Eric Sosman
esosman@ieee-dot-org.invalid

[toc] | [prev] | [next] | [standalone]

#10755

On Dec 14, 11:44 pm, Eric Sosman <esos...@ieee-dot-org.invalid> wrote:
> On 12/14/2011 5:13 PM, markspace wrote:
>
> > On 12/14/2011 12:32 PM, Saxo wrote:
>
> >> This way the single AtomicBoolean object is passed that will be
> >> changed from false to true by reference, so that all new values
> >> become visible for any thread calling aNode.get() "at once".
>
> > But there isn't a single AtomicBoolean, and it isn't passed in. It's
> > assigned as an instance variable initialization. There's one
> > AtomicBoolean per Node object.
>
>      The instance field gets changed in the setNewValue() call.
> The original value is only there, as far as I can see, to cough
> up a "false" until it's replaced.

Yes, right :-).

>      I still wish I knew what he's trying to do, though.

I just posted two replies to your last post to answer your questions.
You might have missed those due to a race condition ;-)). Have to go
to bed now, though. The sun has set some hours ago over here ...

Cheers, Oliver

[toc] | [prev] | [next] | [standalone]

#10757

On 12/14/2011 2:50 PM, Saxo wrote:
> On Dec 14, 11:44 pm, Eric Sosman<esos...@ieee-dot-org.invalid>  wrote:
>>       The instance field gets changed in the setNewValue() call.
>> The original value is only there, as far as I can see, to cough
>> up a "false" until it's replaced.
>
> Yes, right :-).


That's not really good enough.  If the instance value exists for any 
time, then there's a chance that a thread will come in before you have a 
chance to use it.  It's a big trap for anyone who didn't design the 
code.  My way was much more clear and also bullet proof.

[toc] | [prev] | [next] | [standalone]

#10766

Saxo wrote:
> Lew wrote:
> > *Think* about what you're doing.  What references have you checked to improve your ability to reason about concurrent programming?
> >
> > Seriously, which ones have you read?
> >
> > I really am asking.
> 
> This is really funny and made me laugh. Thanks :-).

Why are you wasting your and my time on this board?  

Why do you even bother asking for help here?

-- 
Lew

[toc] | [prev] | [next] | [standalone]

#10749

Lew <lewbloch@gmail.com> wrote:
> Saxo wrote:
>> I have a class Node as displayed below that holds a new value and the
> First, good job with your SSCCE (http://sscce.org/).  That was a well thought-
> out and complete job.  

But it seems like it was futile, as at least two regulars didn't understand
the reason behind the use of AtomicBoolean, despite the sccee.

> Second, _never_ use TAB characters in code posts except in literals.

Why even "except in literals"?  There's \t for (String-)literals.

>> public class Node {
>>   AtomicBoolean useNewValue = new AtomicBoolean(false);
> A 'volatile boolean' should do here.  Actually, a non-'volatile' variable will be just fine:
>     boolean useNewValue;

The initialization is probably only to avoid null-issues
before first setNewValue. After that, there's only *one*
AtomicBoolean instance for all relevant Node instances.

> Why not use the instance itself as the lock?

That, however, is a good question...

[toc] | [prev] | [next] | [standalone]

#10745

On 12/14/2011 12:07 PM, Saxo wrote:
> I have a class Node as displayed below that holds a new value and the
> previous value. Which one applies is determined by the value of
> useNewValue. In my application there is a list of such nodes where for
> each node the current value is replaced with a new one. The new values
> should become effective for the entire list all>at once<  through an
> atomic change of useNewValue in every node. For that purpose, when the
> new value is set, for every node in the list the same AtomicBoolean
> instance is passed on to setNewValue(...), which is stored in
> useNewValueParam. When the commit is done, the value of this instance
> of AtomicBoolean is changed to true (this way the thread doing the
> commit does not have to enter the synchronized block as all the other
> threads calling aNode.get) and thus the new value of every node
> becomes visible at once to every thread calling aNode.get().
>
> public class Node {
>
> 	AtomicBoolean useNewValue = new AtomicBoolean(false);

     Why isn't this `private'?  Is something else going on that
you haven't told us about?

> 	private Object newValue = null;
> 	private Object previousValue = null;
> 	private Object lock = new Object();

     What does `lock' buy you?  Why not just synchronize on the
Node itself?

> 	public Object get() {
> 		synchronized(lock) {
> 			if(useNewValue.get())		// 1
> 				return newValue;	// 2
> 			return previousValue;		// 3
> 		}
> 	}
>
> 	public void setNewValue(Object newValue, AtomicBoolean
> useNewValueParam) {
> 		synchronized(lock) {
> 			if(this.useNewValue.get())
> 				previousValue = this.newValue;
> 			this.newValue = newValue;
> 			// useNewValueParam is allways set to false when setNewValue is
> called

     I don't see why that would matter.

> 			this.useNewValue = useNewValueParam;
> 		}
> 	}
> }
>
> At the same time there is never more than one thread iterating over
> the node list calling setNewValue.

     So `synchronized' is just to ensure that the single setNewValue()
caller doesn't overlap any get() callers, is that right?  (Nothing
wrong with that; I'm just trying to test my understanding of what
you're up to.)

> This is made sure through
> serialization of threads that want to iterate over the list calling
> setNewValue. Serialization of threads is a bit crude, but not relevant
> at the moment for the discussion of the problem described here.
>
> My question is now whether this approach is free of race conditions or
> starvation issues if implemented as described above. I have some
> doubts whether everything is fine here as useNewValue is changed by
> the commit thread by reference without entering synchronized(lock)
> { ... }. So is everything still fine if a context switch happens
> between line 1 and 2 or between line 1 and 3? It would be for sure if
> the commit thread entered the synchronized(lock) { ... } block, but it
> does not (changing to the new values all at once wouldn't be possible
> otherwise).

     When you change the shared AtomicBoolean, some threads executing
get() may have already observed the old value and may have decided
which value to return based on that now-outdated observation.  So yes,
there's a race: It is possible for get() to return an old value after
the AtomicBoolean changes from true to false, or a new value after it
changes from false to true.

     What's the larger problem you're trying to solve?

-- 
Eric Sosman
esosman@ieee-dot-org.invalid

[toc] | [prev] | [next] | [standalone]

#10751

On Dec 14, 10:26 pm, Eric Sosman <esos...@ieee-dot-org.invalid> wrote:

>
> > public class Node {
>
> >    AtomicBoolean useNewValue = new AtomicBoolean(false);
>
>      Why isn't this `private'?  Is something else going on that
> you haven't told us about?

I just forgot to set it private.

>
> >    private Object newValue = null;
> >    private Object previousValue = null;
> >    private Object lock = new Object();
>
>      What does `lock' buy you?  Why not just synchronize on the
> Node itself?

The purpose is only to indicate that some more fine-gtrained locking
would be used for the real thing instead of doing a synchronized(this)
{ ... } thing.

> >    public void setNewValue(Object newValue, AtomicBoolean
> > useNewValueParam) {
> >            synchronized(lock) {
> >                    if(this.useNewValue.get())
> >                            previousValue = this.newValue;
> >                    this.newValue = newValue;
> >                    // useNewValueParam is allways set to false when setNewValue is called
>
>      I don't see why that would matter.

You mean that comment "useNewValueParam is allways set to false when
setNewValue is called"? To make sure that after setting the new value
the previous value is still returned on a call to get() until
useNewValue is set to true. This is why

  if(this.useNewValue.get())
    previousValue = this.newValue;

is done to make sure that the previous value is still returned in case
useNewValue was true before it was replaced with the new AtomicBoolean
through

  this.useNewValue = useNewValueParam;

>      So `synchronized' is just to ensure that the single setNewValue()
> caller doesn't overlap any get() callers, is that right?  (Nothing
> wrong with that; I'm just trying to test my understanding of what
> you're up to.)

Yes, exactly.


>      What's the larger problem you're trying to solve?

Well, I'm basically thinking about how to implement a database commit.
I don't use a database. Instead, the data is on the heap in a simple
list. When the commit is done, all the new values in all the nodes in
my list become visible at once (by changing useNewValue as explained)
to any thread calling get(). This way I get around changing the values
in a big big synchronized block which would cause quite some lock
contention.

Also, if only the elements in my list at position 1 and 3 have
changed, I only need to call setNewValue on the nodes at these
positions. I don't have to lock the entire list. If the values for the
elements at position 2 and 4 have to be changed at the same time, the
commit could be done in parallel to increase throughput (because there
are no overlapping regions) instead of locking the entire list.

> --
> Eric Sosman
> esos...@ieee-dot-org.invalid

[toc] | [prev] | [next] | [standalone]

#10756

On 12/14/2011 4:57 PM, Saxo wrote:
> On Dec 14, 10:26 pm, Eric Sosman<esos...@ieee-dot-org.invalid>  wrote:
>> [...]
>>       What's the larger problem you're trying to solve?
>
> Well, I'm basically thinking about how to implement a database commit.
> I don't use a database. Instead, the data is on the heap in a simple
> list. When the commit is done, all the new values in all the nodes in
> my list become visible at once (by changing useNewValue as explained)
> to any thread calling get(). This way I get around changing the values
> in a big big synchronized block which would cause quite some lock
> contention.

     You'll still have the problem that threads T1 and T2 could get()
different values from the same Node, and both be working with those
conflicting values at the same time.  An "instantaneous" change will
not avoid the issue -- so either (a) it better not be an issue, or
(b) you need The Mother Of All Locks around the whole shebang ...

     Also, the problem ordinarily addressed by a "commit" mechanism
is somewhat different: You want to make changes to Nodes N1 and N2,
and you want to make sure an observer sees either the two old values
or the two new values, but never a mixture.  Again, "instantaneous"
change won't solve the problem, not even in the simple case of just
one mutator and one observer:

	Observer: Fetch the N1 value, then ...
	Mutator: CHANGE THE WORLD INSTANTANEOUSLY
	Observer: ... fetch the N2 value

For the observer's view of N1 and N2 to be consistent, it's not
enough that each get() be guarded against interference; you need
the pair of get()'s guarded as a unit.  Very likely, you also need
the changes to N1 and N2 guarded as a unit.

-- 
Eric Sosman
esosman@ieee-dot-org.invalid

[toc] | [prev] | [next] | [standalone]

#10762

On Dec 15, 12:05 am, Eric Sosman <esos...@ieee-dot-org.invalid> wrote:

>      You'll still have the problem that threads T1 and T2 could get()
> different values from the same Node, and both be working with those
> conflicting values at the same time.  An "instantaneous" change will
> not avoid the issue -- so either (a) it better not be an issue, or
> (b) you need The Mother Of All Locks around the whole shebang ...
>
>      Also, the problem ordinarily addressed by a "commit" mechanism
> is somewhat different: You want to make changes to Nodes N1 and N2,
> and you want to make sure an observer sees either the two old values
> or the two new values, but never a mixture.  Again, "instantaneous"
> change won't solve the problem, not even in the simple case of just
> one mutator and one observer:
>
>         Observer: Fetch the N1 value, then ...
>         Mutator: CHANGE THE WORLD INSTANTANEOUSLY
>         Observer: ... fetch the N2 value
>
> For the observer's view of N1 and N2 to be consistent, it's not
> enough that each get() be guarded against interference; you need
> the pair of get()'s guarded as a unit.  Very likely, you also need
> the changes to N1 and N2 guarded as a unit.
>
> Eric Sosman

All right, I see that the issue cannot be understood without
explaining mmore of the whole thing. What I want to develop is a
transactional concurrent map that is protected by an optimistic
locking mechanism. For every such map there is backing map that
carries the consistent snapshots from commit to commit. The user
interacts with the backing map through a locle cache:

BackingMap backingMap = new BackingMap();

LocalMap map1 = new LocalMap(backingMap);
map1.begin();
Node node1 = map1.get(key1);  // line x - this is the get from my
initial post, augmented by a key for the map value lookup
Node node2 = map1.get(key1);  // line y
node2.value = newValue;
Node node3 = map1.putAndGet(key1, node2);
map1.commit(); // or map1.mergeCommit();

The node objects hold a lock counter to determine whether an
optimistic locking conflict has occured. When map1.get(key) is done
the node object from the corresponding node object in the backing map
is copied and returned. It is then stored in the local cache with the
lock counter in the state when the original node from the backing map
was copied. When another time get is done as in line y, the same value
is returned as in line x even when it has meanwhile changed in the
backing map as on a repeated get for a node already in the local
cache, only the node in the local cache is returned. You can think of
adding a getRepeatableRead(key) method that refetches the current
value from the backing map with the current value of the lock counter.
But this is something for later considerations.

Until the commit is done all changes are held in a local change list.
When the commit is done, the >single< committing thread checks for
every change object in the change list whether the lock counter is
still the same as in the backing map. If so, the values of the change
list are transfered to the backing map as explained in my initial
post. If not, an optimistic locking exception is thrown and the local
change list is thrown away. In case of map1.commit() the commit fails
in case the backing map has changed in any way whether this concerns
the value for key1 or not. In case of map1.mergeCommit() the commit
only fails if the value for key1 has changed meanwhile from within a
different thread.

I now need to return to the get method of my initial post:

public Object get() {
  synchronized(lock) {
    if(useNewValue.get())     // 1
      return newValue;        // 2
    return previousValue;     // 3
  }
}

Let's say the map1.commit() is done as in the example at the beginning
of this post. During the commit phase some other thread x calls
get(key1). When for the commit no optimistic locking conflict has been
detected and the new values have been transfered from the change list,
the atomic boolean useNewValue is set to true. Let's say at this time
thread x were in line 3 (see line numbers in the example above). Then
the meanwhile outdated value previousValue would be returned. But
strictly speaking, chronologically thread x has by passed line 1 at
the time useNewValue was changed to true. So seen from this
perspective there is no true race condition here. Now, if we assume
that what is returned from the synchronized block must be inline with
the value in the backing map, there is a race condition. True. On the
other hand, the purpose of the synchronized block is only to make sure
that get() and setNewValue() are not called at the same time. When a
commit is done with the situation as described in this block, an
optimistic locking conflict would occur and the data would return
consistent. Now you can argue whether the optimistic locking exception
in this case is correct or should not have happened. In my opinion, in
the situation just described, thread x was at the critical line 1
>before< the commit thread changed useNewValue. From that perspective
chronological order was retained correctly.

Thinking about it, I'm still not sure whether newValue or
previousValue need to volatile. They were changed from within a
synchronized block and are returned from with a synchronized block.
Hm ...

Hope this helped a bit.
Regards, Oliver

[toc] | [prev] | [next] | [standalone]

#10763

And, yes ... I have no clue while the line width is that short on
every of my post. The window I type in the text is much wider. After
posting the whole thing every line is wrapped after a much shorter
number of chars per line. Wished I knew how to change this.

-- Oliver

[toc] | [prev] | [next] | [standalone]

#10775

[Multipart message — attachments visible in raw view] — view raw

On Wed, 14 Dec 2011, Saxo wrote:

> On Dec 14, 10:26 pm, Eric Sosman <esos...@ieee-dot-org.invalid> wrote:
>
>>>    private Object lock = new Object();
>>
>>      What does `lock' buy you?  Why not just synchronize on the
>> Node itself?
>
> The purpose is only to indicate that some more fine-gtrained locking
> would be used for the real thing instead of doing a synchronized(this)
> { ... } thing.

It's quite a common pattern. I'm always a bit dubious about using an 
public object (FSVO 'public') as the victim of a synchronized block; how 
do i know some random other bit of code in some other thread isn't going 
to try to lock the object at some point, and cause trouble? You wouldn't 
expose a field, would you? So why expose an object's lock? Essentially, i 
see an object's lock as a feature, like a method or a field; it should 
only be exposed to other classes after due consideration, and if it is, 
its proper use should be documented.

The locking analogue of a private field is an object like the above, 
created for the sole purpose of supplying a lock that is provably only 
accessible to code which can see the private details of the class.

I have coined the name 'lockguffin' for these objects, and i encourage you 
all to use it.

tom

-- 
And the future is certain, give us time to work it out

[toc] | [prev] | [next] | [standalone]

#10799

On Thursday, December 15, 2011 6:44:39 AM UTC-8, Tom Anderson wrote:
> On Wed, 14 Dec 2011, Saxo wrote:
> 
> > On Dec 14, 10:26 pm, Eric Sosman <esos...@ieee-dot-org.invalid> wrote:
> >
> >>>    private Object lock = new Object();
> >>
> >>      What does `lock' buy you?  Why not just synchronize on the
> >> Node itself?
> >
> > The purpose is only to indicate that some more fine-gtrained locking
> > would be used for the real thing instead of doing a synchronized(this)
> > { ... } thing.
> 
> It's quite a common pattern. I'm always a bit dubious about using an 
> public object (FSVO 'public') as the victim of a synchronized block; how 
> do i know some random other bit of code in some other thread isn't going 
> to try to lock the object at some point, and cause trouble? You wouldn't 
> expose a field, would you? So why expose an object's lock? Essentially, i 
> see an object's lock as a feature, like a method or a field; it should 
> only be exposed to other classes after due consideration, and if it is, 
> its proper use should be documented.

I control that by who sees the object, e.g., a 'Collections.synchronizedList()'.

I see the point in what you're saying but I find it over-cautious sometimes.
It depends on whether you want the object to control its own internal locking, 
which sometimes you do, or to be part of its client's thread control, as the numerous standard API classes with 'synchronized' methods do.

> The locking analogue of a private field is an object like the above, 
> created for the sole purpose of supplying a lock that is provably only 
> accessible to code which can see the private details of the class.
> 
> I have coined the name 'lockguffin' for these objects, and i encourage you 
> all to use it.

When appropriate.

-- 
Lew

[toc] | [prev] | [next] | [standalone]

#10802

On 12/16/11 8:27 AM, Lew wrote:
> On Thursday, December 15, 2011 6:44:39 AM UTC-8, Tom Anderson wrote:
>> On Wed, 14 Dec 2011, Saxo wrote:
>>
>>> On Dec 14, 10:26 pm, Eric Sosman<esos...@ieee-dot-org.invalid>  wrote:
>>>
>>>>>     private Object lock = new Object();
>>>>
>>>>       What does `lock' buy you?  Why not just synchronize on the
>>>> Node itself?
>>>
>>> The purpose is only to indicate that some more fine-gtrained locking
>>> would be used for the real thing instead of doing a synchronized(this)
>>> { ... } thing.
>>
>> It's quite a common pattern. I'm always a bit dubious about using an
>> public object (FSVO 'public') as the victim of a synchronized block; how
>> do i know some random other bit of code in some other thread isn't going
>> to try to lock the object at some point, and cause trouble? You wouldn't
>> expose a field, would you? So why expose an object's lock? Essentially, i
>> see an object's lock as a feature, like a method or a field; it should
>> only be exposed to other classes after due consideration, and if it is,
>> its proper use should be documented.
>
> I control that by who sees the object, e.g., a 'Collections.synchronizedList()'.
>
> I see the point in what you're saying but I find it over-cautious sometimes..
> It depends on whether you want the object to control its own internal locking,
> which sometimes you do, or to be part of its client's thread control, as the numerous standard API classes with 'synchronized' methods do.
I seem to recall this practice of using an internal lock object being 
advocated in JCIP, but I'm not near my bookshelf at the moment so I 
can't be sure.

I happen to agree with Eric on this one though. If you expose any method 
as "synchronized" (whether it is public or not), then you have added an 
implicit contract to your object that you may not have intended.  That 
contract is "If *any* external process synchronizes on this object, it 
will be serialized with certain operations I perform."

More often than not, you will want to be explicit about behavior in 
concurrent situations, not implicit.
>
>> The locking analogue of a private field is an object like the above,
>> created for the sole purpose of supplying a lock that is provably only
>> accessible to code which can see the private details of the class.
>>
>> I have coined the name 'lockguffin' for these objects, and i encourage you
>> all to use it.
>
> When appropriate.
"When appropriate" should be the default modifier for all statements, 
because you shouldn't ever do something when its not appropriate :-)

[toc] | [prev] | [next] | [standalone]

#10807

On Fri, 16 Dec 2011, Daniel Pitts wrote:

> On 12/16/11 8:27 AM, Lew wrote:
>> On Thursday, December 15, 2011 6:44:39 AM UTC-8, Tom Anderson wrote:
>
>>> I have coined the name 'lockguffin' for these objects, and i encourage 
>>> you all to use it.
>> 
>> When appropriate.
>
> "When appropriate" should be the default modifier for all statements, 
> because you shouldn't ever do something when its not appropriate :-)

that doesn't sound like any fun at all.

tom

-- 
Get a fucking hobby that isn't breathing, browsing 4chan, or fapping. --
The Well Cultured Anonymous, on Manners

[toc] | [prev] | [next] | [standalone]

#10806

[Multipart message — attachments visible in raw view] — view raw

On Fri, 16 Dec 2011, Lew wrote:

> On Thursday, December 15, 2011 6:44:39 AM UTC-8, Tom Anderson wrote:
>> On Wed, 14 Dec 2011, Saxo wrote:
>>
>>> On Dec 14, 10:26 pm, Eric Sosman <esos...@ieee-dot-org.invalid> wrote:
>>>
>>>>>    private Object lock = new Object();
>>>>
>>>>      What does `lock' buy you?  Why not just synchronize on the
>>>> Node itself?
>>>
>>> The purpose is only to indicate that some more fine-gtrained locking
>>> would be used for the real thing instead of doing a synchronized(this)
>>> { ... } thing.
>>
>> It's quite a common pattern. I'm always a bit dubious about using an
>> public object (FSVO 'public') as the victim of a synchronized block; how
>> do i know some random other bit of code in some other thread isn't going
>> to try to lock the object at some point, and cause trouble? You wouldn't
>> expose a field, would you? So why expose an object's lock? Essentially, i
>> see an object's lock as a feature, like a method or a field; it should
>> only be exposed to other classes after due consideration, and if it is,
>> its proper use should be documented.
>
> I control that by who sees the object, e.g., a 'Collections.synchronizedList()'.

That's why i said "public object (FSVO 'public')" - there are times when 
you can wrap your objects like that, but others when it doesn't make 
sense, because of the extra complexity it introduces.

Wrapping is definitely a powerful tool here, though, very good point. 
Wrapping is, generally, a way of creating a sort of neutral zone; things 
that are behind the wrapper can be public without actually being very 
exposed. You control their effective scope by controlling the availability 
of the reference.

> I see the point in what you're saying but I find it over-cautious sometimes.

Fair point. I suppose this is something you can do if you want to be able 
to make strong assumptions about when the object is locked. You don't 
always need that.

> It depends on whether you want the object to control its own internal 
> locking, which sometimes you do, or to be part of its client's thread 
> control, as the numerous standard API classes with 'synchronized' 
> methods do.

Yes, quite true. This is what i was getting at with "an object's lock 
[...] should only be exposed to other classes after due consideration, and 
if it is, its proper use should be documented". Making it part of the 
client's thread control is a valid thing to do, but it should be a 
considered act.

>> The locking analogue of a private field is an object like the above,
>> created for the sole purpose of supplying a lock that is provably only
>> accessible to code which can see the private details of the class.
>>
>> I have coined the name 'lockguffin' for these objects, and i encourage you
>> all to use it.
>
> When appropriate.

Oh, sorry, i meant that i encourage you to use the name, not the pattern! 
The name is always appropriate; the pattern certainly might not be.

tom

-- 
Get a fucking hobby that isn't breathing, browsing 4chan, or fapping. --
The Well Cultured Anonymous, on Manners

[toc] | [prev] | [next] | [standalone]

#10753

>      When you change the shared AtomicBoolean, some threads executing
> get() may have already observed the old value and may have decided
> which value to return based on that now-outdated observation.  So yes,
> there's a race: It is possible for get() to return an old value after
> the AtomicBoolean changes from true to false, or a new value after it
> changes from false to true.

Oh shit, of course! I have thought about it. First though was to call

if(useNewValue.get()) {
    // ...
}

that often till it returns the same value. But useNewValue could
change just thereafter. So you would have to call this infinitely
often till you are sure you get the right value. Obviously, this
doesn't work. Then I forgot about the issue.

Thanks for the hint. Now I have to think how to solve this ...

Chhers, Oliver

[toc] | [prev] | [next] | [standalone]

#10760

Saxo wrote:
>>      When you change the shared AtomicBoolean, some threads executing
>> get() may have already observed the old value and may have decided
>> which value to return based on that now-outdated observation.  So yes,
>> there's a race: It is possible for get() to return an old value after
>> the AtomicBoolean changes from true to false, or a new value after it
>> changes from false to true.
> 
> Oh shit, of course! I have thought about it. First though was to call
> 
> if(useNewValue.get()) {
>     // ...
> }
> 
> that often till it returns the same value. But useNewValue could
> change just thereafter. So you would have to call this infinitely
> often till you are sure you get the right value. Obviously, this
> doesn't work. Then I forgot about the issue.
> 
> Thanks for the hint. Now I have to think how to solve this ...

Bearing in mind that you don't need, and shouldn't use, an 'AtomicBoolean' for
this in the first place.

-- 
Lew

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | comp.lang.java.programmer

csiph-web