Thread output wrong

Question

I want to print out each thread's number aka 1,2,3... regardless of the order of the number outputed, but when I ran the code below, many numbers were repeated in the output

public static int ThreadNumber = 0;

for (int i = 0; i <= 8; i++) {

        new Thread(() -> {
            System.out.println("Thread number : " +  ThreadNumber);
        }).start();

        ThreadNumber++;
}

Output:

Thread number : 3
Thread number : 3
Thread number : 4
Thread number : 5
Thread number : 6
Thread number : 9
Thread number : 9
Thread number : 9
Thread number : 9

How can I fix this so each thread outputs a specific number only

You need a thread lock if you want to share a static variable between the threads, or just print the value of `i` in the threads. — ruohola, Jan 10 '20 at 20:01
So for each loop you create a new thread. Threads take time to spin up, so, it's possible you will have n number of threads who run at some point in the future. If you want the thread to print just a single number then you should either pass the number to the thread directly or just a semaphore to stop the thread until a new number is available. I can't really see the point of using multiple threads for this, as a single thread would do the job for you — MadProgrammer, Jan 10 '20 at 20:01
How would you pass it to the thread directly? @MadProgrammer — George, Jan 10 '20 at 20:16
@George via a constructor, so you'd need to use a custom `Runnable` — MadProgrammer, Jan 10 '20 at 20:18
@George see https://stackoverflow.com/questions/877096/how-can-i-pass-a-parameter-to-a-java-thread — jrook, Jan 10 '20 at 20:21
I need it to run multiple separate threads; hence, why it's like that in the code snippet I posted — George, Jan 10 '20 at 20:27
@George The linked question should work for your case. Just wrap your code inside a custom Runnable like the answers in that question. — jrook, Jan 10 '20 at 20:38

Tom Hawtin - tackline · Accepted Answer · 2020-01-10T22:04:12.350

2

Avoid mutable global state ("statics").

To pass a value to a lambda expression, assign it to an effectively final local variable. No need to escalate to a whole class, yet.

int threadNumber = 0;

for (int i = 0; i <= 8; i++) {
    int thisThread = threadNumber;
    new Thread(() -> {
        System.out.println("Thread number : " +  thisThread);
    }).start();

    ++threadNumber; // Just i in this case.
}

Obviously there's no guarantee which order this will print out in - that's the point of threads.

edited Jan 10 '20 at 22:04

answered Jan 10 '20 at 21:47

Tom Hawtin - tackline

145,806
30
211
305

Thank you for your answer, it works the way I wanted it to, but I don't understand why making the variable static makes it not work – George Jan 11 '20 at 05:19

Jason Law · Answer 2 · 2020-01-10T22:10:56.007

ThreadNumber is shared mutable data. So, you need synchronization to ensure only one thread accesses that shared mutable data.

public class Main {

    public static int ThreadNumber = 0;

    public static void main(String[] args) {
        Object lock = new Object();

        for (int i = 0; i <= 8; i++) {
            new Thread(() -> {
                synchronized (lock) {
                    System.out.println("Thread number : " + ++ThreadNumber);
                }
            }).start();
        }
    }
}

Thread output wrong

2 Answers2