Concurrency issue when coordinating two threads

I am trying to divide the work of a loop into two threads.

I am using an ExecutorService to create the 2nd thread while using the main thread as the other one.

I am using a counter to stop the loop when it reaches a certain value but I'm not able to sync the counter among these two threads and loop is running one extra time.

Also, I am using a while loop in main thread to know when the desired count has reached print the end time, which is also not working.

ExecutorService executorService = Executors.newFixedThreadPool(1);
    
    Clock clock = new Clock();

    int count = 5;
    AtomicInteger c = new AtomicInteger(1); 
    clock.start();
    
    executorService.execute(()->{
        while (c.get() <= count) {
            
            System.out.println("LOOP ONE" + "----PRINT_ME");
            c.incrementAndGet();
        }
    });
    
    while (c.get() <= count) {
        
        System.out.println("LOOP TWO" + "----PRINT_ME");
        c.incrementAndGet();
    }
    
    while(true) {
        if(c.get() == count) {
            System.out.println("STOPPED");
            clock.stop();
            break;
        }
    }

Output -

Clock started at -- 2020-07-25T12:32:59.267
LOOP TWO----PRINT_ME
LOOP TWO----PRINT_ME
LOOP TWO----PRINT_ME
LOOP TWO----PRINT_ME
LOOP TWO----PRINT_ME
LOOP ONE----PRINT_ME

In the code above PRINT_ME should get printed 5 times(int count = 5;) only but is is getting printed 6 times (One time extra).

I am not sure what is going on here and how we can share a counted among two threads.

PSI: If I add Thread.sleep(2000); in both loops

executorService.execute(()->{
        while (c.get() <= count) {
            try {
                Thread.sleep(2000);
            } catch (InterruptedException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
            System.out.println("LOOP ONE" + "----PRINT_ME");
            c.incrementAndGet();
        }
    });
    
    while (c.get() <= count) {
        
        try {
            Thread.sleep(2000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
        System.out.println("LOOP TWO" + "----PRINT_ME");
        c.incrementAndGet();
    }

the output is:

Clock started at -- 2020-07-25T12:54:15.596
LOOP TWO----PRINT_ME
LOOP ONE----PRINT_ME
LOOP TWO----PRINT_ME
LOOP ONE----PRINT_ME
LOOP TWO----PRINT_ME
LOOP ONE----PRINT_ME

The problem is that you poll the counter first, perform an action, and then increment the counter signaling that the job has been done.

This creates a window where both threads can be doing the work at the same time and performing the incrementation after they are finished which is just too late.

To make it easier to visualize, let's imagine that there the desired count is 1 and that there are two threads fighting for a task to execute. In your implementation, both threads would start by checking c.get() <= count which would be true for both, and then execute the task which results in the problem you described.

In order to prevent this, threads need to claim a "free slot" first, and only then perform the task. This can be performed by incrementing the counter before the job starts, and then verifying the max count condition before starting each job:

    int count = 5;
    AtomicInteger c = new AtomicInteger(0);

    executorService.execute(()->{
        while (c.incrementAndGet() <= count) {
            // ...
    });

    while (c.incrementAndGet() <= count) {
            // ..
    }