Thread safety (or not) of Spring singletons using setter injection

Recently I have seen several instances, one of which would have caused very serious defects had it been released, of very experienced programmers creating code with thread safety issues due to lack of familiarity with the implications of the Spring singleton scope.

It is very common we create a class:

public class MySingleton {
    private int myVar;

    public void setMyVar(int myVar) {
      this.myVar = myVar;
    }

    public int getMyVar() {
      return myVar;
    }
  }

And configure it in Spring:

<bean class="com.example.spring.threading.MySingleton">
    <property name="myVar" value="7" />
  </bean>

So far so good... or is it? We've got a mutable instance variable on a class that is setup as a singleton. As soon as we configure it to be used by something that runs on multiple threads we've got a serious potential problem. Since the majority of our Spring applications are web applications they will very naturally tend to utilize some of these beans from request processing threads so we'll end up with concurrent access. Normally it is a non-issue: the property is only ever set through Spring so even though it is sitting there mutable we don't actually get any problems. However, this is not guaranteed by our code; it's essentially accidental that it works.

So, moving right along, our hapless developer - who is, btw, a Java expert though perhaps not a Spring expert yet - creates some instance variable and a method that can alter its value.

public class MySingleton {
    private int myVar;
    private List myList;

    public void setMyVar(int myVar) {
      this.myVar = myVar;
    }

    public int getMyVar() {
      return myVar;
    }
    
    public void doSomethingToMyList() {
      //...code that alters myList...
    }
  }

Looks fine, the type doesn't show any signs of needing to be threadsafe, right? This looks fine in unit testing, works fine when they try the software through a web browser, and then blows up (sometimes) when the software is used by concurrent users.

Since our singleton is probably really intended to be immutable (or would be if the developer stopped and thought it through) we could presumably set it up with final fields and an initialization constructor whose arguments are provided via Spring constructor injection. This is rather ugly in Spring due to the inability to provide named constructor-arg values (which is in turn due to lack of parameter name metadata in Java). Our developers strongly favor setter injection and the Spring docs on the subject seem to generally recommend this approach so for now we're left with nothing but code reviews between us and threading disaster.

This sucks. None of our current automated checks will notice. A code review should catch it but inevitably some will slip through. Possibly we can use annotations with @Resource on the field, as suggested in http://www.infoq.com/articles/spring-2.5-part-1, and provide only a getter. This is probably the most promising solution at present.