Simple java synchronization question

Question

In Groovy code something simple: #!/usr/bin/env groovy

public class test {
  boolean val
  def obj=new Object()

  def dos() {
    val=false
    Thread.start() {
      synchronized(obj) {
    val=true
    obj.notifyAll()
      }
    }
    Thread.sleep(5000)
    synchronized(obj) {
      while (!val) {
    obj.wait()
      }
    }
  }

  static void main(String[] args) {
    def t=new test()
    t.dos()
  }
}

---

Ok, here is my problem in more detail.

Thread (A) start an action in a separate thread, then wait for its completion -- OK THIS IS NOT EXACTLY TRUE OTHERWISE COULD USE thread.join(). This Thread actually starts a task that then signals methodOne eventually

Thread (B) we get a signal when action is completed

class A {
   private boolean finished

   public synchronized void methodOne() {
       finished=true;
       notifyAll();
   } 

   public void methodTwo() {
       new ThreadThatCallsMethodOneWhenDone().start();
       synchronized(this) {
           while (!finished) {
                 wait();
           }
       }
   }
}

Is this code okay or am I still running into potential problems? What's a better way to solve?

Misha

---

I was wondering, which is correct:

Option One

class A {
   public void methodOne() {
       synchronized(this) {
           modifyvalue
           notifyAll()
       }
   }

   public void methodTwo() {
       while (valuenotmodified) {
           synchronized(this) {
              wait()
           }
       }
   }

Option Two

class A {
   public void methodOne() {
       modifyvalue
       synchronized(this) {
           notifyAll()
       }
   }

   public void methodTwo() {
       while (valuenotmodified) {
           synchronized(this) {
              wait()
           }
       }
   }

and why?

Answer 1

I think that both are dangerous because your valuenotmodified check is performed without synchronisation. So, there is no telling what happens if methodOne modifies the value while methodTwo is in process of verifying whether it has changed.

And I see no difference between your two "options". Both have this potential problem.

Answer 2

All access to "value" should be synchronized:

class A {
   public void methodOne() {
       synchronized(this) {
           modifyvalue
           notifyAll()
       }
   }

   public void methodTwo() {
       synchronized(this) {
           if (valuenotmodified) {
              wait()
           }
       }
   }

}

Note that this is equivalent to:

class A {
   public synchronized void methodOne() {
       modifyvalue
       notifyAll()
   }

   public synchronized void methodTwo() {
       if (valuenotmodified) {
          wait()
       }
   }
}

Answer 3

Problems like this are better handled by the concurrency library, first released in 1998, it became part of JDK 5 in 2004. I suggest you learn how to use these as they are usually much easier to use and understand than the notify/notifyAll/wait constructs.

In your case you could use Condition In its javadoc it comments

Condition factors out the Object monitor methods (wait, notify and notifyAll) into distinct objects to give the effect of having multiple wait-sets per object, by combining them with the use of arbitrary Lock implementations. Where a Lock replaces the use of synchronized methods and statements, a Condition replaces the use of the Object monitor methods.