which object will this synchronize on?

[Daniil Sosonkin]

hmmm, a question, would this make sence?

ArrayList messages = new ArrayList();
 
..... addMessage .....
synchronized (messages) {
   messages.add(message);
   messages.notify();
}
....
 

..... run() .....
synchronized (messages) {
   messages.wait();
}
.....
 
..... sendingMessages() .....
ArrayList pending;
synchronized (messages) {
   pending = messages;
   messages = new ArrayList();
}
 
sendPending(pending);
......

As I understand, there wouldn't be a deadlock or memory leak. Can any of you experts trash my understanding or confirm it?

thanx

[Peter den Haan]

No, it doesn't quite make sense yet...

ArrayList messages = new ArrayList();

Please code to interface (List) rather than implementation.

..... run() .....
synchronized (messages) {
   messages.wait();
}
.....

I'm missing a wait condition here. Also, given that you notify() and not notifyAll(), make sure that you check the condition before launching into a wait().

..... sendingMessages() .....
ArrayList pending;
synchronized (messages) {
   pending = messages;
   messages = new ArrayList();
}

But this is the killer. All wait()ing threads are waiting on the old messages object now, never to be woken up What you should do is either

List pending;
synchronized (messages) {
   pending = new ArrayList(messages);
   messages.clear();
}

Or perhaps not synchronize on the messages list at all, but on the object which wraps it. The latter would probably be my preference as you could use easier-to-read synchronized methods.

- Peter

[Daniil Sosonkin]

Thanks! Frankly, I don't know where my brain was See, this is a the advantage of C++ - you explicitly have to deallocate an object and thus you know where you loose the reference. In Java, not much attention payed so one does really stupid stupid things.

What I have now is two objects: ArrayList messages and Object lock. And synchronize/wait/notify on lock object. In my case one thread adds messages while another sends them out. There can be a lot of messages, so I don't want to copy them from one object to another and rather create whole new object. Thats why I synchronize:

ArrayList pending;
synchronized (lock) {
   pending = messages;
   messages = new ArrayList();
}

and now its waiting on the lock:

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

[Mr. C Lamont Gilbert]

advantage of c++? If this were c++ it would fail in exactly the same way, except as a bonus you get a memory leak

anyway. you should always use a seperate lock object. Some list implementations, particularly the synchronized ones may take acception to you using them as the monitor. treat your lock as any other object, and do not expose it to the public, only those that require it.