Updating an active collections on a scheduler

Question

I think this might an easy thing but wanted to know if there is a pattern for this or an elegant way. I took a look at Guava. I have a class level List, a method that refers to this listOfObjects on scheduler, and a method that updates it on scheduler. The updater method collects all Objects that should be in this list and has a new list ready to reinitilize listOfObjects. However should I just set it even if it is being used by the method that is referring to it, or is there a more safe way to do it.

private List<Object> listOfObjects = new ArrayList<Object>();

@Scheduled
public referToList(){
    for(Object o : listOfObjects){
        doSomething(o);
    }
}

@Scheduled
public updateList(){
    List<Object> tempList = new ArrayList<Object>();
    tempList = doSomethingToPopulateList();
    this.listOfObjects = tempList;
}

So its possible updateList can be updating list when referToList can be in the middle of iterating through. I can create a temp List in referToList as well so its working on a copy of listOfObjects but not sure how efficient that is

score -2 · Answer 1 · answered Jul 14 '16 at 23:12

tl;dr - your code is safe, but could be made more resilient to future bugs.

First, while not immediately necessary, you should tend to prefer immutable collections rather than mutable ones, especially when dealing with concurrency. It's good practice to do this so that to prevent unwanted modifications and so that you can reason much more easily about the state of the list. Your code isn't broken now, it would be if some other thread directly modified listOfObjects while it was being iterated over, so making it immutable prevents that.

In other words, when assigning to listOfObjects wrap the list with Collections.unmodifableList(tempList) or Guava's ImmutableList.copyOf(tempList) (the difference being that Guava's ImmutableList actually makes a copy of the list items, whereas unmodifableList just makes an immutable view - meaning that changes to the underlying list would still appear). Or just return an immutable list from doSomethingToPopulateList().

But to answer your question, you're right not to modify it directly and instead update the listOfObjects when you've collected the new list into tempList. This won't cause a problem even if referToList() is currently executing. That's because listOfObjects is a reference. When you run

for(Object o : listOfObjects){
    doSomething(o);
}

the JVM takes the memory reference listOfObjects is referring to at the point the for loop starts, and iterates over the items in that memory space. While that iteration is happening, your other code can update what memory reference listOfObjects is pointing to (by reassigning it in updateList()), but that won't affect the progress of your running for loop.

Good points, but the code is *not* safe. The list is not safely published from the `updateList` method; the thread that executes `referToList` may see the list in an inconsistent state. Unless I'm missing something and the same thread is guaranteed to execute both methods. — dnault, Jul 14 '16 at 23:29
Assigning a variable is an atomic operation, right? What inconsistent state would be seen here? — Rik, Jul 16 '16 at 00:40
Yes, variable assignment in Java is always atomic. But visibility is a separate issue -- without safe publication it's possible that Thread A will *never* see an assignment by Thread B. Or worse, it might see the assignment of the reference but see the referred-to object in a partially-constructed state. See [this SO question](http://stackoverflow.com/questions/801993/java-multi-threading-safe-publication) and [this article by Brian Goetz](http://www.ibm.com/developerworks/library/j-jtp0618/) (primarily about safe *construction*, but the section titled "Visibility Hazards" also applies here). — dnault, Jul 18 '16 at 18:32

Updating an active collections on a scheduler

1 Answers1