Permalink
Browse files

SI-6690 Release reference to last dequeued element.

Avoids leaks in MutableList and its more better known child,
mutable.Queue, when the last element is dequeued or when we
take the tail of a one element collection.

Refactors copy/pasted code between the two implementations of
tail.
  • Loading branch information...
retronym committed Dec 8, 2012
1 parent fd57069 commit d526f8bd74a3a6b878dda77bf19beb60dbc28f81
@@ -56,12 +56,16 @@ extends AbstractSeq[A]
/** Returns the rest of this list
*/
override def tail: MutableList[A] = {
- require(nonEmpty, "tail of empty list")
val tl = new MutableList[A]
+ tailImpl(tl)
+ tl
+ }
+
+ protected final def tailImpl(tl: MutableList[A]) {
+ require(nonEmpty, "tail of empty list")
tl.first0 = first0.tail
- tl.last0 = last0
tl.len = len - 1
- tl
+ tl.last0 = if (tl.len == 0) new LinkedList[A]() else last0
}
/** Prepends a single element to this list. This operation takes constant
@@ -66,7 +66,7 @@ extends MutableList[A]
else {
val res = first0.elem
first0 = first0.next
- len -= 1
+ decrementLength()
res
}
@@ -82,11 +82,11 @@ extends MutableList[A]
else if (p(first0.elem)) {
val res: Option[A] = Some(first0.elem)
first0 = first0.next
- len -= 1
+ decrementLength()
res
} else {
val optElem = removeFromList(p)
- if (optElem != None) len -= 1
+ if (optElem != None) decrementLength()
optElem
}
@@ -119,7 +119,7 @@ extends MutableList[A]
while ((first0.nonEmpty) && p(first0.elem)) {
res += first0.elem
first0 = first0.next
- len -= 1
+ decrementLength()
}
if (first0.isEmpty) res
else removeAllFromList(p, res)
@@ -130,10 +130,10 @@ extends MutableList[A]
var leftlst = first0
while (leftlst.next.nonEmpty) {
if (p(leftlst.next.elem)) {
- res += leftlst.next.elem
- if (leftlst.next eq last0) last0 = leftlst
- leftlst.next = leftlst.next.next
- len -= 1
+ res += leftlst.next.elem
+ if (leftlst.next eq last0) last0 = leftlst
+ leftlst.next = leftlst.next.next
+ decrementLength()
} else leftlst = leftlst.next
}
res
@@ -154,7 +154,7 @@ extends MutableList[A]
else {
val res: Option[LinkedList[A]] = Some(cell.next)
cell.next = cell.next.next
- len -= 1
+ decrementLength()
res
}
}
@@ -170,11 +170,8 @@ extends MutableList[A]
// TODO - Don't override this just for new to create appropriate type....
override def tail: Queue[A] = {
- require(nonEmpty, "tail of empty list")
val tl = new Queue[A]
- tl.first0 = first0.tail
- tl.last0 = last0
- tl.len = len - 1
+ tailImpl(tl)
tl
}
@@ -183,6 +180,11 @@ extends MutableList[A]
bf ++= seq
bf.result
}
+
+ private[this] def decrementLength() {
+ len -= 1
+ if (len == 0) last0 = new LinkedList[A]()
+ }
}
View
@@ -0,0 +1,62 @@
+import scala.collection.mutable
+
+object Test extends App {
+ def last0(ml: mutable.MutableList[Int]) =
+ ml.asInstanceOf[{def last0: mutable.LinkedList[Int]}].last0
+
+ def first0(ml: mutable.MutableList[Int]) =
+ ml.asInstanceOf[{def first0: mutable.LinkedList[Int]}].first0
+
+ val f = mutable.Queue[Int]()
+ def check(desc: String) {
+ assert(f.length == 0, s"$desc: non empty: $f")
+ assert(last0(f).isEmpty, s"$desc: last0 leak: ${last0(f)}")
+ assert(first0(f).isEmpty, s"$desc: first0 leak: ${last0(f)}")
+ }
+
+ f.enqueue(1)
+ f.dequeue()
+ check("dequeue 1")
+
+ f.enqueue(1)
+ f.enqueue(2)
+ f.dequeue()
+ assert(last0(f).toList == List(2), last0(f))
+ f.dequeue()
+ check("dequeue 2")
+
+ f.enqueue(1)
+ f.dequeueAll(_ => false)
+ f.dequeueAll(_ => true)
+ check("dequeueAll")
+
+ f.enqueue(1)
+ f.dequeueFirst(_ => true)
+ check("dequeueFirst")
+
+ {
+ f.enqueue(1)
+ val tail = f.tail
+ assert(last0(tail).isEmpty, last0(tail))
+ assert(first0(tail).isEmpty, first0(tail))
+ }
+
+ {
+ val ml = mutable.MutableList[Int]()
+ 1 +=: ml
+ val tail = ml.tail
+ assert(last0(tail).isEmpty, last0(tail))
+ assert(first0(tail).isEmpty, first0(tail))
+ }
+
+ {
+ val ml = mutable.MutableList[Int]()
+ 1 +=: ml
+ ml += 2
+ val tail = ml.tail
+ assert(last0(tail).toList == List(2), last0(tail))
+ assert(first0(tail) == last0(tail).toList, first0(tail))
+ assert(last0(tail.tail).toList == Nil, last0(tail.tail).toList)
+ assert(first0(tail.tail) == Nil, first0(tail.tail))
+ }
+}

0 comments on commit d526f8b

Please sign in to comment.