Changing a Java List Reference While Another Thread Reads It

Question

I couldn't find a clear answer to this question, so apologies if this is nicely answered elsewhere.

Imagine I have something roughly like:

Class SomeClass
{
  List<String> strings = new LinkedList<>();


  public void updateList(List<String> newStrings) {
    strings = newStrings;
  }

  public List<String> getMatchingStrings(String pattern) {
    List<String> matches = new LinkedList<>();
    for (String s : strings) {
      if (matchesPattern(s, pattern)) { //Assume this just works
        matches.add(s);
      }
    }
    return matches;
  }

}

Assuming another thread calls updateList(...) while some thread is calling getMatchingStrings(), will there be a "problem"?

The intended behavior is that the thread currently executing getMatchingStrings() will run on the old list, and a subsequent call will run on the updated list.

Any help is appreciated!

Answer 1

There is 'unsafe publication' in you code (no 'happens-before order' between write list field and read list field). Use blocked 'synchronized' or non-blocked 'volatile' or 'AtomicReference':

// with 'synchronized'
class SomeClass0 {
    List<String> strings = new LinkedList<>();

    public synchronized void updateList(List<String> newStrings) {
        this.strings = newStrings;
    }

    public synchronized List<String> getMatchingStrings(String pattern) {
        List<String> matches = new LinkedList<>();
        for (String s : strings) {
            if (matchesPattern(s, pattern)) { //Assume this just works
                matches.add(s);
            }
        }
        return matches;
    }
}

// with 'volatile'
class SomeClass1 {
    volatile List<String> strings = new LinkedList<>();

    public void updateList(List<String> newStrings) {
        this.strings = newStrings;
    }

    public List<String> getMatchingStrings(String pattern) {
        List<String> localCopy = this.strings;
        List<String> matches = new LinkedList<>();
        for (String s : localCopy) {
            if (matchesPattern(s, pattern)) { //Assume this just works
                matches.add(s);
            }
        }
        return matches;
    }
}

// with 'AtomicReference'
class SomeClass2 {
    AtomicReference<List<String>> strings = 
                        new AtomicReference<>(new LinkedList<>());

    public void updateList(List<String> newStrings) {
        this.strings.set(newStrings);
    }

    public List<String> getMatchingStrings(String pattern) {
        List<String> localCopy = this.strings.get();
        List<String> matches = new LinkedList<>();
        for (String s : localCopy) {
            if (matchesPattern(s, pattern)) { //Assume this just works
                matches.add(s);
            }
        }
        return matches;
    }
}

Answer 2

If you mean will there be a runtime exception, the answer is no. The code that iterates over strings will use whatever the reference happens to be when the iterator invokes.

It doesn't matter if the strings reference changes while iterating. Think of it like this,

Iterator<String> i = strings.iterator();
strings = new List<String>();
while (i.hasNext()) {
 ...
 i.next();
}

This code is fine. The object pointed to by strings hasn't changed. We only changed the strings reference to point to somewhere else. The iterator still points to the original object referenced by strings before the assignment.

EDIT: some comments below have said the original example is unsafe publication. I don't believe it is. Unsafe publication is about allowing thread B to access an object under construction in thread A- thread A is unsafely publishing the object to thread B.

However, in the example above, there's no object construction with regard to the fields in question. As far as we know both the lists were fully constructed in a thread-safe manner. There is only reference assignment. Object assignment in Java is atomic From the Java language spec,

"When a thread uses the value of a variable, the value it obtains is in fact a value stored into the variable by that thread or by some other thread. This is true even if the program does not contain code for proper synchronization. For example, if two threads store references to different objects into the same reference value, the variable will subsequently contain a reference to one object or the other, not a reference to some other object or a corrupted reference value. (There is a special exception for long and double values; see §17.4.)

Now, there might be unsafe publication in the OP's code if the object passed to updateList() wasn't constructed safely, but I'm not going to hypothesize what bugs might exist in code I can't see.

Answer 3

Jeffrey is correct- there is no unsafe publication here, because at no time is one thread constructing the object while another may read it (the instance is protected by the updateList method call). There is, however, significant likelihood of inconsistent access- e.g. the list you end up iterating may not be the list you expect, even if you call updateList first, because there is no explicit memory barrier. The JVM may do weird things at runtime in this situation.

Assuming that the list that ends up being iterated (again, this may not be the list that you expect) is not null, is not modified while iteration is occurring, is properly implemented, etc, etc, then this "pattern" will not cause exceptions. That said, it shouldn't be used- you need to perform synchronization and (probably) mark the local variable with volatile if this will be used in a multithreaded scenario.