Weird race-condition in java ThreadPoolExecutor

Question

I`m trying to make an api-checking app that must send requests asynchronously using the pool of bots while termination date is not reached.

On startup creates a final List<Bot> is created and will be executed until the termination date. The idea is that at each iteration of the while cycle, we would iterate through the List<Bot> and try to launch them through the executor in wrapper BotRunner. (if the bot is already launched by another thread, we will simply hang on the mutex monitor and wait for completion Bot#run from another thread. After that - execute Bot#run normally).

After execution i see 1-2% of duplicating requests from different bots (server sends correct data i swear). Looks like simple race-condition, but i don't realize what is causes. Is it Bot? - i suppose not (use synchronized). Is it final List<Bot>? - is guess no (we don't change state of list)

Bot entity:

public class Bot 
{
    private final BotStatistic statisic = new BotStatistic();
    private final Object mutex = new Object();
    private String responseData;    

    void run() 
    {
        synchronized(mutex) {
          // first http call -> responseData = "j43iFS135SFSF";
          // second http call(responseData) -> {"success": "true"}
          // save to statistic
          // responseData = null;
        }
    }
}

BotRunner entity:

public class BotRunner implements Runnable 
{
    private final Bot bot;
    
    public BotRunner(Bot bot) 
    {
        this.bot = bot;
    }
    
    @Override
    public void run() 
    {
       bot.run();
    }
}

Trying to execute in this way:

//init executor
LocalDateTime termination = LocalDateTime.now().plusSeconds(5L);
while (LocalDateTime.now().isBefore(termination)) 
{
    for (Bot bot : bots) 
    {
        executor.execute(new BotRunner(bot));
    }
}
//shutdown & close executor

WHAT NOT WORKS:

1. String -> final StringBuffer not helps.
2. Use local method variable String not helps.

Could someone say what i did wrong?

Answer 1

Not sure if this is the cause of your problem (which you do not actually explain), but since you emphasized the importance of every bot running in its own thread I must point out that calling Executor#execute does not necessarily run its task on a background thread. Read the Javadoc; it clearly states that the given task may, or may not, run on a background thread.

If you always want to run tasks on a background thread, call the ExecutorService methods such as submit, invoke, invokeAll.

---

Kill your mutex and synchronized. Since each bot instance is updating its own member fields, and each bot instance is being run exactly once in its own thread, there is no conflict across threads.

---

Your BotRunner class seems utterly redundant. The Bot class already has a run method. So Bot can be declared as implementing Runnable and serve as our task.

Your response data member field could just as well be a local variable.

public class Bot implements Runnable
{
    private Statistic statistic = null ;

    void run() 
    {
        String responseData = firstHttpCall … 
        this.statistic = secondHttpCall ( responseData ) ;
    }
}

Simply submit your list of Bot objects to an executor service.

If your tasks involve blocking, such as making network calls, then you likely should be using virtual threads in Java 21+ for maximum performance.

In modern Java, an executor service is AutoCloseable. So we can use try-with-resources syntax to automatically close it.

try 
(
    ExecutorService executorService = Executors.newVirtualThreadPerTaskExecutor() ;
)
{
     bots.forEach( executorService :: submit ) ;
}
System.out.println( "Tasks done. Executor service closed." ) ;

---

By the way… Never call LocalDateTime.now. That class cannot represent a moment as it lacks the context of a time zone or offset from UTC. To track a point on the timeline, use Instant, OffsetDateTime, or ZonedDateTime.