2

I have a small question for you.

I have two threads:

  • HelloThread - This prints "hello" five times.
  • GoodbyeThread - This prints "Goodbye" five times.

I would like the HelloThread run first and after it's done the GoodbyeThread run.
I have already solve this problem with semaphore (but semaphore doesn't really true java way, it's more C way).

HelloThread.java

    /*
     * To change this license header, choose License Headers in Project Properties.
     * To change this template file, choose Tools | Templates
     * and open the template in the editor.
     */
    package modifiedthreadhellogoodbye;

    import static java.lang.Thread.sleep;
    import java.util.logging.Level;
    import java.util.logging.Logger;
    import java.util.concurrent.Semaphore;

    class HelloThread implements Runnable {

        private final Object lock;
        //private final Semaphore lock;

        public HelloThread(Semaphore lock) {
            this.lock = lock;
        }

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

        @Override
        public void run() {
            int pause;
            synchronized (lock) {
                for (int i = 0; i < 5; i++) {
                    System.out.println("Hello!");
                    pause = (int) (Math.random() * 1000);
                    try {
                        sleep(pause);
                    } catch (InterruptedException ex) {
                        Logger.getLogger(HelloThread.class.getName()).log(Level.SEVERE, null, ex);
                    }
                }
                lock.notifyAll();
                System.out.println("Outsite hello");
            }
                //lock.release();

        }
    }

GoodbyeThread.java

/*
 * To change this license header, choose License Headers in Project Properties.
 * To change this template file, choose Tools | Templates
 * and open the template in the editor.
 */
package modifiedthreadhellogoodbye;

import static java.lang.Thread.sleep;
import java.util.concurrent.Semaphore;
import java.util.logging.Level;
import java.util.logging.Logger;

class GoodbyeThread implements Runnable {

    int pause;
    //private final Semaphore lock;
    private final Object lock;

    public GoodbyeThread(Semaphore lock) {
        this.lock = lock;
    }

    public GoodbyeThread(Object lock) {
        this.lock = lock;
    }

    @Override
    public void run() {
        synchronized (lock) {
            System.out.println("Inside the synchronized");
            try {
                lock.wait();
            } catch (InterruptedException ex) {
                Logger.getLogger(GoodbyeThread.class.getName()).log(Level.SEVERE, null, ex);
            }

            //lock.acquire();
            for (int i = 0; i < 5; i++) {
                System.out.println("Goodbye");
                pause = (int) (Math.random() * 1000);
                try {
                    sleep(pause);
                } catch (InterruptedException ex) {
                    Logger.getLogger(GoodbyeThread.class.getName()).log(Level.SEVERE, null, ex);
                }
            }
        }
    }

}

and this is my class who run thread :

public class ModifiedThreadObject {

    private final Object lock = new Object();
    //private final Semaphore lock = new Semaphore(0);

    public ModifiedThreadObject() {
        HelloThread hello = new HelloThread(lock);
        GoodbyeThread goodbye = new GoodbyeThread(lock);

        Thread t1 = new Thread(hello);
        Thread t2 = new Thread(goodbye);
        t1.start();
        t2.start();
    }

}

The main idea is GoodbyeThread should wait() for the signal from HelloThread.
If GoodbyeThread is run first, it's work perfectly, but HelloThread runs first I have the following output:

Hello!
Hello!
Hello!
Hello!
Hello!
Outsite hello
Inside the synchronized

HelloThread sends a notifyAll(), but nobody is waiting so the "signal" is lost ...

Anybody have an idea?

Mr. Polywhirl
  • 42,981
  • 12
  • 84
  • 132
