How to end a thread after coming out of a for loop

Question

So I have a program that prints Hello and Goodbye by using 2 threads. I need to print Hello 100 times and Goodbye 100 times. This is working fine, however, after it prints, the program hangs and doesn't finish. I have a feeling it is because the threads aren't finishing up.

I was wondering what the best way to do this is?

So my main is like this

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

    new HelloThread(lock).start();
    new GoodbyeThread(lock).start();
}

And here is the HelloThread class. GoodbyeThread looks the same:

    class HelloThread extends Thread

    {
        private Object lock;
        private boolean isRunning = false;

    HelloThread(Object l)
    {
        this.lock = l;
    }

    public void run()
    {
        for(int i = 0; i < 100; i++)
        {
            synchronized(lock)
            {
                isRunning = true;
                System.out.println("Hello");
                lock.notify();

                try
                {
                    lock.wait();
                }
                catch(InterruptedException e)
                {
                    e.printStackTrace();
                }
            }
        }
    }
}

I was thinking there might be a way to have the isRunning variable be used to close the thread, however I'm not sure what the best way to go about it. Thanks for any help

@MadConan Nothing yet that was just something I left in from messing about to try get it to work — Ayohaych, Nov 21 '13 at 18:43

score 1 · Answer 1 · edited May 23 '17 at 12:12

1

This is working fine, however, after it prints, the program hangs and doesn't finish. I have a feeling it is because the threads aren't finishing up.

If the thread is leaving the run() method then it will finish up. I suspect that your thread is stuck on lock.wait() after the 100th printing of "Hello". If you use jconsole or dump a thread stack trace you should be able to see where the threads are stuck.

One thing to realize is that if you call lock.notify() the only time the other thread will get the notification is if it is in lock.wait(). Notify calls do not queue up.

edited May 23 '17 at 12:12

Community

1
1

answered Nov 21 '13 at 18:40

Gray

115,027
24
293
354

Yeah that sounds about right. Just need to figure out how to fix this. What is the name given to this? Is this deadlock? – Ayohaych Nov 21 '13 at 18:47
1

It's not deadlock @AndyOHart. It's just bad logic. – Gray Nov 21 '13 at 19:10
1

I've added more details @AndyOHart about notify does not get saved if the other thread is not waiting. – Gray Nov 21 '13 at 20:03

score 1 · Answer 2 · answered Nov 21 '13 at 18:44

1

The for loop isn't important, the key things happen in the last iteration. It is very probable that one of the threads will enter its wait() after the other thread has already executed its notify(), thus missing the only chance to break out of that wait().

answered Nov 21 '13 at 18:44

Marko Topolnik

195,646
29
319
436

Ah that makes sense. How can you go about fixing this? – Ayohaych Nov 21 '13 at 18:46
1

First of all by not using the `wait-notify` mechanism at all. It is low-level and prone to many issues; the recommended way is to use one of the synchronization tools from the `java.util.concurrent` package. As I figure you want strict interleaving of the two threads: one goes for a step, then the other. You can achieve that with two `CountDownLatch`es or with a single `Phaser`. – Marko Topolnik Nov 21 '13 at 18:51
Ah I see so I shouldn't really use wait-notify then. I had a feeling I was doing it wrong as from what I remember from Threading modules in college, using wait and notify was too simple haha – Ayohaych Nov 21 '13 at 19:22

score 1 · Answer 3 · answered Nov 21 '13 at 18:52

1

Your GoodbyeThread is waiting for your HelloThread to send a notify().

What is likely happening at i == 99:

HelloThread prints "Hello" calls notify(), then calls wait()

GoodbyeThread prints "Goodbye" calls notify() [here HelloThread exits the for loop and terminates], then calls wait().

Nothing will ever call notify() to end the wait() of the GoodbyeThread.

One fix would be to only call wait() if i < 99.

answered Nov 21 '13 at 18:52

andykellr

789
6
9

Ah I see! There is probably a better way for me to do it though yeah? – Ayohaych Nov 21 '13 at 19:19

score 0 · Answer 4 · answered Nov 22 '13 at 12:47

As mentioned above: the thread is deadlocking because at least one of them is still waiting for the notify() call.

Imagine you only had one run to do (print "Hello world" only once):

Thread A starts, prints "Hello", then runs notify(), but no one is listening, then wait().
Thread B starts, prints "World", then runs notify().
Thread A wakes up and tries to regain the monitor lock but this one is still hold by Thread B, so Thread A has to wait further.
Thread B runs wait(), releasing the monitor.
Thread A wakes up again, regains the monitor and finishes the loop.

Thread B still waits, but is never woken up.

This can be solved through an exit notify like this:

for(int i = 0; i < 100; i++) {
  // ...
}
synchronized (lock) {
  lock.notifyAll();
}

The wait/notify constructs are the most basic tools for threading, and not well suited for many threading problems out there, as they very often can result in deadlocks or worse. For easier and less error-prone threading concepts, you should use the concurrent package, which has ready-made tools available for most problems out there.

But it is necessary to be familiar with these tools, and you absolutely must design your code around these tools and not vice versa. An existing program cannot be just filled up with multi-threading to make it faster, it needs to be designed for it before you even write the first line of code. Otherwise you run into many issues, including that your result might be slower than a single-threaded execution would have been.

How to end a thread after coming out of a for loop

4 Answers4