MrG
MrG

Reputation: 5287

Java: synchronization of variables between threads

I'm trying to start/stop Java threads in the following way.

public class ThreadTester {
    public static void main(String[] args) {
        MyThread mt;
        int max = 3;

        for (int i = 0; i < max; i++) {
            mt = new MyThread();
            mt.start();
            mt.finish();
        }
    }
}

public class MyThread extends Thread {
    private volatile boolean active;

    public void run() {
        this.active = true;
        while (isActive()) {
            System.out.println("do something");
        }
    }

    public void finish() {
        this.active = false;
    }

    public boolean isActive() {
        return active;
    }
}

Everything works as expected only if max <= 2. Otherwise some threads continue with their output, although isActive should return false. That was at least my expectation.

The question: what is the proper way to synchronize a variable between the "master" and it's "slave" threads?

Upvotes: 0

Views: 926

Answers (3)

Klark
Klark

Reputation: 8290

When you say start you are saying that thread is ready for execution but the run method doesn't start at that moment (it starts when scheduler says). So, it is quite possible that you first do this.active = false and than get stuck in the endless loop.

Upvotes: 2

John Gardner
John Gardner

Reputation: 25146

what isn't working as expected?

i don't see anything in there that's synchronized across threads, aside from your active member, which isn't really shared, as its private... and since the only method that modifies it sets it to false.

The only thing i can see there is that you might be calling finish before your thread ever runs, which is more and more likely to happen as you increase the number of threads.

if you wanted your thread to do at least one thing before stopping, you could switch your while loop to a do instead:

do
{
    // do nothign at least once
}
while( active );

then each thread would do something before checking to see if it should stop.

Upvotes: 0

Alexander Pogrebnyak
Alexander Pogrebnyak

Reputation: 45596

You should initialize active to true during declaration and NOT in a run method.

public class MyThread extends Thread {
    private volatile boolean active = true;

    public void run() {
        // this.active = true;
        while (isActive()) {
            // do nothing
        }
    }

    public void finish() {
        this.active = false;
    }
}

The way you are doing it there is a race condition.

Also, the better approach for safely stopping a thread is to use interruption.

Upvotes: 8

Related Questions