Threading in Java (practicing for college)

Question

Create a program that simulates training at an athletic stadium, there is one track in the stadium that can be used by up to 5 people at a time and the coach does not allow that number to exceed, but when some of the athletes finish their run (2sec) and free up space then notify other athlete for running.

After 2 seconds, all processes are frozen

My question is, could anyone explain to me why something like this does not work and how to handle this problem?

class JoggingTrack {
    public int numOfAthlete;

    public JoggingTrack() {
        this.numOfAthlete = 0;
    }

    @Override
    public String toString() {
        return "\nNumber of Athlete: " + numOfAthlete + "\n";
    }
}

class Athlete extends Thread {

    private JoggingTrack track;
    private boolean running;

    public Athlete(JoggingTrack s) {
        this.track = s;
        this.running = false;
    }

    public synchronized boolean thereIsSpace() {
        if(track.numOfAthlete < 5) {
            return true;
        }
        return false;
    }

    public synchronized void addAthlete() {
        track.numOfAthlete++;
        this.running = true;
    }

    public synchronized void removeAthlete() {
        track.numOfAthlete--;
        this.running = false;
    }

    @Override
    public void run() {
        try {
            while(true) {
                while(!this.thereIsSpace()) {
                    wait();
                }
                while(!this.running) {
                    addAthlete();
                    sleep(2000);
                } 
                while(this.running) {
                    removeAthlete();
                    notify();
                }
            }
        } catch (Exception e) {
        }
    }

}

public class Program {
    static JoggingTrack track;
    static Athlete[] a;

    public static void main(String[] args) {
        track = new JoggingTrack();
        a = new Athlete[10];
        for(int i = 0; i < 10; i++) {
            a[i] = new Athlete(track);
            a[i].start();
        }
        while(true) {
            try {
                System.out.println(track);
                Thread.sleep(500);
            } catch (Exception e) {
            }
        }
    }
}

Answer 1

My question is, could anyone explain to me why something like this does not work and how to handle this problem?

Debugging multithreaded programs is hard. A thread-dump might help and println-debugging might also be helpful however they can cause the problem to migrate so it should be used with caution.

In your case, you are confusing your objects. Think about Athlete.thereIsSpace() and Athlete.addAthlete(...). Does that make any sense? Does an athlete have space? Do you add an athlete to an athlete? Sometimes the object names don't help you make these sorts of evaluations but in this case, they do. It is the JoggingTrack that has space and that an athlete is added to.

When you are dealing with multiple threads, you need to worry about data sharing. If one thread does track.numOfAthlete++;, how will other threads see the update? They aren't sharing memory by default. Also ++ is actually 3 operations (read, increment, write) and you need to worry about multiple threads running the ++ at the same moment. You will need to use a synchronized block to ensure memory updates or use other concurrent classes such as AtomicInteger or a Semaphore which take care of the locking and data-sharing for you. Also, more generally, you really should not modify another object's fields in this way.

Lastly, you are confused about how wait/notify work. First of all, they only work if they are inside a synchronized block or method so I think the code you've posted won't compile. In your case, the thing that the multiple Athletes are contending for is the JoggingTrack, so the track needs to have the synchronized keyword and not the Athlete. The Athlete is waiting for the JoggingTrack to get space. No one is waiting for the athlete. Something like:

public class JoggingTrack {
    public synchronized boolean thereIsSpace() {
        return (numOfAthletes < 5);
    }
    public synchronized void addAthlete() {
        numOfAthletes++;
    }
    ...

Also, like the ++ case, you need to be really careful about race conditions in your code. No, not jogging races but programming races. For example, what happens if 2 athletes both go to do the following logic at precisely the same time:

while (!track.thereIsSpace()) {
    track.wait();
}
addAthlete();

Both athletes might call thereIsSpace() which returns true (because no one has been added yet). Then both go ahead and add themselves to the track. That would increase the number of athletes by 2 and maybe exceed the 5 limit. These sorts of races-conditions happen every time unless you are in a synchronized block.

The JoggingTrack could instead have code like:

public synchronized void addIfSpaceOrWait() {
    while (numOfAthletes >= 5) {
        wait();
    }
    numOfAthletes++;
}

Then the althetes would do:

track.addIfSpaceOrWait();
addAthlete();

This code has no race condition because only one athlete will get the synchronized lock on the track at one time -- java guarantees it. Both of them can call that at the same time and one will return and the other will wait.

Couple other random comments:

• You should never do a catch (Exception e) {}. Just doing an e.printStackStrace() is bad enough but not seeing your errors is really going to confuse you ability to debug your program. I will hope you just did that for your post. :-)
• I love the JoggingTrack object name but whenever you reference it, it should be joggingTrack or maybe track. Be careful of JoggingTrack s.
• An Athlete should not extend thread. It isn't a thread. It should implement Runnable. This is a FAQ.

Answer 2

Your threads are waiting forever, because they are waiting on some object (their instance itself), and nobody ever notify-es them, using the right instance.

One way to fix this is to have all athlete-s to synchronize/wait/notify on the same object, in example, the JoggingTrack. So that an athlete will wait on the track with track.wait(), and when an athlete is done running, it will call track.notify() , and then a waiting athlete will be waken up.

Then there are other issues as noted by Gabe-

Once you fix the first issue, you will find the race conditions- eg. too many threads all start running even though there are some checks (thereIsSpace) in place.

Answer 3

A lot of issues with this.

Your methods are in the wrong place. The synchronized keyword synchronizes on an instance of the class, not across multiple instances. So your remove and add functions on different athletes would cause race conditions. These functions should be moved to the Track object, because all athletes are using the same track (so should your isThereSpace function). At the same time, you should not be directly accessing the member variables of Track in Athlete, use a getter for it instead.

Secondly, you use of wait and notify are wrong. They leave lots of holes for race conditions, although it may work most of the time. And this isn't really a good place for using them- a counting semaphore in the Track class would be a better solution- its exactly what counting semaphores are made for. Look at the Semaphore class for more details. Its basically a lock that will allow N owners of the lock at a time, and block additional requesters until an owner releases it.