How to know if a thread is finished in Java?

Question

Working on a Java project and I would like to know if a thread has finished the required calculations.

First attempt: done variable in runnable

The first class is a manager

import uckochfractalfx.UCKochFractalFX;

public class KochManager {
    private final UCKochFractalFX application;
    private final IEdgeCollection edges;

    private int level;
    private int count;

    public KochManager(UCKochFractalFX application) {
        this.application = application;
        this.edges = new EdgeArrayList();
    }

    public synchronized void changeLevel(int nxt) {
        this.level = nxt;
        this.count = 0;
        this.edges.clear();

        EdgeGenerator left, right, bottom;
        left = new EdgeGenerator(this, EdgeLocation.LEFT);
        right = new EdgeGenerator(this, EdgeLocation.RIGHT);
        bottom = new EdgeGenerator(this, EdgeLocation.BOTTOM);

        Thread tLeft, tRight, tBottom;
        tLeft = new Thread(left);
        tRight = new Thread(right);
        tBottom = new Thread(bottom);

        tLeft.start();
        tRight.start();
        tBottom.start();

        while (!(left.isDone() && right.isDone() && bottom.isDone()) {
              wait();
        }

        this.application.setTextCalc(String.valueOf(this.totalTimeExecution));
        this.application.setTextNrEdges(String.valueOf(this.count));    
        application.requestDrawEdges();
    }

    public synchronized void addEdge(Edge edge) {
        this.edges.add(edge);
    }

    public synchronized void increaseCount() {
        count++;
    }

    public int getLevel() {
        return level;
    }

    public void drawEdges() {
        this.application.clearKochPanel();
        this.edges.getAll().forEach((Edge e) -> this.application.drawEdge(e));

        this.application.setTextDraw(String.valueOf(this.totalTimeExecution));
    }
}

And this is the EdgeGenerator class

import java.util.Observable;
import java.util.Observer;

public class EdgeGenerator implements Runnable, Observer {

    private final KochManager kochManager;
    private final EdgeLocation edgeLocation;
    private final KochFractal koch;
    private boolean done;

    public EdgeGenerator(KochManager kochManager, EdgeLocation edgeLocation) {
        this.kochManager = kochManager;
        this.edgeLocation = edgeLocation;

        this.koch = new KochFractal();
        this.koch.addObserver(this);
    }

    @Override
    public synchronized void run() {
        koch.setLevel(kochManager.getLevel());
        this.done = false;

        switch (this.edgeLocation) {
            case LEFT:
                this.koch.generateLeftEdge();
                break;
            case RIGHT:
                this.koch.generateRightEdge();
                break;
            case BOTTOM:
                this.koch.generateBottomEdge();
                break;
        }
        this.done = true;
    }

    public boolean isDone() {
        return done;
    }

    @Override
    public void update(Observable o, Object o1) {
        this.kochManager.addEdge((Edge) o1);
        this.kochManager.increaseCount();
    }
}

Everything works but one thing, and that is getting the values from the objects in the thread back to the main thread.

The variable I would like to focus on here is done in the EdgeGenerator class.

I have tried a couple of ways here.

First is the implementation as you can see it in the code above: Set this.done after all the calculations, which should then be returned by calling isDone().
However when calling isDone() it always returns false

Second attempt: done in caller of runnable

Then I decided to create a variable in KochManager called leftDone, create a setter setLeftDone() and call that in EdgeGenerator. This also always retuned false.

Manager: public class KochManager { private final leftDone; ...

    public void setLeftDone(boolean done) {
        this.leftDone = done;
    }
    ...  

    public synchronized void changeLevel(int nxt) {
        ...
        while (!(this.leftDone && this.rightDone && this.BottomDone)){
            wait();
        }
        ...
    }
}

EdgeGenerator:

public class EdgeGenerator {
    ...
    @Override
    public synchronized void run() {
        ...
        switch (this.edgeLocation) {
            case LEFT:
                this.koch.generateLeftEdge();
                this.kochManager.setLeftDone(true);
                break;
            ...
        }
    }
}

As you might have guessed that didn't work either, so after some more searching I decided to use another object that would have a single value.

Third attempt: Create class Done that would hold a variable to be set.

Manager:

public class KochManager {
    private class Done {
        public boolean done;
    }
    ...

    private Done leftDone;

    public KochManager(UCKochFractalFX application) {
        this.application = application;
        this.edges = new EdgeArrayList();
        this.leftDone = new Done();
    }

    public void setLeftDone(boolean done) {
        this.leftDone.done = done;
    }
    ...  

    public synchronized void changeLevel(int nxt) {
        ...
        while (!(this.leftDone.done && this.rightDone.done && this.BottomDone.done)){
            wait();
        }
        ...
    }
}

EdgeGenerator:

public class EdgeGenerator {
    ...
    @Override
    public synchronized void run() {
        ...
        switch (this.edgeLocation) {
            case LEFT:
                this.koch.generateLeftEdge();
                this.kochManager.setLeftDone(true);
                break;
            ...
        }
    }
}

Once this did not work out either I decided to call the help of StackOverflow. Can someone help me out here?

Answer 1

The problem with your code is that you have one of the common gotchas with concurrency: memory visibility. When one thread updates done the others don't see it has been updated. You must declare you done variable as volatile in order to prevent this issue from happening:

volatile boolean done;

Answer 2

However when calling isDone() it always returns false

You need to go with Option-1, but mark your done variable as volatile as shown below, otherwise the other threads will not be guaranteed to see the changes to the done variable.

private volatile boolean done;

The following is the quote taken from here about volatile

Changes to a volatile variable are always visible to other threads. What's more, it also means that when a thread reads a volatile variable, it sees not just the latest change to the volatile, but also the side effects of the code that led up the change.

Answer 3

The problem is the value of done is cached. You need to mark it as volatile so that calls to it from various threads always get the most up-to-date value.