[jsword-devel] Close All bug

[DM Smith]

This is a multi-part message in MIME format.

------=_NextPart_000_4ff7_a35_6ef4
Content-Type: text/plain; format=flowed

When doing a "Close All" it creates a ConcurrentModificationException 
because the "views" array list is modified while it is being iterated over 
by two different iterators.

This really was two bugs: The first was in the loop that removed the view 
and the second was in the setting up of an initial view to replace the last 
one that was removed.

The solution is simple, since the only access to the views list is via the 
Desktop class (that's what information hiding is good for). I merely changed 
the call that got the iterator to create a copy of the list and return that.

I also found a call in Desktop to next() on an iterator without first 
calling hasNext. According to the documentation for iterators they are 
always to be paired. By way of an example, I saw an iterator in Xerces that 
used the call to hasNext to set up the variable to be returned by next.

I have attached a patch that fixes the bug.

_________________________________________________________________
Free up your inbox with MSN Hotmail Extra Storage. Multiple plans available. 
http://click.atdmt.com/AVE/go/onm00200362ave/direct/01/

------=_NextPart_000_4ff7_a35_6ef4
Content-Type: text/plain; name="patchclose.txt"; format=flowed
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename="patchclose.txt"

Index: Desktop.java
===================================================================
retrieving revision 1.42
diff -u -r1.42 Desktop.java
--- Desktop.java	18 Mar 2004 09:29:11 -0000	1.42
+++ Desktop.java	18 Mar 2004 16:38:19 -0000
@@ -11,6 +11,7 @@
import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Iterator;
import java.util.List;

@@ -200,7 +201,10 @@

         // And setup the initial display area, by getting the first
         // BibleViewPane and asking it for a PassagePane.
-        last = ((BibleViewPane) 
iterateBibleViewPanes().next()).getPassagePane();
+        // According to the iterator contract hasNext has to be called 
before next
+        Iterator iter = iterateBibleViewPanes();
+        iter.hasNext();
+        last = ((BibleViewPane) iter.next()).getPassagePane();

         // Preload the PassageInnerPane for faster initial view
         InnerDisplayPane.preload();
@@ -430,11 +434,12 @@
     }

     /**
-     * Iterate through the list of views
+     * Iterate through a copied list of views
      */
     public Iterator iterateBibleViewPanes()
     {
-        return views.iterator();
+        Collection copy = new ArrayList(views);
+        return copy.iterator();
     }

     /**


------=_NextPart_000_4ff7_a35_6ef4--