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