Spring Service instance variable in a multi-threaded environment

Question

I recently ran into a race condition issue because of declaring an instance variable inside a default scoped (Singleton scope) Service class. The purpose of the instance variable was to make my code more readable and avoid the constant passing of the same variable to different private methods within the Service class. The example goes:

@Service
public class SomeServiceImpl implements SomeService {
    private final StatusRepository statusRepository;
    private Predicate<Status> statusPredicate;

    @Autowired
    public SomeServiceImpl(StatusRepository statusRepository) {
        this.statusRepository = statusRepository;
    }

    @Override
    public List<Status> getAllowedStatuses(String userId) {
        statuses = statusRepository.getAll();
        initPredicate();
        appendPredicateA();
        appendPredicateB();
        List<Status> results = statuses.stream()
            .filter(statusPredicate)
            .collect(Collectors.toList());
        return results;
    }

    private void initPredicate() {
        statusPredicate = p -> p.getDefault().equals("default");
    }

    private void appendPredicateA() {
        statusPredicate.and(p -> p.getA().equals("A"));
    }

    private void appendPredicateB() {
        statusPredicate.and(p -> p.getB().equals("B"));
    }
}

This is a very simple example of the kind of things I want to achieve. This is clearly not thread-safe because now the service class is stateful. I could simply resolve this by turning the statusPredicate variable into a local variable and have the void methods return the predicate after it has been appended new conditions, but that would become cluttered like this:

@Override
public List<Status> getAllowedStatuses(String userId) {
    statuses = statusRepository.getAll();
    Predicate<Status> statusPredicate = p -> p.getDefault().equals("default");
    statusPredicate = appendPredicateA(statusPredicate);
    statusPredicate = appendPredicateB(statusPredicate);
    List<Status> results = statuses.stream()
        .filter(statusPredicate)
        .collect(Collectors.toList());
    return results;
}

It'd be constantly calling to modify the variable and return the variable.

I know a few solutions that can resolve this such as adding @RequestScope on the Service class to ensure each request from the HTTP will get a new instance of the Service object, or use ThreadLocal on the Predicate variable. However, I'm not quite certain what is the best approach and whether declaring an instance variable in a Service class is even okay to begin with. If it is bad to make Service class stateful to begin with, how should I structure my code to make it cleaner and still keeping it stateless?

Please advise! Thanks in advance :D

Answer 1

Spring Service can be stateful and this is why scopes for.

Default scope of Spring Service is Singleton because this is the widest use case. But you shouldn't use mutable local variables.

Simply, try for a stateless design and use scopes to solve the issue only if the class instantiation is fast otherwise TreadLocal will perform better.