[jsword-devel] Close All bug

Joe Walker jsword-devel@crosswire.org
Thu, 18 Mar 2004 18:18:26 +0000


Thanks for this too.
I didn't realize that about Iterator.
Applied.

Joe.

DM Smith wrote:

> 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/
> 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();
>     }
>
>     /**
>