Yeah I noticed that mistake after I posted in the forum. I got that corrected and also, as mentioned earlier, I moved the synchronization inside the loop.
In relation to this post I have couple of questions related to synchronization and locking which I wanted to ask here.
1) In SCJP preparation book, I vaguely remember reading, sleep() would not release the locks the thread holds and calling wait() on an object would have the thread release its lock on that object. Now, what would happen if both are used together. Would there be any negative impact? I do not think so, as they don't happen at the same time anyways.
2) In an online resource, I read that synchronization needs to be called at the method level of the object, on which a lock needs to be attained but never using a synchronization block.
IMHO, synchronized blocks are cool compared to method synchronization. But when coming to inter thread communication, using wait(), notify() is it always advisable to use synchronized methods on the object than synchronized blocks. If yes, please justify.
I would say nothing disastrous. Lets say we have threads A and B synchronized on common object. Thread A sleeps and then waits. They both will simply do nothing during sleep and then as soon as wait phase starts, the action resumes (in thread B). Probably such a combination might be even used for some fine-tuned thread orchestration, for example if you want to do some processing in thread B in some period of time (on the other hand, it would make much more sense to put sleep() in thread B in this case).
synchronization needs to be called at the method level of the object
Actually I am interested in this question myself. In fact I discovered recently that in C# it is not recommended to declare public methods as synchronized (as well as using this as a monitor lock, instead it is encouraged to use lock() (which is equivalent to synchronized block in Java) on some other object whenever possible. The reason is simple. If you declare public non-static method as synchronized, somewhere else the object which has these methods might be used as a monitor lock itself. This can be quite confusing, one can easily "lose" the track of locks and end up in something terrible like deadlock.
I think the main idea is to keep things as simple for reviewers as possible. Sometimes synchronized methods are fine, in other situations it is better to introduce a dummy private object and use it as a monitor lock.