Path: csiph.com!x330-a1.tempe.blueboxinc.net!usenet.pasdenom.info!news.albasani.net!eternal-september.org!feeder.eternal-september.org!mx04.eternal-september.org!.POSTED!not-for-mail
From: Eric Sosman <esosman@ieee-dot-org.invalid>
Newsgroups: comp.lang.java.programmer
Subject: Re: Question whether a problem with race conditions exists in this case
Date: Wed, 14 Dec 2011 16:26:16 -0500
Organization: A noiseless patient Spider
Lines: 88
Message-ID: <jcb49r$350$1@dont-email.me>
References: <8bbbbee3-adcc-4f28-aff5-2e230b047401@u6g2000vbg.googlegroups.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Injection-Date: Wed, 14 Dec 2011 21:26:19 +0000 (UTC)
Injection-Info: mx04.eternal-september.org; posting-host="HSlJAUb3pGXi3i7ZL/HoAw"; logging-data="3232"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX19htM7mVn6TMzebvNcDSdyG"
User-Agent: Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20111105 Thunderbird/8.0
In-Reply-To: <8bbbbee3-adcc-4f28-aff5-2e230b047401@u6g2000vbg.googlegroups.com>
Cancel-Lock: sha1:0EpKgdlbb10PVlqu84/t15ZoRmM=
Xref: x330-a1.tempe.blueboxinc.net comp.lang.java.programmer: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