views:

788

answers:

5

Hi,

I have a swing application which stores a list of objects. When the users clicks a button, I want to perform two operations on each object in the list, and then once that is complete, graph the results in a JPanel. I've been trying SwingWorker, Callable & Runnable to do the processing, but no matter what I do, while processing the list (which can take up to a few minutes, as it is IO bound), the GUI is locked up. I have a feeling it's probably the way I'm calling the threads or something, or could it be to do with the graphing function? That isn't threaded as it is very quick.

I have to do the two processing stages in order too, so what is the best way to ensure the second one has waited on the first? I've used join(), and then

while(x.isAlive())
{
Thread.sleep(1000);
}

to try and ensure this, but I'm worried this could be the cause of my problem too.

I've been looking everywhere for some pointers, but since I can't find any I'm sure I'm doing something stupid here. Any help would be greatly appreciated!

Thanks

Simon

+4  A: 

The problem is, your long running task is blocking the Thread that keeps the GUI responsive.

What you will need to do is put the long running task on another thread.

Some common ways of doing this are using Timers or a SwingWorker.

The Java tutorials have lots of information regarding these things in their lesson in concurrency.

To make sure the first task finishes before the second, just put them both on the same thread. That way you won't have to worry about keeping two different threads timed correctly.

Here is a sample implementation of a SwingWorkerFor your case:

public class YourTaskSwingWorkerSwingWorker extends SwingWorker<List<Object>, Void> {
    private List<Object> list
    public YourClassSwingWorker(List<Object> theOriginalList){
        list = theOriginalList;
    }

    @Override
    public List<Object> doInBackground() {
        // Do the first opperation on the list
        // Do the second opperation on the list

        return list;
    }

    @Override
    public void done() {
        // Update the GUI with the updated list.
    }
}

To use this code, when the event to modify the list is fired, create a new SwingWorker and tell it to start.

jjnguy
I'm pretty sure the question mentioned threads and SwingWorker already. Also, SwingUtilities.invokeLater() runs things on the GUI thread.
R. Bemrose
Yeah, I gave kinda' generic swing threading answer...kinda sucks
jjnguy
OP here, Yeah I've tried both of those suggestions already, but I may not have implemented them correctly... I've been through the swing tutorial, but it doesn't have much on swing worker at all, at least not the info I'm after
Simonw
Have a look at my edit, does it help?
jjnguy
That should be public Void done(), right?
Michael Myers
no, Void is used as a return type if you want to update progress of the worker. We aren't using it here at all. (I don't think)
jjnguy
I was too lazy to look up the Javadoc yesterday, but I see now that the second type parameter is used only as a parameter to publish() and process(). I retract my first comment.
Michael Myers
SwingUtilities.invokeLater() executes tasks on the event dispatch thread, which will cause the same problem at an unspecified future time. Bad advice.
Rob
@mmyers: I think you're thinking of protected void process( List<Void> chunks ), which is the signature for the method which processes intermediate data; void of course being the intermediate chunk type.
Rob
+2  A: 

You are not returning the swing thread properly. I realize you are using callable/runnable but i'm guessing you are not doing it right (although you didn't post enough code to know for sure).

The basic structure would be:

swingMethod() { // Okay, this is a button callback, we now own the swing thread
    Thread t=new Thread(new ActuallyDoStuff());
    t.start();
}

public class ActuallyDoStuff() implements Runnable {
    public void run() {
        // this is where you actually do the work
    }
}

This is just off the top of my head, but I'm guessing that you either aren't doing the thread.start and are instead calling the run method directly, or you are doing something else in the first method that locks it up (like thread.join). Neither of these would free up the swing thread. The first method MUST return quickly, the run() method can take as long as it wants.

If you are doing a thread.join in the first method, then the thread is NOT being returned to the system!

Edit: (Second edit actually) I think to speak to the problem you are actually feeling--you might want to think more in terms of a model/view/controller system. The code you are writing is the controller (the view is generally considered to be the components on the screen--view/controller are usually very tightly bound).

When your controller gets the event, it should pass the work off to your model. The view is then out of the picture. It does not wait for the model, it's just done.

When your model is finished, it needs to then tell the controller to do something else. It does this through one of the invoke methods. This transfers control back to the controller and you go on your merry way. If you think about it this way, separating control and deliberately passing it back and forth doesn't feel so bulky, and it's actually very common to do it this way.

Bill K
+1  A: 

It sounds like the problem might be that you are waiting on the threads to finish from inside the GUI thread. Your GUI thread should not wait on these threads, instead you should have the worker threads invoke some method on the GUI thread that sets a flag. When both flags are set then you know both threads finished and you can do the graph.

dotjoe
A: 

I can't really speak to the swing threading model, but:

I have to do the two processing stages in order too, so what is the best way to ensure the second one has waited on the first?

For this kind of functionality, I'd suggest you create two worker threads, and embed a JMS broker. Deliver work to the two threads by passing messages into JMS queues that they read from. Your GUI thread is free to examine the queues to determine when work is happening and represent the state of play in your UI.

Jherico
A: 

The solution to my problem was a mixture of jjnguy and Bill K's answers, so thanks very much for that guys. I needed to use threads within a SwingWorker like this:

public class Worker extends SwingWorker<Void, Void>   
{  
    private List<Object> list;  
    public YourClassSwingWorker(List<Object> theOriginalList){  
        list = theOriginalList;  
    }

    @Override
    public List<Object> doInBackground() {
        Thread t = new Thread(new ProcessorThread(list));  
        t.start();
    }

    @Override
    public void done() {
        // draw graph on GUI
    }
}  
class ProcessorThread implements Runnable {  
    //do lots of IO stuff  
    Thread t2 = new Thread(new SecondProcess());
    t2.start();  
}

This made sure all the work was being done by worker threads away from the GUI, and also ensuring that the SwingWorker itself wasn't doing all of the work, which might have been a problem.

Simonw
Whoa whoa whoa! Why are you spawning a thread from the SwingWorker? SwingWorkers (when started with the execute() method) are not run on the GUI thread, so there should be no reason to create another thread here.
Michael Myers
I must've been doing it wrong initially, because that was the only solution that worked for me
Simonw