Race condition in ThreadPoolExecutor

Question

What code must to do:

• execute Bot#run synchronous.

What need to fix:

• sharing resource (Bot.responseData or.. more) between other bots and threads.

Bot.run() execution stages:

1. http request -> get random value from server (like f8c3de3d-1fea-4d7c-a8b0-29f63c4c3454). (server return always random value!)
2. save value to Bot.responseData.
3. http request (use Bot.responseData as param) -> return JSON.
4. save JSON to Bot.statistics.
5. cleanup Bot.responseData = null.

Bot entity:

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

    void run() 
    {
        synchronized(mutex) 
        {
          String myHost = "http://myhost";
          firstCall(myHost)
          secondCall(myHost);
          cleanup();
        }
    }

    private void firstCall(String host) 
    {
        // first http call -> responseData = "f8c3de3d-1fea-4d7c-a8b0-29f63c4c3454";
    }

    private void secondCall(String host) 
    {
        // second http call(responseData) -> 
        // JSON {"success": true, "requestValue": "f8c3de3d-1fea-4d7c-a8b0-29f63c4c3454"}
        statistic.add(json);
    }

    private void cleanup() 
    {
        this.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 i see now

After execution i see 1-2% of duplicate Bot.responseData from DIFFERENT Bot. Stuff like that:

thread-1 : start (Bot@44g35)
thread-1 : f8c3de3d-1fea-4d7c-a8b0-29f63c4c3454
thread-2 : start (Bot@898g)
thread-2 : f8c3de3d-1fea-4d7c-a8b0-29f63c4c3454
thread-1 : end
thread-2 : end

What i must to see

thread-1 : start (Bot@44g35)
thread-1 : f8c3de3d-1fea-4d7c-a8b0-29f63c4c3454
thread-2 : start (Bot@898g)
thread-2 : 018b2f19-e79e-7d6a-a56d-29feb6211b04
thread-1 : end
thread-2 : end

I would really appreciate if someone could help me :)

Answer 1

Your code is incomplete, so I cannot be sure - but you must be sharing a variable between threads.

The Bot class is not threadsafe based on the code fragment you give (responseData is shared). I am curious why you'd even attempt to solve thread safety issues with a 20-year approach like synchronized(mutex); this would just not be necessary if the your Bot threads were certainly not shared. (A more modern approach would to use a Lock).

How are you creating the collection of Bots? Are these all distinct instances?

Or how are you making the HTTP calls - is that code thread-safe?

And as @tgdavies says, how are you making any output?