Best practice for updating/writing to static variable?

Question

I have a project which displays department documentation. I store all the docs (get from database) in a static arrayList. Every X hour, I have that arrayList rebuilt based on the new doc (if any) from the database. There is also a static variable to control to rebuild that array or not, set and unset in a method which does the rebuild task. Each web browser hit the server will create this class's instance, but the doc arrayList and that control variable is shared among all the class instances.

Find-Bugs tool complains that "Write to static field someArrayName and someVariableName from instance method someClassMethod". Seems this is not the good thing to do (let class instance method write to static field). Does anyone have good recommendation how to get around this problem ? Thanks.

Answer 1

Per the FindBugs bug descriptions:

ST: Write to static field from instance method (ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD)

This instance method writes to a static field. This is tricky to get correct if multiple instances are being manipulated, and generally bad practice.

Aside from the concurrency issues, it means that all of the instances in the JVM are accessing the same data, and would not allow two separate groups of instances. It would be better if you had a singleton "manager" object and passed it to each of the instances as a constructor parameter or at least as a setManager() method argument.

As for the concurrency issues: if you must use a static field, your static field should be final; explicit synchronization is difficult. (There are also some tricky aspects if you are initializing non-final static fields, beyond my knowledge of Java but I think I've seen them in the Java Puzzlers book.) There are at least three ways of dealing with this (warning, untested code follows, check first before using):

1. Use a thread-safe collection, e.g. Collections.synchronizedList wrapped around a list that is not accessed in any other way.

   static final List<Item> items = createThreadSafeCollection();
   
   
   static List<Item> createThreadSafeCollection()
   {
      return Collections.synchronizedList(new ArrayList());
   }
   

   and then later when you are replacing this collection, from an instance:

   List<Item> newItems = getNewListFromSomewhere();
   items.clear();
   items.add(newItems);
   

   The problem with this is that if two instances are doing this sequence at the same time, you could get:

   Instance1: items.clear(); Instance2: items.clear(); Instance1: items.addAll(newItems); Instance2: items.addAll(newItems);

   and get a list that doesn't meet the desired class invariant, namely that you have two groups of newItems in the static list. So this method doesn't work if you are clearing the entire list as one step, and adding items as a second step. (If your instances just need to add an item, though, items.add(newItem) would be safe to use from each instance.)

2. Synchronize access to the collection.

   You'll need an explicit mechanism for synchronizing here. Synchronized methods won't work because they synchronize on "this", which is not common between the instances. You could use:

   static final private Object lock = new Object();
   static volatile private List<Item> list;
   // technically "list" doesn't need to be final if you
   // make sure you synchronize properly around unit operations.
   
   
   static void setList(List<Item> newList)
   {
     synchronized(lock)
     {
         list = newList;
     }
   }
   

3. use AtomicReference

   static final private AtomicReference<List<Item>> list;
   
   
   static void setList(List<Item> newList)
   {
     list.set(newList);
   }