3
\$\begingroup\$

I wrote those examples to show different approaches to sharing results between threads. The whole code is on GitHub. All examples should return "Hello world".

Are these OK from the perspective of data visibility?

The most hardcore one:

import com.google.common.util.concurrent.Uninterruptibles;

import java.util.concurrent.TimeUnit;
public class WaitNotifyExample {

    static String result1;
    static String result2;

    private static final Object object = new Object();

    public static void main(String[] args) {

        Thread producer1 = new Thread() {
            @Override
            public void run() {
                Uninterruptibles.sleepUninterruptibly(200, TimeUnit.MILLISECONDS);
                result1 = "Hello";
                synchronized (object) {
                    object.notify();
                }
            }
        };

        Thread producer2 = new Thread() {
            @Override
            public void run() {
                Uninterruptibles.sleepUninterruptibly(400, TimeUnit.MILLISECONDS);
                result2 = "world";
                synchronized (object) {
                    object.notify();
                }
            }
        };

        producer1.start();
        producer2.start();

        synchronized (object) {
            while(true) {
                try {
                    object.wait();
                } catch (InterruptedException e) {
                    System.out.println("Interrupted!");
                }

                System.out.println("Notified!");

                if(result1 != null && result2 != null) {
                    System.out.println(result1 + " " + result2);
                    break;
                } else {
                    System.out.println("Not ready");
                }
            }
        }


    }
}
\$\endgroup\$
3
\$\begingroup\$

Your code here is pretty mixed up and is not representative of a decent test of concurrency. It will work and give you the results you expect, but the reason is a bit polluted.

The internal code for System.out.println() looks like:

public void println(String x) {
    synchronized (this) {
        print(x);
        newLine();
    }
}

As a result, there are many more synchronizations than you expect. These are nested inside another synchronization block.

In essense, any time there is a println() you have a possibly conflicting synchronization problem.

In this case, it does not make much of a difference, but your code concerns me.

Additionally, it is traditional to incorporate the wait-conditions in the while-loop, and not as a break condition. Your code:

   synchronized (object) {
        while(true) {
            try {
                object.wait();
            } catch (InterruptedException e) {
                System.out.println("Interrupted!");
            }

            System.out.println("Notified!");

            if(result1 != null && result2 != null) {
                System.out.println(result1 + " " + result2);
                break;
            } else {
                System.out.println("Not ready");
            }
        }
    }

Would be better as:

   synchronized (object) {
        while(result1 == null && result2 == null) {
            object.wait();
        }
    }
    System.out.println(result1 + " " + result2);

Note that I have stripped out your InterruptException handling. It was not sufficient for "real" code, and if you replace it, should probably go outside the calls. If someone interrupts your code, continuing in a loop is probably not the right thing to do.

\$\endgroup\$

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.