Re: Join thread with SwingWorker objects
Douwe wrote:
Where should I start :/. First of all the field
Where should I start? The example you provided is rife with
unsynchronized access to shared data.
... Try implementing
something like the following piece of code
private final Object lock = new Object();
private volatile int countCompleted;
pricate volatile int activeProcesses;
You're controlling these variables with different monitors - sometimes
'lock', sometimes the internal one due to being 'volatile'. I'm not
convinced that the relationship between these variables is reliably
readable.
public void processCompleted(BatchExtractionItem completedItem) {
//System.out.println("Completed "+completedItem);
synchronized(lock) {
activeProcesses--;
countCompleted++;
lock.notifyAll(); // wake up =
the main thread
}
}
enum State { WAITING, RUN_NEXT };
protected Integer doInBackground() throws Exception {
List<BatchExtractionItem> queuedItems = new
ArrayList<BatchExtractionItem>();
queuedItems.addAll(items);
activeProcesses = 0;
countCompleted = 0;
boolean keepRunning = true;
BatchExtractionItem itemToRun = null;
while (keep_running) {
switch(state) {
The read of 'state' is not synchronized with the write.
case WAITING : {
synchroni=
zed(lock) {
=
if (activeProcesses<MAX_ACTIVE_PROCESSES) {
=
if (queuedItems.isEmpty()) {
=
if (activeProcesses==0) {
=
keep_running = fa=
lse;
=
break;
=
}
=
} else {
=
state = FIND_TO_RUN;
=
break;
=
}
=
}
=
try {
=
lock.wait(20000l); // wa=
it for 20 seconds max
(or a notify from processCompleted) and then check again
=
} catch(Exception ignore) {}
}
} break;
case RUN_NEXT : {
BatchExtr=
actionItem item = queuedItems.removeLast(queuedItems);
You don't synchronize the change to 'queuedItems'.
if (!item=
..getStatus().equals(BatchExtractionItem.COMPLETED) && !
or the 'getStatus()' read.
item.getStatus().equals(BatchExtractionItem.PROCESSING)) {
=
item.setStatus(BatchExtractionItem.PROCESSING);
=
BatchTask task = new BatchTask(item);
=
task.execute();
=
activeProcesses++;
} else {
=
// spitt out a warning
=
System.err.println("warn: next item was already processing or has
completed");
=
// countCompleted++; // add this if it should be counted as a
completed task
}
state ==
WAITING;
} break;
}
}
// no further checking needed ... all items have finised =
processing
return 0;
}
@Override
public void actionPerformed(ActionEvent e) {
//issued by Timer event
int progress = countCompleted/items.size();
setProgress(progress);
form.getProgressBar().setValue(progress);
}
I'm having difficulty reasoning about the synchronization the way
you've written all this. I suspect there are subtle threading bugs
there.
--
Lew