Is this class thread-safe?

Question

I have an entity class

    package org.demo.stack;

    import javax.persistence.Column;
    import javax.persistence.Entity;
    import javax.persistence.Id;

    @Entity
    public class ValueHolder{
        @Id
        private Long id;

        @Column
        private Long value;

        /* getters and setters... */
    }

a related Spring JPA Repository

    package org.demo.stack;

    import org.springframework.data.jpa.repository.JpaRepository;

    public interface ValueHolderRepository extends JpaRepository<ValueHolder, Long>
    {
    }

and a class that acts as an accumulator of some kind of values, using the repository (Please note that this is a simplified version of a real program)

    package org.demo.stack;

    import java.util.List;

    public class ValueAccumulator{
        private ValueHolderRepository repository;
        private Long accumulator;

        public ValueAccumulator(ValueHolderRepository repository){
            this.repository = repository;
            accumulator = 0L;
        }

        public Long sumOverResults(){
            accumulator = 0L;

            List<ValueHolder> holders = repository.findAll();
            for(ValueHolder holder : holders){
                accumulator = accumulator + holder.getValue();
            }
            return accumulator;
        }
    }

ValueAccumulator is clearly not thread-safe. If I share the same instance across different threads and sumOverResults is called concurrently, the instance variable accumulator is going to have a non predictable value.

I also have an adapter that looks like ConcurrentValueAccumulator:

    package org.demo.stack;

    public class ConcurrentValueAccumulator{
        private ValueHolderRepository repository;

        public Long sumOverResults(){
            ValueAccumulator oneShotAccumulator = new ValueAccumulator(repository);
            return oneShotAccumulator.sumOverResults();
        }
    }

Now each call to sumOverResults uses its own instance of ValueAccumulator, so no data are shared across threads. Under the assumption that no ValueEntitys are inserted or updated during the process, can I be 100% sure that ConcurrentValueAccumulator is thread-safe?

Answer 1

As per the code provided here, You are right, If we look the class ValueAccumulator seperately, then definitely its not a thread-safe(for the obvious reason which you have provided in the question).

Now If you are using ValueAccumulator class through ConcurrentValueAccumulator, then I have following observations :

1. In the method sumOverResults(), everytime the new instance of ValueAccumulator is created,so every thread will have its own instance, no instance of ValueAccumulator is shared among threads, so there is no point of data corruption as threads are working on their own objects of ValueAccumulator, so there is no need of any synchronization here.

2. In the ConcurrentValueAccumulator class,

      private ValueHolderRepository repository;
   

   The repository is common in all the ValueAccumulator instances, so at this point we can think of some thread safety because we have something which is shared among different threads(repository in our case). But in the code provided here, we are just reading the repository, and

there is no create/update/write operation on repository instance

, so again there is no need of any synchronization here.

So, The code is thread-safe (provided we are doing any create/write/update operation on any shared data).