views:

334

answers:

3

Hello, I've got lots of JTree in my app, all using different instances (with different options) of the same custom TreeModel class. I'm trying to add some threading to speed things up, since the getChildren() member takes a while to run, I added a SwingWorker subclass to the TreeModel and in my getChildren() I create an instance of that worker and execute it, then return it's get() result.

I keep getting ConcurrentModificationExceptions, and I know I'm supposed to synchronize something, but not sure what. Anyone got any suggestions on how to keep my TreeModel responsive safely?

Thank you! Joshua

+2  A: 

Firstly, I wonder why you are attempting to use threads to speed things up. Is it a CPU bound operation, or network/disk bound?

Your description reads as if your getChildren wait for the operation to complete before returning. That could be useful for parallelisable operation on multi-core hardware, but isn't the general model. Usually what we (and what SwingWorker does) is to run code the blocks on another thread, whilst the Event Dispatch Thread is free to carry on until it receives an event to process the data from the background thread.

SwingWorker doesn't document that it throws ConcurrentModificationException anywhere. A stack trace might be helpful.

I generally suggest avoiding SwingWorker. It's great for creating a little demo operation, but imposes a poor design.

Threading is difficult, especially now that multithreaded hardware actually more bugs. Liberal sprinkling of synchronized will not make problems go away. A relatively easy general approach is to avoid sharing mutable objects (or at least mutating shared objects). Pass a set of arguments into the background thread that are not being used in the EDT. Likewise, pass arguments back into the EDT which are not being otherwise modified.

Tom Hawtin - tackline
+1  A: 

ConcurrentModificationExceptions happen when you access the same List on two threads. Your TreeModel probably uses ArrayLists, Vectors, Hashtables, or something similiar to store the nodes.

My guess is that one of two things is happening:

1) Your TreeModel is constantly being queried by the JTree on the EDT thread every time it is rendered. This is unavoidable and how Swing works. If you are accessing the tree model, or underlying Lists on a different thread, you'll occasionally do it at the same time as a rendering and List throws the exception.

2) you have two swing workers running, each on their own threads. They are simultaneously updating/querying the List. Same problem as above.

I aggree with Tom that using SwingWorker to make your TreeModel "asynchronous" is a very tricky problem and should be avoided. But, to answer your question, I would look at the following piece of code. Notice how all query and update actions to the tree takes place in the "finished" method, which is always run on EDT. My guess is that you are doing gets/sets in the construct() method.

     public TreeNode[] getChildren(TreeNode parent){

         // Do all normal work you would do
         ...


         // figure out if this parents children have not been fetched yet
         boolean needToFetch = ....

         if(needToFetch){

          worker = new SwingWorker() {
           public Object construct() {
            // go fetch your children from whatever database, server, file...
            // fetchNewChildren must NOT access the TreeModel or any of the underlying
            // lists, because you will get a modification exception.  This is what
            // Tom meant by "Pass a set of arguments into the background thread 
            // that are not being used in the EDT"

            return fetchNewChildNodes(parent);
          }

          public void finished() {
            List<TreeNode> newNodes = (List<TreeNode>)get();

            // insert the newly fetched nodes into the parent
            // if done correclty, this will fire TreeModelChanged events
            // so the tree should re-render, repaint, etc...
            parent.addChildren(newNodes); 
         }
         };

         worker.start();
}
Andrew
A: 

Perhaps try storing data in your TreeModel in a CopyOnWriteArrayList

basszero