Retry Runnable java

I want to make class to retry doing what I want if specific exception happen for n times before throwing exception, so I write below class and used Builder to build it could you please review it and update me with your feedback:

public class Retry implements Runnable {
private final long sleep;
private final Runnable r;
private final int times;
private final Predicate p;
private AtomicInteger counter = new AtomicInteger();

private Retry(Builder b) {
    this.r = b.r;
    this.times = b.times;
    this.p = b.p;
    this.sleep = b.sleep;
}

@Override
public void run() {
    try {
        r.run();
    } catch (Throwable th) {
        if (counter.getAndIncrement()  p;

    public Builder(Runnable r) {
        this.r = r;
    }

    public static Builder of(Runnable r) {
        return new Builder(r);
    }

    public Builder times(int times) {
        this.times = times;
        return this;
    }

    public Builder sleep(long sleep) {
        this.sleep = sleep;
        return this;
    }

    public Builder on(Predicate p) {
        this.p = p;
        return this;
    }

    public Retry build() {
        if (this.sleep  true;
        return new Retry(this);
    }
}
}

how to use it

public static void main(String[] args) {
Runnable r = () -> {
    throw new IllegalStateException("Test");
};
runAsync(
        Retry.Builder.of(r).times(10)
                .on(th -> th instanceof IllegalStateException).build())
        .join();
}

Defaulting to properties inside the Builder

You should perform validation inside the times(int), sleep(int) and on(Predicate) to either warn callers of invalid values, if not to throw an Exception. Even if you don't and prefer to silently use the default values, this is what you can do:

public Builder times(int times) {
    this.times = times  p) {
    this.p = Optional.ofNullable(p).orElse(th -> true);
    return this;
}

public Retry build() {
    return new Retry(this);
}

Retry logic

This is roughly how it looks like currently:

Retry.run();
    r.run();
        |_ (Exception) handle(th);
                            |_ (after sleeping) r.run();

The problem here is that you will simply have a cascading chain of calling r.run() on the method call stack, which may result in StackOverflowError if it gets too deep.

You should consider doing the retrying within run() itself as such:

@Override
public void run() {
    // edit: should be < times so that it doesn't run for the (times + 1)-th time
    // while (counter.getAndIncrement() <= times) {
    while (counter.getAndIncrement() < times) {
        try {
            r.run();
        } catch (Throwable th) {
            if (counter.get() <= times && p.test(th)) {
                handle(th);
            } else {
                throw th;
            }
        }
    }
}

private void handle(Throwable th) {
    // corrected typo below
    System.out.println("handle #" + counter.get());
    try {
        Thread.sleep(sleep);
    } catch (InterruptedException e) {
        Thread.currentThread().interrupt();
        throw new RuntimeException(e);
    }
    // don't need to call again
    // run();
}

Design

Not much to comment here, but here is one open-ended question to ponder about:

• Is it always safe to call r.run() again for your use cases?

In other words, are there things to clean up/reinitialize across restarts?

StackExchange Code Review Q#107082, answer score: 3