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

Groups > comp.lang.java.programmer > #10735

Re: Question whether a problem with race conditions exists in this case

Show all headers | View raw

On 12/14/11 9:07 AM, Saxo wrote:
> I have a class Node as displayed below that holds a new value and the
> previous value.
[snip]
>
> public class Node {
>
> 	AtomicBoolean useNewValue = new AtomicBoolean(false);
> 	private Object newValue = null;
> 	private Object previousValue = null;
> 	private Object lock = new Object();
>
> 	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
If it is always false, why pass it in at all? I don't believe this comment.
> 			this.useNewValue = useNewValueParam;
> 		}
> 	}
> }
Don't use tabs on usenet, it makes formatting bonkers.  Use 2 to 4 
spaces for indenting.

Since you have a lock here, you don't need to use an AtomicBoolean. 
Even so, you're not using the AtomicBoolean as expected.  The field for 
it should be final, and you call "set(value)" on it, rather than replace 
it with another AtomicBoolean instance. That defeats the purpose.

>
> At the same time there is never more than one thread iterating over
> the node list calling setNewValue. 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.
The threads are all serialized because of the synchronize(lock).
>
> 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).
synchronized(lock) prevents more than one thread from being inside *any* 
synchronized(lock) block at *any* given time, so you will never have a 
setter and getter running at the same time.  This code will "work", but 
is still not great quality.
>
> I would be thankful if someone could look over this and tell me her/
> his opinion.
Unrelated, it might be useful to use Generics here.  I don't know much 
of your use-case, but I'll demonstrate how it might be used. There are 
actually two ways I would suggest doing this, depending on the frequency 
of sets vs gets.  If they are about equal, or sets are more frequent, I 
suggest this class:

public class Node<V> {
    private boolean useNew;
    private V previousValue;
    private V value;
    private final Object lock = new Object();

    public V getValue() {
       synchronize(lock) {
          return useNew ? value : previousValue;
       }
    }

    public void setValue(V value, boolean useNew) {
       synchronize(lock) {
          previousValue = getValue();
          this.value = value;
          this.useNew = useNew;
       }
    }
}

If the sets are somewhat infrequent, I suggest the following class. It 
will allow much faster access by multiple threads for getValue(). It 
does have a small amount of overhead when setting the value.

public class Node<V> {
    private volatile State<V> state = new State<V>(null, null, true);
    private final Object setLock = new Object();

    public V getValue() {
       // volatile allows us to safely read a value.
       return state.getValue();
    }

    public V setValue(V newValue, boolean useNew) {
       // we need to serialize access here, otherwise we might get the
       // wrong previous value.
       synchronized(setLock) {
           state = new State<V>(state.value, newValue, useNew);
       }
    }

    // Note, this <V> is independent of the <V> in Node.
    private static class State<V> {
       private final V previousValue;
       private final V newValue;
       private volatile boolean V useNew;
       public State(V previousValue, V newValue, boolean useNew) {
          this.previousValue = previousValue;
          this.newValue = newValue;
          this.useNew = useNew;
       }
       public V getValue() {
          return useNew ? newValue : previousValue;
       }
    }
}

Back to comp.lang.java.programmer | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

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

csiph-web