CODEWITHSUNDEEP
javamultithreadingthread-safety
coder25
coder25

Reputation: 2393

How to make manual retry thread safe

I am trying to do a manual retry. But I feel the code is not thread safe. Can anyone please provide suggestion on how to make it thread safe 

while (retryCounter < maxRetries) {
          try {
            //TODO - add delay with config
            Thread.sleep(3000);
            t.run();
            break;
          } catch (Exception e) {
            retryCounter++;
            //TODO - Add audit logs
            if (retryCounter >= maxRetries) {
              LOG.info("Max retries exceeded");
              //TODO - remove exception add audit logs
              throw new RuntimeException("Max retry exceeded");
            }
          }
        }

Thanks in advance

Upvotes: 1

Views: 1779

Answers (2)

David Soroko
David Soroko

Reputation: 9096

As pointed out by @ SteffenJacobs, t.run does not execute the run logic on a separate thread but rather on the thread that makes the invocation. If you replace t.run with t.start then the run logic will be executed on a different thread asynchronously which means that exceptions in that new thread will never be handled by your catch block. For example the code below:

public static void main(String[] args) {
    int retryCounter = 0;
    int maxRetries = 3;

    Thread t = new Thread(new Runnable() {
        public void run() {
            System.out.println("..." + Thread.currentThread());
            throw new RuntimeException("Thrown by me!!!");
        }});

    while (retryCounter < maxRetries) {
        try {
            Thread.sleep(3000);
            System.out.println("***" + Thread.currentThread());
            t.start();
            break;
        } catch (Exception e) {
            System.out.println("retrying attempt " + retryCounter);
            System.out.println(e);
            retryCounter++;
            if (retryCounter >= maxRetries) {
                throw new RuntimeException("Max retry exceeded");
            }
        } finally {
            System.out.println("in finally");
        }
    }
}

prints:

***Thread[main,5,main]
in finally
...Thread[Thread-0,5,main]
Exception in thread "Thread-0" java.lang.RuntimeException: Thrown by me!!!
...
Process finished with exit code 0

Bottom line - To detect errors in your threaded tasks, you will need to think of a different approach. You could consider using Futures (https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/Future.html). In this case your tasks will need to return a status of their execution while the code that launched them waits on Future.get , examines the status and reruns the task as needed.

Upvotes: 1

Kushal
Kushal

Reputation: 8508

The answer of @David Soroko is very close, but I think it need little modification. As your code would run under multi-threaded environment, we should take care of Synchronization

If multiple thread tries to enter while loop, it may happen that 2 invocations of Retry Thread would get started, with 2 calls of t.start(). Ideally, we should start only single invocation during single iteration of while loop.

Little modification :

Declare LOCK object in your class level

// Object to ensure thread safety between multiple threads
private final LOCK = new Object();

Use LOCK object with synchronized block in while loop 

synchronized(LOCK) { // This will ensure single thread entry at a time
    while (retryCounter < maxRetries) {
        try {
            Thread.sleep(3000);
            System.out.println("***" + Thread.currentThread());
            t.start();
            break;
        } catch (Exception e) {
            System.out.println("retrying attempt " + retryCounter);
            System.out.println(e);
            retryCounter++;
            if (retryCounter >= maxRetries) {
                throw new RuntimeException("Max retry exceeded");
            }
        } finally {
            System.out.println("in finally");
        }
    }
}

The variables retryCounter and maxRetries are read by multiple threads. This means they are shared resources between threads. We need to ensure thread safety there as well. There is concept of Atomicity and Volatility.

Atomicity : When any thread T1 is doing operation on Variable A, at the same time, if other thread T2 tries to do operation on A, either operation of T1 should get completed fully or operation of T1 should be aborted fully, before starting operation of T2.

Volatility : When multiple threads are accessing single variable A, all threads should read the actual value from memory. In multithreaded environment, depending on processor, threads do optimisation while reading variable resources like A. Optimised value of A may not match actual value of A in memory. This may lead to Race condition or misbehaviour of your business logic. In your case, the Retry operation may run more than Max Retry count in case of misbehaviour.

Make both shared resources to comply Volatile and Atomic properties

final AtomicInteger retryCounter = new AtomicInteger(0);
final AtomicInteger maxRetries = new AtomicInteger(3);

Upvotes: 0

Related Questions