Threads getting wrong member variable values

Question

In my project I am facing weird issue with thread.

This issue is only occurring when I am running multiple thread at once(load testing).

In my project there are multiple Interceptors which intercepts request/response at different level in application and send the request/response to WritetoFile class which writes the details into a flat file using log4j framework.

Below is the sample interceptor code. There are multiple interceptor and each can process in parallel.

/*we have multiple Interceptor class which will pre-process the 
request/response and then send it to WritetoFile*/

    public class IntercerptorA {

        // some code ...
        public synchronized void sendRequestToWritetoFile(IRequest request,IResponse response){
        WritetoFile wtf = new WritetoFile(); //this class is responsible for writing request/response information into LOG file
        wtf.setRequest(request);
        wtf.setResponse(response);
        Thread thread=new Thread(wtf, "t1");//**assume wtf.getRequest is having "ABC"**
        thread.start();
        }
    }

Now suppose there 2 more Interceptor and has only single line difference in the code.

//For interceptorB
    Thread thread=new Thread(wtf, "t2");//**assume wtf.getRequest is having "DEF"**

//For interceptorC
    Thread thread=new Thread(wtf, "t3");//**assume wtf.getRequest is having "XYZ"**

Below is the code for WritetoFile class -:

public class WritetoFile implements Runnable{

    private volatile IRequest request;
    private volatile IResponse response;

    public synchronized IRequest getRequest() {
        return request;
    }
    public synchronized void setRequest(IRequest request) {
        this.request = request;
    }
    public synchronized IResponse getResponse() {
        return response;
    }
    public synchronized void setResponse(IResponse response) {
        this.response = response;
    }

    @Override
    public void run() {
        // I have added synchronized as I was trying to resolve the issue
        synchronized(WritetoFile.class){
            putItInFile(this.request,this.response);
        }

    }
    private synchronized void putItInFile (IRequest request,IResponse response){
        // This is the logger where I find discrepancies
        LOGGER.info("Current thread is : "+Thread.currentThread().getName()+" data is"+request);
        //some code and  method call
    }

}

Having said that, now when I am running a single request the LOGGER.info("Current thread is : "+Thread.currentThread().getName()+" data is"+request); line is giving output as below -:

Current thread is t1 data is ABC

Current thread is t2 data is DEF

Current thread is t3 data is XYZ

which is perfectly fine. BUT on running multiple thread at once I am getting sometime wrong output as below -:

Current thread is t1 data is DEF

Current thread is t2 data is DEF

Current thread is t3 data is XYZ

It seems to be that before thread t1 can use the value of "wtf" object in method putItInFile , thread t2 have already reset the wtf value using setter in interceptorB. But what my thinking is, when I am creating new instance WritetoFile class for each thread ,how is thread t2 operation changing thread t1 cache. Please let me know where am I going wrong and what I need to change.

Thanks in advance :)

Answer 1

Using synchronized everywhere does not make a class thread safe.

In your case, as soon as WritetoFile.setRequest(request1) returns there is a window where the lock is not held and any other thread is free to call it before there is an opportunity for it to be used.

Rather than assigning the requests to an instance variable you would be better off adding them to one of the java.util.concurrent queue classes and consuming them from the queue in the Thread.run() method.

Have a look at the java.util.concurrent javadoc as there are heaps of examples in there.

Answer 2

Most likely the DEF request is getting intercepted at two different levels, resulting in the request getting logged twice.

Answer 3

Your problem is a textbook concurrency problem. You have multiple threads running at the same time that are able to read/write variables.

In order to make sure that these values stay correct you need to add a lock around the code that modifies your variables so that only one thread can modify these variables at any one time.

1) code needs to wait until a method that modifies variables becomes available. 2) when a thread is done modifying a variable and is about to exit the code block it needs to notify the other waiting threads that it is done.

Please read the API and review your code, keeping the above points in mind you should have no problems fixing it.