Nymeria
  • 267
  • 4
  • 13
  • Why is Semaphore not "the right way" if it works for you? – Patrick Jan 13 '15 at 13:38
  • This is just an academic example. I don't remember where but I have read java don't follow the mutex / semaphore pattern because it's too much complicated – Nymeria Jan 13 '15 at 13:43
  • "It's too complicated"? According to who, and in what aspect? Why would the class be available if you weren't supposed to use it? – Patrick Jan 13 '15 at 13:48
  • Another option by the way is to use an observer pattern, where the goodbye thread listens for a trigger to start, which is fired by the hello thread when it's finished. – Patrick Jan 13 '15 at 13:49
  • I have not think about observer pattern, it could be good solution too. I have read the good way for synchronize the thread in java is follow the notify / wait pattern. – Nymeria Jan 13 '15 at 14:03

3 Answers3

7

Firstly, I'd question the use of separate threads at all here. If you want one thing to happen after another, just use a single thread.

However, it's easy to wait until one thread has finished - just use join:

Thread t1 = new Thread(hello);
Thread t2 = new Thread(goodbye);
t1.start();
t1.join();
t2.start();

That way you don't need any synchronization within the "hello" or "goodbye" code.

If you want anything more complicated, I'd recommend looking in the java.util.concurrent package. While you can use wait() and notify(), it's generally a better idea to use higher-level constructs. For example, for a producer/consumer scenario, you may well want to use BlockingQueue rather than implement it all yourself.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Beaten to it by seconds. + 1 – Rudi Kershaw Jan 13 '15 at 13:40
  • Hello, I understand another method exist, but this is just an academic example for understand the thread synchronizing. – Nymeria Jan 13 '15 at 13:44
  • @Nymeria: Well if you want a solution that works for a different situation, you should show that different situation. It's very hard to produce a solution to requirements we don't actually know about. – Jon Skeet Jan 13 '15 at 13:55
  • I'm looking for synchronizing thread properly in java. I'm new in java and I wondering what is a good way for executing a thread before another (like producer / consumer problem) – Nymeria Jan 13 '15 at 14:06
  • 2
    @Nymeria: If you want to do *everything for a whole thread's operation* before starting the other one, then you use `Thread.join`. That's not even slightly like the producer/consumer pattern, however, where they're interleaved. There are plenty of articles around producer/consumer patterns in Java. – Jon Skeet Jan 13 '15 at 14:12
  • BlockingQueue sound very interesting ! Thank you a lot – Nymeria Jan 13 '15 at 14:23
3

You can achive that with help of CountDownLatch, simple example:

import java.util.concurrent.CountDownLatch;

public class Test {

    public static void main(String... s){

        final CountDownLatch cdl = new CountDownLatch(1);

        Thread t1 = new Thread(new Runnable() {

            @Override
            public void run() {
                int i = 5;
                while((i--)>0)
                    System.out.println("hello");
                cdl.countDown();
            }
        });


        Thread t2 = new Thread(new Runnable() {

            @Override
            public void run() {
                try {
                    cdl.await();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
                System.out.println("goodbye");
            }
        });

        t2.start();
        t1.start();
    }

}
alex2410
  • 10,904
  • 3
  • 25
  • 41
0

While John Skeet provided the correct way to do this type of thing, it may also be useful to understand exactly what the program flow error is as well.

In GoodbyeThread:

synchronized (lock) {
    System.out.println("Inside the synchronized");
    try {
        lock.wait();
    }

So, after aquiring the lock, you insist on waiting on it? The problem with this is that HelloThread has already terminated by the time GoodbyeThread aquires it; when HelloThread calls notifyAll, that does nothing because nobody is waiting on the lock yet.


Another issue is the use of Semaphore. As written, your classes do not make use of any feature of Semaphore, and in fact, will cast it to an Object if given one anyway. You should erase all mention of Semaphore from your code, as it is unnecessary and potentially confusing.

AJMansfield
  • 4,039
  • 3
  • 29
  • 50
  • This is exactly the problem. I'm looking for a solution to waiting "properly" in GoodBye thread. Many solution was given (observator pattern, countDownLatch) – Nymeria Jan 13 '15 at 14:01