3

I have two blocks of code, one waits for the other to notify it.

synchronized(this) {
    wait();
}

and

while(condition) {
    //do stuff
    synchronized(this) {
        notify();
    }
}

Weirdly enough that didn't wait for the notify while this did:

synchronized(objectLock) {
    objectLock.wait();
}

and

while(condition) {
    //do stuff
    synchronized(objectLock) {
        objectLock.notify();
    }
}

I'm very curious about the difference of both sets, and why the first one worked while the other didn't. Note that the two blocks reside in two different threads on two different methods (if that helps).

I hope someone could explain why this is so. I edited my question so it would be more detailed.

Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
Chad
  • 2,041
  • 6
  • 25
  • 39
  • 3
    It's very hard to tell *just* from that code snippet. We've no idea what you mean by "didn't work" nor what the rest of your code is doing. – Jon Skeet May 23 '13 at 14:26
  • By doesn't work, I meant it didn't wait. I'm not sure if that's enough but that is pretty much the gist of everything. – Chad May 23 '13 at 14:27
  • In most use cases, it is the `wait` call that is put inside the condition-loop, not `notify`. I am almost certain this applies to your case as well. I.e. it should be `while(!condition) { wait(); }` and `notify()` elsewhere. If you use the Monitor pattern, you'd also get the benefit of encapsulation, and wouldn't need the synchronized blocks (you'd have `synchronized` methods instead.) – Armen Michaeli May 23 '13 at 14:39

4 Answers4

2

You can use any object you like. However, it is generally clearer to other programmers to see an explicit lock object.

My wild guess as to why this didn't work for you is you had a different this in scope. (ie, in an anonymous function/callback). You can be explicit about which this to use by appending the class name, eg, WonderClass.this - again a reason why this is not as clear. (edit: actually WhateverClass.this won't help you if this really is a different instance)

Also do read this: http://docs.oracle.com/javase/tutorial/essential/concurrency/locksync.html - I generally find it easier to put all the thread-unsafe code into small synchronized methods (which do an implict lock on this)

Tom Carchrae
  • 6,398
  • 2
  • 37
  • 36
2

It didn't work because you synchronized on this which in two different threads pointed to two different Thread objects.

Synchronization with wait() and notify() would only work properly when you synchronize on the same object for locking like the objectLock that you used later on.

EDIT: If the two thread instances belonged to the same MyThread class then to achieve the effect that you thought you're code was having, you would have to acquire a lock on their class object itself:

synchronized(MyThread.class)
Ravi K Thapliyal
  • 51,095
  • 9
  • 76
  • 89
  • But isn't `this` supposed to point to the instance of the class where both methods belong to which is also true for this case? – Chad May 23 '13 at 14:35
  • If the two thread instances belong to the same `MyThread` class then to achieve what you want you should have locked on `synchronized(MyThread.class)` object. – Ravi K Thapliyal May 23 '13 at 14:39
  • Now I kind of got it, since both were residing in two different threads even if the the methods were both in the same instance, this would refer to a thread not the calling object. Cheers. – Chad May 23 '13 at 14:40
  • Yes, because the instances that the two "this" were pointing at were different (although if their class was the same). Simple! – Ravi K Thapliyal May 23 '13 at 14:40
  • You should avoid locking on this or other publicly accessible object like MyThread.class. Others' code may accidentally interact with it as well. I think use a dedicated and private lock object is the best practice. – minhle_r7 May 23 '13 at 14:51
  • @ngọcminh.oss Yes, you're correct. In fact, using a ReentrantLock would let us use locking in other advanced ways. (tryLock() etc.) But, I think OP here is just trying to learn how the fundamentals work first. – Ravi K Thapliyal May 23 '13 at 14:59
1

When you say the two blocks reside in two different threads that makes me think they're not locking on the same object because this is not the same thing. When you name an explicit lock you're using the same thing to lock on.

By the way you should call wait in a loop, like this:

synchronized(someLock) {
   while (!someCondition) {
       wait();
   }
   // now the thread has the lock and it can do things 
   // knowing for sure that someCondition is true
}

Without this you will be vulnerable to spurious wakeups (not all notifications come from your application code) and the order in which wait and notify are called becomes problematic (if you have two threads and one notifies before the other waits then that notification never gets seen).

Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
1

I'd advise using the Monitor pattern (http://en.wikipedia.org/wiki/Monitor_(synchronization)) anyway, that could save you from errors later on, especially as your use case gets more complex:

class Monitor
{
    /** Initialised to `false` by default in Java. */
    boolean condition;

    synchronized void waitForSomething()
    {
        while(!condition)
        {
            wait();
        }
    }

    synchronized void signal()
    {
        condition = true;

        notify();
    }
}

That way everything is nicely encapsulated and protected (I don't usually use private modifiers in examples, but you might want to enforce additional "privacy" in your code, specifically making the condition private.)

As you can observe, in my condition loop there is wait() call, as opposed to your example where you have notify() in the loop instead. In most use cases doing what you did with notify is a mistake, although I can't speak for your particular case as you didn't provide us with enough details. I am willing to bet yours is the typical one though, for which the Monitor pattern applies beautifully.

The usage scenario is along the following: thread that wants to wait for something calls waitForSomething and another thread may cause it to continue by invoking signal method which will set the condition flag.

Armen Michaeli
  • 8,625
  • 8
  • 58
  • 95