We have a possible memory leak in our GapContent implementation. The
problem is that the GapContent stores all the Position objects it
creates in an ArrayList, but since there is no way (except
WeakReferences) to detect when they are no longer used, they are never
removed from that array. I fixed this by storing the Position objects in
WeakReferences and adjusting the class to handle this. This involved
implementing a special Comparator that compares weakly referenced
objects and a clearing mechanism that clears up the ArrayList before new
Positions are created.
2006-02-21 Roman Kennke <[EMAIL PROTECTED]>
PR classpath/26368
* javax/swing/text/GapContent.java
(GapContentPosition): Made class private.
(InsertUndo): Made class private.
(UndoRemove): Made class private.
(WeakPositionComparator): New inner class.
(positions): Made field private.
(createPosition): Clear up GC'ed positions before creating
a new one. Store position as WeakReference.
(getPositionsInRange): Changed to handle WeakReference
positions.
(setPositionsInRange): Changed to handle WeakReference
positions.
(adjustPositionsInRange): Changed to handle WeakReference
positions.
(dumpPositions): Handle WeakReference positions.
(clearPositionReferences): New method.
/Roman
Index: javax/swing/text/GapContent.java
===================================================================
RCS file: /cvsroot/classpath/classpath/javax/swing/text/GapContent.java,v
retrieving revision 1.39
diff -u -r1.39 GapContent.java
--- javax/swing/text/GapContent.java 20 Feb 2006 12:07:31 -0000 1.39
+++ javax/swing/text/GapContent.java 21 Feb 2006 11:28:49 -0000
@@ -1,5 +1,5 @@
/* GapContent.java --
- Copyright (C) 2002, 2004, 2005 Free Software Foundation, Inc.
+ Copyright (C) 2002, 2004, 2005, 2006 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -39,8 +39,10 @@
package javax.swing.text;
import java.io.Serializable;
+import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.Comparator;
import java.util.Iterator;
import java.util.ListIterator;
import java.util.Vector;
@@ -68,8 +70,8 @@
/**
* A [EMAIL PROTECTED] Position} implementation for <code>GapContent</code>.
*/
- class GapContentPosition
- implements Position, Comparable
+ private class GapContentPosition
+ implements Position, Comparable
{
/** The index within the buffer array. */
@@ -130,7 +132,7 @@
}
}
- class InsertUndo extends AbstractUndoableEdit
+ private class InsertUndo extends AbstractUndoableEdit
{
public int where, length;
String text;
@@ -169,7 +171,7 @@
}
- class UndoRemove extends AbstractUndoableEdit
+ private class UndoRemove extends AbstractUndoableEdit
{
public int where;
String text;
@@ -206,7 +208,41 @@
}
}
-
+
+ /**
+ * Compares WeakReference objects in a List by comparing the referenced
+ * objects instead.
+ *
+ * @author Roman Kennke ([EMAIL PROTECTED])
+ */
+ private class WeakPositionComparator
+ implements Comparator
+ {
+
+ /**
+ * Compares two objects of type WeakReference. The objects are compared
+ * using the referenced objects compareTo() method.
+ */
+ public int compare(Object o1, Object o2)
+ {
+ // Unwrap references.
+ if (o1 instanceof WeakReference)
+ o1 = ((WeakReference) o1).get();
+ if (o2 instanceof WeakReference)
+ o2 = ((WeakReference) o2).get();
+
+ GapContentPosition p1 = (GapContentPosition) o1;
+ GapContentPosition p2 = (GapContentPosition) o2;
+
+ int retVal;
+ if (p1 == null || p2 == null)
+ retVal = -1;
+ else
+ retVal = p1.compareTo(p2);
+ return retVal;
+ }
+ }
+
/** The serialization UID (compatible with JDK1.5). */
private static final long serialVersionUID = -6226052713477823730L;
@@ -233,9 +269,10 @@
/**
* The positions generated by this GapContent. They are kept in an ordered
- * fashion, so they can be looked up easily.
+ * fashion, so they can be looked up easily. The value objects will be
+ * WeakReference objects that in turn hold GapContentPosition objects.
*/
- ArrayList positions;
+ private ArrayList positions;
/**
* Creates a new GapContent object.
@@ -446,18 +483,22 @@
throw new BadLocationException("The offset was out of the bounds of this"
+ " buffer", offset);
+ clearPositionReferences();
+
// We store the actual array index in the GapContentPosition. The real
// offset is then calculated in the GapContentPosition.
int mark = offset;
if (offset >= gapStart)
mark += gapEnd - gapStart;
GapContentPosition pos = new GapContentPosition(mark);
+ WeakReference r = new WeakReference(pos);
// Add this into our list in a sorted fashion.
- int index = Collections.binarySearch(positions, pos);
+ int index = Collections.binarySearch(positions, r,
+ new WeakPositionComparator());
if (index < 0)
index = -(index + 1);
- positions.add(index, pos);
+ positions.add(index, r);
return pos;
}
@@ -642,19 +683,30 @@
int endOffset = offset + length;
int index1 = Collections.binarySearch(positions,
- new GapContentPosition(offset));
+ new GapContentPosition(offset),
+ new WeakPositionComparator());
if (index1 < 0)
index1 = -(index1 + 1);
// Search the first index with the specified offset. The binarySearch does
// not necessarily find the first one.
- while (index1 > 0
- && ((GapContentPosition) positions.get(index1 - 1)).mark == offset)
- index1--;
+ while (index1 > 0)
+ {
+ WeakReference r = (WeakReference) positions.get(index1 - 1);
+ GapContentPosition p = (GapContentPosition) r.get();
+ if (p != null && p.mark == offset || p == null)
+ index1--;
+ else
+ break;
+ }
for (ListIterator i = positions.listIterator(index1); i.hasNext();)
{
- GapContentPosition p = (GapContentPosition) i.next();
+ WeakReference r = (WeakReference) i.next();
+ GapContentPosition p = (GapContentPosition) r.get();
+ if (p == null)
+ continue;
+
if (p.mark > endOffset)
break;
if (p.mark >= offset && p.mark <= endOffset)
@@ -672,24 +724,35 @@
* @param length the length of the range to search
* @param value the new value for each mark
*/
- void setPositionsInRange(int offset, int length, int value)
+ private void setPositionsInRange(int offset, int length, int value)
{
int endOffset = offset + length;
int index1 = Collections.binarySearch(positions,
- new GapContentPosition(offset));
+ new GapContentPosition(offset),
+ new WeakPositionComparator());
if (index1 < 0)
index1 = -(index1 + 1);
// Search the first index with the specified offset. The binarySearch does
// not necessarily find the first one.
- while (index1 > 0
- && ((GapContentPosition) positions.get(index1 - 1)).mark == offset)
- index1--;
+ while (index1 > 0)
+ {
+ WeakReference r = (WeakReference) positions.get(index1 - 1);
+ GapContentPosition p = (GapContentPosition) r.get();
+ if (p != null && p.mark == offset || p == null)
+ index1--;
+ else
+ break;
+ }
for (ListIterator i = positions.listIterator(index1); i.hasNext();)
{
- GapContentPosition p = (GapContentPosition) i.next();
+ WeakReference r = (WeakReference) i.next();
+ GapContentPosition p = (GapContentPosition) r.get();
+ if (p == null)
+ continue;
+
if (p.mark > endOffset)
break;
@@ -707,23 +770,35 @@
* @param length the length of the range to search
* @param incr the increment
*/
- void adjustPositionsInRange(int offset, int length, int incr)
+ private void adjustPositionsInRange(int offset, int length, int incr)
{
int endOffset = offset + length;
int index1 = Collections.binarySearch(positions,
- new GapContentPosition(offset));
+ new GapContentPosition(offset),
+ new WeakPositionComparator());
if (index1 < 0)
index1 = -(index1 + 1);
// Search the first index with the specified offset. The binarySearch does
// not necessarily find the first one.
- while (index1 > 0
- && ((GapContentPosition) positions.get(index1 - 1)).mark == offset)
- index1--;
+ while (index1 > 0)
+ {
+ WeakReference r = (WeakReference) positions.get(index1 - 1);
+ GapContentPosition p = (GapContentPosition) r.get();
+ if (p != null && p.mark == offset || p == null)
+ index1--;
+ else
+ break;
+ }
+
for (ListIterator i = positions.listIterator(index1); i.hasNext();)
{
- GapContentPosition p = (GapContentPosition) i.next();
+ WeakReference r = (WeakReference) i.next();
+ GapContentPosition p = (GapContentPosition) r.get();
+ if (p == null)
+ continue;
+
if (p.mark > endOffset)
break;
@@ -787,8 +862,23 @@
{
for (Iterator i = positions.iterator(); i.hasNext();)
{
- GapContentPosition pos = (GapContentPosition) i.next();
+ WeakReference r = (WeakReference) i.next();
+ GapContentPosition pos = (GapContentPosition) r.get();
System.err.println("position at: " + pos.mark);
}
}
+
+ /**
+ * Clears all GC'ed references in the positions array.
+ */
+ private void clearPositionReferences()
+ {
+ Iterator i = positions.iterator();
+ while (i.hasNext())
+ {
+ WeakReference r = (WeakReference) i.next();
+ if (r.get() == null)
+ i.remove();
+ }
+ }
}
