Jeremy,
Pls find the patch attached.
Ta
Meeraj
-----Original Message-----
From: Meeraj Kunnumpurath [mailto:[EMAIL PROTECTED]
Sent: 24 July 2006 20:30
To: [email protected]
Subject: RE: [jira] Created: (TUSCANY-573) Race condition in
ThreadPoolWorkManager
>> I agree the order of callbacks will be correct. However, if there is
significant delay in scheduling the work the latency of the callbacks
would be unexpected - as a client, I would expect workAccepted to be
called before the scheduling delay (on enqueue) and workStarted after
the delay (on dispatch).
Ok, I take your point. I will make the changes and submit the patch.
-----Original Message-----
From: Jeremy Boynes [mailto:[EMAIL PROTECTED]
Sent: 24 July 2006 20:23
To: [email protected]
Subject: Re: [jira] Created: (TUSCANY-573) Race condition in
ThreadPoolWorkManager
On Jul 24, 2006, at 12:00 PM, Meeraj Kunnumpurath wrote:
> Jeremy,
>
> Apologies if you have received my earlier email (my internet mail
> account doesn't seem to work always)
I think this is one of them - the previous mail I have was at 10:11AM
PDT.
> Executors.newFoxedThreadPool() creates a thread pool backed by an
> unbounded queue. This means even when all threads are in use, work
> won't be rejected. In terms of when and where the workAccepted method
> is invoked, can we say is an implementation detail, as long as the
> order in which the callbacks are executed are guaranteed.
> Alternatively, we could wait on workAccepted callback on the monitor
> for the work item before calling workStarted.
I agree the order of callbacks will be correct. However, if there is
significant delay in scheduling the work the latency of the callbacks
would be unexpected - as a client, I would expect workAccepted to be
called before the scheduling delay (on enqueue) and workStarted after
the delay (on dispatch).
Invoking the callback whilst holding a monitor is normally a bad idea
but in this case I think it should be safe enough. There may be problems
if the workAccepted callbacks run too long but perhaps we just put that
down to user error.
--
Jeremy
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
This message has been checked for all email viruses by MessageLabs.
*****************************************************
You can find us at www.voca.com
*****************************************************
This communication is confidential and intended for the exclusive use of
the addressee only. You should not disclose its contents to any other
person.
If you are not the intended recipient please notify the sender named
above immediately.
Registered in England, No 1023742,
Registered Office: Voca Limited
Drake House, Three Rivers Court,
Homestead Road, Rickmansworth,
Hertfordshire, WD3 1FX
This message has been checked for all email viruses by MessageLabs.
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
This message has been checked for all email viruses by MessageLabs.
This message has been checked for all email viruses by MessageLabs.Index: ThreadPoolWorkManager.java
===================================================================
--- ThreadPoolWorkManager.java (revision 425093)
+++ ThreadPoolWorkManager.java (working copy)
@@ -47,6 +47,9 @@
// Map of work items currently handled by the work manager
private Map<DefaultWorkItem, WorkListener> workItems = new
ConcurrentHashMap<DefaultWorkItem, WorkListener>();
+ // A map of accepted work
+ private Map<DefaultWorkItem, Boolean> enqueuedWork = new
ConcurrentHashMap<DefaultWorkItem, Boolean>();
+
// Thread-pool
private Executor executor;
@@ -84,6 +87,10 @@
workItems.put(workItem, workListener);
}
workAccepted(workItem, work);
+ synchronized(workItem) {
+ enqueuedWork.put(workItem, Boolean.TRUE);
+ workItem.notify();
+ }
return workItem;
} else {
workItem.setStatus(WorkEvent.WORK_REJECTED);
@@ -198,6 +205,17 @@
* Overrides the run method.
*/
public void run() {
+ synchronized(workItem) {
+ try {
+ while(!enqueuedWork.containsKey(workItem)) {
+ workItem.wait();
+ enqueuedWork.remove(workItem);
+ }
+ } catch(InterruptedException ex) {
+ // TODO is there a better exception to
throw
+ throw new RuntimeException(ex);
+ }
+ }
workStarted(workItem, decoratedWork);
try {
decoratedWork.run();
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
