Java – why don’t these code blocks give the same result?

So I'm a novice to thread. I wrote a simple program to test and avoid competition conditions My first attempt was to use the named inner class:

/* App1.java */

package ehsan;

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class App1{
    private final int poolSize = 10;
    private final int numLoop = 5;
    private int lastThread = 0;

    public App1() {
        ExecutorService taskList = Executors.newFixedThreadPool(poolSize);
        for (int i = 0;i < poolSize;i++) {
            taskList.execute(new Counter());
        }
        taskList.shutdown();
    }

    private class Counter implements Runnable{

        @Override
        public void run() {
            synchronized (this) {
                int currentThread = lastThread;
                System.out.println("Current thread : "+currentThread);
                lastThread = lastThread + 1;
            }
            System.out.println("Thread was executed");
        }
    }

}

And app1test java：

package ehsan;

import java.io.IOException;

public class Test {
    public static void main(String[] args) throws IOException {
        new App1();
    }
}

As a result, it shows:

Current thread : 0
Thread was executed
Current thread : 1
Thread was executed
Current thread : 1
Thread was executed
Current thread : 3
Thread was executed
Current thread : 4
Thread was executed
Current thread : 5
Thread was executed
Current thread : 6
Thread was executed
Current thread : 7
Thread was executed
Current thread : 6
Current thread : 8
Thread was executed
Thread was executed

The whole thing became chaotic, and even if I used synchronization there, I faced competitive conditions

But my second attempt worked!:

package ehsan;

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class App1 implements Runnable{
    private final int poolSize = 10;
    private final int numLoop = 5;
    private int lastThread = 0;

    public App1() {
        ExecutorService taskList = Executors.newFixedThreadPool(poolSize);
        for (int i = 0;i < poolSize;i++) {
            taskList.execute(this);
        }
        taskList.shutdown();
    }

    @Override
    public void run() {
        synchronized (this) {
            int currentThread = lastThread;
            System.out.println("Current thread : "+currentThread);
            lastThread = lastThread + 1;
            System.out.println("Thread was executed");
        }
    }
}

As I expected:

Current thread : 0
Thread was executed
Current thread : 1
Thread was executed
Current thread : 2
Thread was executed
Current thread : 3
Thread was executed
Current thread : 4
Thread was executed
Current thread : 5
Thread was executed
Current thread : 6
Thread was executed
Current thread : 7
Thread was executed
Current thread : 8
Thread was executed
Current thread : 9
Thread was executed

So my question is why my first attempt didn't work and my second attempt worked a lot? Thank you for your help. I'm a beginner of multithreaded programming!

Solution

In the first program, you create a different counter instance as runnable, and its run () method is executed by each thread. Therefore, synchronized (this) uses different locks for each thread, so the code is not thread safe If you use the same counter instance instead of creating a new instance for each thread, the program will also run as expected

Counter counter = new Counter();
    for (int i = 0;i < poolSize;i++) {
        taskList.execute(counter);
    }

In the second program, you use the same app1 instance as runnable, and its run () method is executed by all threads, so synchronized (this) uses the same lock for all threads