Doubts on thread safety

Question

I'm new about multithread processing in java and now I have to implement an multithreads module using some legacy classes. I think to have some problem of race condition beacause I'm getting different result between single thread and multi threads execution. My main doubts is the following:

public class Worker implements Runnable{

    final private int minIndex; // first index, inclusive
    final private int maxIndex; // last index, exclusive
    final private MyDTO dto;
    private MyService myService;

    public Worker(MyDTO dto) {
        this.minIndex = dto.getMinIndex();
        this.maxIndex = dto.getMaxIndex();
        this.dto = dto;
        myService = new MyService();

    }

    public void run() {
        int countReg = 0;
        if(!initErrors){

            try {
                Connection conn = ConnectionFactory.getConnection();
                for(int i = minIndex ; i<maxIndex; i++){
                    myService.executeCalculation(dto,conn);
                    countReg++;
                }
                conn.close();
            } catch (SQLException e1) {
                // TODO Auto-generated catch block
                e1.printStackTrace();
            }           
        }
    }


public class MyService{


    //It has some method to get object from the database

    public void executeCalculation(MyDTO dto,Connection conn) {

        //I do some actions and 
        RegolaDAO regolaDAO = new RegolaDAO(conn);
        MyObject obj = new MyObject(conn);
        // I use obj durin the calculation

        }   
    }
}

public class MyObject{

    Connection conn;

    public MyObject (Connection conn){
        super();
        this.conn = conn;
    }
}

My service is a stateless object and then don't should make any problem while MyObject is a statefull object and this could be make problem. My main doubt since I' using new keyword for each calculation MyObject obj = new MyObject(); this help me to prevent race condition.

Is MyObject thread safe? Can you give me some suggestion about my implementation?

Answer 1

MyObject thread safe? Can you give me some suggestion about my implementation?

Since MyObject has non-final fields (myService) then it is considered not thread safe. If two threads were using the same MyObject instance, they could start using myService before it was properly initialized. As @Peter points out, making myService be final would solve that particular issue.

Looking at the code, the following lines are making calls to other classes so are candidates for thread-safety issues:

if (!initErrors) {

I don't see how initErrors is defined. If that is shared between threads then it will need to be volatile or an AtomicBooolean.

Connection conn = ConnectionFactory.getConnection();

Is the ConnectionFactory thread-safe? Can multiple threads call it at once?

myService.executeCalculation(dto, conn);

Same questions for the executeCalculation(...) method. Also, as @Peter points out, the dto is being used by the service. If the service is making changes to the dto instance then it will need to be synchronized.

Answer 2

Since the myService is not being changed, you can make it final.

It appears that your dto is shared between threads, I would check that it is read only as well.

Also how is conn being passed to executeCalculation, you should make sure you are not using a field for this.