scolebourne 2003/09/27 03:07:14
Modified: collections/src/java/org/apache/commons/collections
CursorableLinkedList.java
collections/src/test/org/apache/commons/collections
TestCursorableLinkedList.java
Log:
Change CursorableLinkedList to use weak references to avoid memory leaks
from Simon Kitching
Revision Changes Path
1.18 +72 -19
jakarta-commons/collections/src/java/org/apache/commons/collections/CursorableLinkedList.java
Index: CursorableLinkedList.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/CursorableLinkedList.java,v
retrieving revision 1.17
retrieving revision 1.18
diff -u -r1.17 -r1.18
--- CursorableLinkedList.java 20 Sep 2003 14:03:57 -0000 1.17
+++ CursorableLinkedList.java 27 Sep 2003 10:07:14 -0000 1.18
@@ -69,6 +69,7 @@
import java.util.List;
import java.util.ListIterator;
import java.util.NoSuchElementException;
+import java.lang.ref.WeakReference;
/**
* A doubly-linked list implementation of the [EMAIL PROTECTED] List} interface,
@@ -88,10 +89,9 @@
*
* @author Rodney Waldhoff
* @author Janek Bogucki
+ * @author Simon Kitching
*/
public class CursorableLinkedList implements List, Serializable {
- // TODO: use weak references to cursors in case they aren't closed directly
-
/** Ensure serialization compatability */
private static final long serialVersionUID = 8836393098519411393L;
@@ -304,11 +304,9 @@
* [EMAIL PROTECTED] ListIterator#nextIndex} and [EMAIL PROTECTED]
ListIterator#previousIndex}
* methods (they throw [EMAIL PROTECTED] UnsupportedOperationException} when
invoked.
* <p>
- * Clients must close the cursor when they are done using it.
- * The returned [EMAIL PROTECTED] ListIterator} will be an instance of
- * [EMAIL PROTECTED] CursorableLinkedList.Cursor}. To close the cursor,
- * cast the [EMAIL PROTECTED] ListIterator} to [EMAIL PROTECTED]
CursorableLinkedList.Cursor}
- * and invoke the [EMAIL PROTECTED] CursorableLinkedList.Cursor#close} method.
+ * Historical Note: In previous versions of this class, the object
+ * returned from this method was required to be explicitly closed. This
+ * is no longer necessary.
*
* @see #cursor(int)
* @see #listIterator
@@ -839,7 +837,16 @@
* of changes to this list.
*/
protected void registerCursor(Cursor cur) {
- _cursors.add(cur);
+ // We take this opportunity to clean the _cursors list
+ // of WeakReference objects to garbage-collected cursors.
+ for (Iterator it = _cursors.iterator(); it.hasNext(); ) {
+ WeakReference ref = (WeakReference) it.next();
+ if (ref.get() == null) {
+ it.remove();
+ }
+ }
+
+ _cursors.add( new WeakReference(cur) );
}
/**
@@ -847,7 +854,21 @@
* the set of cursors to be notified of changes to this list.
*/
protected void unregisterCursor(Cursor cur) {
- _cursors.remove(cur);
+ for (Iterator it = _cursors.iterator(); it.hasNext(); ) {
+ WeakReference ref = (WeakReference) it.next();
+ Cursor cursor = (Cursor) ref.get();
+ if (cursor == null) {
+ // some other unrelated cursor object has been
+ // garbage-collected; let's take the opportunity to
+ // clean up the cursors list anyway..
+ it.remove();
+
+ } else if (cursor == cur) {
+ ref.clear();
+ it.remove();
+ break;
+ }
+ }
}
/**
@@ -856,8 +877,14 @@
*/
protected void invalidateCursors() {
Iterator it = _cursors.iterator();
- while(it.hasNext()) {
- ((Cursor)it.next()).invalidate();
+ while (it.hasNext()) {
+ WeakReference ref = (WeakReference) it.next();
+ Cursor cursor = (Cursor) ref.get();
+ if (cursor != null) {
+ // cursor is null if object has been garbage-collected
+ cursor.invalidate();
+ ref.clear();
+ }
it.remove();
}
}
@@ -869,8 +896,14 @@
*/
protected void broadcastListableChanged(Listable elt) {
Iterator it = _cursors.iterator();
- while(it.hasNext()) {
- ((Cursor)it.next()).listableChanged(elt);
+ while (it.hasNext()) {
+ WeakReference ref = (WeakReference) it.next();
+ Cursor cursor = (Cursor) ref.get();
+ if (cursor == null) {
+ it.remove(); // clean up list
+ } else {
+ cursor.listableChanged(elt);
+ }
}
}
@@ -880,8 +913,14 @@
*/
protected void broadcastListableRemoved(Listable elt) {
Iterator it = _cursors.iterator();
- while(it.hasNext()) {
- ((Cursor)it.next()).listableRemoved(elt);
+ while (it.hasNext()) {
+ WeakReference ref = (WeakReference) it.next();
+ Cursor cursor = (Cursor) ref.get();
+ if (cursor == null) {
+ it.remove(); // clean up list
+ } else {
+ cursor.listableRemoved(elt);
+ }
}
}
@@ -891,8 +930,14 @@
*/
protected void broadcastListableInserted(Listable elt) {
Iterator it = _cursors.iterator();
- while(it.hasNext()) {
- ((Cursor)it.next()).listableInserted(elt);
+ while (it.hasNext()) {
+ WeakReference ref = (WeakReference) it.next();
+ Cursor cursor = (Cursor) ref.get();
+ if (cursor == null) {
+ it.remove(); // clean up list
+ } else {
+ cursor.listableInserted(elt);
+ }
}
}
@@ -1171,6 +1216,14 @@
_valid = false;
}
+ /**
+ * Mark this cursor as no longer being needed. Any resources
+ * associated with this cursor are immediately released.
+ * In previous versions of this class, it was mandatory to close
+ * all cursor objects to avoid memory leaks. It is <i>no longer</i>
+ * necessary to call this close method; an instance of this class
+ * can now be treated exactly like a normal iterator.
+ */
public void close() {
if(_valid) {
_valid = false;
1.11 +80 -4
jakarta-commons/collections/src/test/org/apache/commons/collections/TestCursorableLinkedList.java
Index: TestCursorableLinkedList.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/collections/src/test/org/apache/commons/collections/TestCursorableLinkedList.java,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -r1.10 -r1.11
--- TestCursorableLinkedList.java 20 Sep 2003 14:03:57 -0000 1.10
+++ TestCursorableLinkedList.java 27 Sep 2003 10:07:14 -0000 1.11
@@ -70,7 +70,10 @@
import junit.framework.Test;
/**
+ * Test class.
+ *
* @author Rodney Waldhoff
+ * @author Simon Kitching
* @version $Id$
*/
public class TestCursorableLinkedList extends TestList {
@@ -334,6 +337,79 @@
it.close();
}
+ public void testCursorConcurrentModification() {
+ // this test verifies that cursors remain valid when the list
+ // is modified via other means.
+ list.add("1");
+ list.add("2");
+ list.add("3");
+ list.add("5");
+ list.add("7");
+ list.add("9");
+
+ CursorableLinkedList.Cursor c1 = list.cursor();
+ CursorableLinkedList.Cursor c2 = list.cursor();
+ ListIterator li = list.listIterator();
+
+ // test cursors remain valid when list modified by std ListIterator
+ // test cursors skip elements removed via ListIterator
+ assertEquals("1",li.next());
+ assertEquals("2",li.next());
+ li.remove();
+ assertEquals("3",li.next());
+ assertEquals("1",c1.next());
+ assertEquals("3",c1.next());
+ assertEquals("1",c2.next());
+
+ // test cursor c1 can remove elements from previously modified list
+ // test cursor c2 skips elements removed via different cursor
+ c1.remove();
+ assertEquals("5",c2.next());
+ c2.add("6");
+ assertEquals("5",c1.next());
+ assertEquals("6",c1.next());
+ assertEquals("7",c1.next());
+
+ // test cursors remain valid when list mod via CursorableLinkedList
+ // test cursor remains valid when elements inserted into list before
+ // the current position of the cursor.
+ list.add(0, "0");
+
+ // test cursor remains valid when element inserted immediately after
+ // current element of a cursor, and the element is seen on the
+ // next call to the next method of that cursor.
+ list.add(5, "8");
+
+ assertEquals("8",c1.next());
+ assertEquals("9",c1.next());
+ c1.add("10");
+ assertEquals("7",c2.next());
+ assertEquals("8",c2.next());
+ assertEquals("9",c2.next());
+ assertEquals("10",c2.next());
+
+ boolean nosuch = false;
+ try {
+ c2.next();
+ }
+ catch (java.util.NoSuchElementException nse) {
+ nosuch = true; // expected
+ }
+ assertTrue(nosuch);
+
+ boolean listIteratorInvalid = false;
+ try {
+ li.next();
+ }
+ catch(java.util.ConcurrentModificationException cme) {
+ listIteratorInvalid = true; // expected
+ }
+ assertTrue(listIteratorInvalid);
+
+ c1.close(); // not necessary
+ c2.close(); // not necessary
+ }
+
public void testEqualsAndHashCode() {
assertTrue(list.equals(list));
assertEquals(list.hashCode(),list.hashCode());
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
