data shared between threads and synchronized on different objects

[Marcin Rodzik]

Hello,

I have a thread (the essential piece of its code can be found here:
http://pastebin.com/KM8Yiqgs) which sends some objects ("tasks") over
the network (in method tryToSendTask). Another thread submits objects
to be sent by means of the first thread's method submit - namely, the
"task" is put into queue.

The sending thread "goes to sleep" when the queue is empty but is
woken up if someone calls its submit method. The problem is that we
need to flush the stream before letting the thread sleep. It might be
a little time-consuming operation, so I do it outside synchronized
section (lines 25-31). The problem is that meanwhile someone can add a
new task to the queue. Moreover, before going definitely to sleep
(line 35), I check some flags which can tell me that someone wants my
thread to finish (goOn=false means finishing no matter what,
waitableClosing=true means that we should finish the thread after
sending out the last task from the queue, i.e. only if the queue is
already empty).

My biggest concern is to avoid the following scenario:
- due to discovering that the queue (line 19) is empty the output
stream is flushed
- during flush some objects are added to the queue and flag
waitableClosing is set to false
- let's assume that there is no line 29 in my code
- due to waitableClosing=true the thread ends (goOn=false is set in
line 31)
- as a result, last task objects are never sent

This scenario is something I didn't think about on the beggining, so
after some debugging I decided to add line 29 But now I'm afraid of
the weird synchronization thing. Let's assume that the flags goOn and
waitableClosing are synchronized on the this reference, but the task
queue has its own synchronization. Is it possible that my thread will
see the old (not refreshed) state of the queue (i.e. empty) but the
new value of the flag responsible for ending the thread? Which would
mean that the bug is still present in my code...

A possible remedy involves adding the synchronized attribute to the
entire submit method, but using a synchronized collection then seems
to be an overkill. On the other hand, we have to add a synchronized
section around line 24 where we read the state of the queue...

....don't we? Maybe we don't. Determining the queue is empty costs us
flushing the stream, but then the state of the queue will be checked
again in line 34 (inside the synchronized section).

What do you think?

Marcin

 

[Tom Anderson]

I have a thread (the essential piece of its code can be found here:
http://pastebin.com/KM8Yiqgs) which sends some objects ("tasks") over
the network (in method tryToSendTask). Another thread submits objects
to be sent by means of the first thread's method submit - namely, the
"task" is put into queue.

[...]

What do you think?

I think you can make this a good bit simpler by using a BlockingQueue,
such as a LinkedBlockingQueue. Any concurrency advantage a
ConcurrentLinkedQueue might have is lost by the use of a synchronized
method in your object.

With a BlockingQueue, the code looks more like:

public void submit(T t) {
t.put(t);
}

public void run() {
long timeout = Long.MAX_VALUE;
while (goOn) {
T t = taskSendingQueue.poll(timeout, TimeUnit.SECONDS);
if (t != null) {
sendTask(t);
timeout = 0;
}
else {
taskOutputStream.flush();
timeout = Long.MAX_VALUE;
}
}
completeCommunication();
}

I am mildly abusing poll here, by using extreme values of the timeout to
effectively switch between blocking and non-blocking modes.

I don't know what this waitableClosing thing is, but i suspect you should
combine it with goOn. A loop doesn't need more than one condition
variable.

tom

 

[markspace]

I have a thread (the essential piece of its code can be found here:
http://pastebin.com/KM8Yiqgs) which sends some objects ("tasks") over
the network (in method tryToSendTask). Another thread submits objects
to be sent by means of the first thread's method submit - namely, the
"task" is put into queue.

[...]

What do you think?

I think you can make this a good bit simpler by using a BlockingQueue,

That's funny. Marcin (Marteno?) posted this same question twice. I
basically told him the exact same thing 6 hours ago, sans the nice code
example.

 

[Tom Anderson]

I have a thread (the essential piece of its code can be found here:
http://pastebin.com/KM8Yiqgs) which sends some objects ("tasks") over
the network (in method tryToSendTask). Another thread submits objects
to be sent by means of the first thread's method submit - namely, the
"task" is put into queue.

[...]

What do you think?

I think you can make this a good bit simpler by using a BlockingQueue,

That's funny. Marcin (Marteno?) posted this same question twice. I
basically told him the exact same thing 6 hours ago, sans the nice code
example.

How embarrassing. I must pay more attention to what's going on around
here.

tom

 

[Tom Anderson]

On 8/2/2011 2:41 PM, Tom Anderson wrote:
On Tue, 2 Aug 2011, Marcin Rodzik wrote:

I have a thread (the essential piece of its code can be found here:
http://pastebin.com/KM8Yiqgs) which sends some objects ("tasks") over
the network (in method tryToSendTask). Another thread submits objects
to be sent by means of the first thread's method submit - namely, the
"task" is put into queue.

[...]

What do you think?

I think you can make this a good bit simpler by using a BlockingQueue,

That's funny. Marcin (Marteno?) posted this same question twice. I
basically told him the exact same thing 6 hours ago, sans the nice
code example.

How embarrassing. I must pay more attention to what's going on around here.

Maybe duplicate, independent recommendations for the same solution will
encourage the OP to look at it.

True. Although really, someone who repeat-posts mostly needs encouraging
not to do that.

Also, the OP has no way of knowing that i hadn't just read Mark's answer
and added some more curly brackets to it!

tom

 

[supercalifragilisticexpialadiamaticonormalizeringe]

True. Although really, someone who repeat-posts mostly needs encouraging
not to do that.

Also, the OP has no way of knowing that i hadn't just read Mark's answer
and added some more curly brackets to it!

It's not that. The OP is, unfortunately, using Google Groups, and Google
Groups has lately been lagging hours behind other newsservers. He
reposted his original query because it didn't appear in (his view of)
the newsgroup in a reasonable timeframe, let alone get a reply that he
could see.

Of course, he won't see this information for hours either ...

 

[Marcin Rodzik]

It's not that. The OP is, unfortunately, using Google Groups, and Google

Groups has lately been lagging hours behind other newsservers. He
reposted his original query because it didn't appear in (his view of)
the newsgroup in a reasonable timeframe, let alone get a reply that he
could see.

Of course, he won't see this information for hours either ...

Yes, exactly. I'm really sorry for my double (tripple?) post I really believed that my post was lost, so I repeated it.

Meanwhile, I was on holidays, so this is why I'm writing this words after such a long time. Thank you for all your answers.

Marcin aka Marteno