1
\$\begingroup\$

I saw How to wait current thread execution and execute another thread completedly? on StackOverflow, and I answered it there, but would like more eyes on the code I posted.

The requirement is to pause one thread whilst a different thread is running.

Could you please review and comment on this implementation?

public class App {

    public static void main(String[] args) {
        Thread1 thread1 = new Thread1();
        Thread2 thread2 = new Thread2();

        try {
            thread1.start();
            Thread.sleep(500);
            synchronized (thread1) {
                thread1.waiting = true;
                thread2.start();
                thread2.join();
                thread1.waiting = false;
                thread1.notify();
            }
        } catch (Exception e) {
            //TODO actually handle exception
        }
    }
}

Class Thread1.java

public class Thread1 extends Thread {
    boolean waiting = false;

    public void run() {
        testFun1();
    }

    public void testFun1() {
        for (int i = 1; i < 10; i++) {
            synchronized (this) {
                while (waiting) {
                    try {
                        wait();
                    } catch (Exception e) {
                        //TODO Handle exception
                    }
                }
            }
            try {
                Thread.sleep(100);
                System.out.println("From testFun1() = " + i);
            } catch (Exception e) {
                //TODO Handle exception

            }
        }
    }
}

Class Thread2.java

public class Thread2 extends Thread {

    public void run() {
        testFun2();
    }

    public void testFun2() {
        try {
            for (int i = 20; i <= 25; i++) {
                try {
                    Thread.sleep(100);
                } catch (InterruptedException e) {
                    //catch
                }
                System.out.println("From testFun2() = " + i);
            }

        } catch (Exception e) {
            //TODO do something
        }

    }
}

Output:

C:\Java\jdk1.6.0_25\bin\java -Didea.launcher.port=7543 "-Didea.launcher.bin.path=C:\Program Files\JetBrains\IntelliJ IDEA 10.5.1\bin" -Dfile.encoding=UTF-8 -classpath "C:\Java\jdk1.6.0_25\jre\lib\alt-rt.jar;C:\Java\jdk1.6.0_25\jre\lib\alt-string.jar;C:\Java\jdk1.6.0_25\jre\lib\charsets.jar;C:\Java\jdk1.6.0_25\jre\lib\deploy.jar;C:\Java\jdk1.6.0_25\jre\lib\javaws.jar;C:\Java\jdk1.6.0_25\jre\lib\jce.jar;C:\Java\jdk1.6.0_25\jre\lib\jsse.jar;C:\Java\jdk1.6.0_25\jre\lib\management-agent.jar;C:\Java\jdk1.6.0_25\jre\lib\plugin.jar;C:\Java\jdk1.6.0_25\jre\lib\resources.jar;C:\Java\jdk1.6.0_25\jre\lib\rt.jar;C:\Java\jdk1.6.0_25\jre\lib\ext\dnsns.jar;C:\Java\jdk1.6.0_25\jre\lib\ext\localedata.jar;C:\Java\jdk1.6.0_25\jre\lib\ext\sunjce_provider.jar;C:\Java\jdk1.6.0_25\jre\lib\ext\sunmscapi.jar;C:\Java\jdk1.6.0_25\jre\lib\ext\sunpkcs11.jar;C:\IdeaProjects\PracticeModule\target\classes;C:\Program Files\JetBrains\IntelliJ IDEA 10.5.1\lib\idea_rt.jar" com.intellij.rt.execution.application.AppMain jmhp.core.App
From testFun1() = 1
From testFun1() = 2
From testFun1() = 3
From testFun1() = 4
From testFun1() = 5
From testFun2() = 20
From testFun2() = 21
From testFun2() = 22
From testFun2() = 23
From testFun2() = 24
From testFun2() = 25
From testFun1() = 6
From testFun1() = 7
From testFun1() = 8
From testFun1() = 9

Thanks!

\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

The implementation looks fine. Some small notes:

  1. Consider using Guava's Uninterruptibles class if you don't want to handle InterruptedExceptions.
  2. The default access waiting flag in Thread1 is makes the code hard to read/follow and results tight coupling.
  3. Consider using Runnables instead of Threads. See: Java: “implements Runnable” vs. “extends Thread”
  4. Consider synchronizing on a separate lock object: Avoid synchronized(this) in Java?

Anyway, it would be much easier, less error-prone and more readable with a fair Semaphore (from the concurrency utilities package):

App.java:

public class App {

    public static void main(final String[] args) throws InterruptedException {
        final Semaphore control = new Semaphore(1, true);

        final Thread1 thread1 = new Thread1(control);
        final Thread2 thread2 = new Thread2();

        thread1.start();
        Uninterruptibles.sleepUninterruptibly(500, TimeUnit.MILLISECONDS);
        control.acquireUninterruptibly();
        thread2.start();
        thread2.join();
        control.release();
    }
}

Thread1.java:

public class Thread1 extends Thread {
    private final Semaphore control;

    public Thread1(final Semaphore control) {
        this.control = control;
    }

    @Override
    public void run() {
        for (int i = 1; i < 10; i++) {
            control.acquireUninterruptibly();
            try {
                Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS);
                System.out.println("From testFun1() = " + i);
            } finally {
                control.release();
            }
        }
    }
}

Thread2.java:

public class Thread2 extends Thread {

    public void run() {
        for (int i = 20; i <= 25; i++) {
            Uninterruptibles.sleepUninterruptibly(100, TimeUnit.MILLISECONDS);
            System.out.println("From testFun2() = " + i);
        }

    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Wow! Thanks a lot, threads is a extensive theme with lots of things that can be learned! like i just did! :) \$\endgroup\$
    – Jhurtado
    Aug 9, 2012 at 22:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.