Skip to content

Commit c6ec67a

Browse files
committed
Fixed NPE in VectorStepper due to dirty display vector.
(Could occur with structurally shared vectors.)
1 parent 675831f commit c6ec67a

File tree

4 files changed

+90
-6
lines changed

4 files changed

+90
-6
lines changed

src/main/java/scala/compat/java8/runtime/CollectionInternals.java

+7
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,19 @@
55
public class CollectionInternals {
66
public static <A> Object[] getTable(scala.collection.mutable.FlatHashTable<A> fht) { return fht.hashTableContents().table(); }
77
public static <A, E extends scala.collection.mutable.HashEntry<A,E>> scala.collection.mutable.HashEntry<A, E>[] getTable(scala.collection.mutable.HashTable<A,E> ht) { return ht.hashTableContents().table(); }
8+
public static <A> boolean getDirt(scala.collection.immutable.Vector<A> v) { return v.dirty(); }
89
public static <A> Object[] getDisplay0(scala.collection.immutable.Vector<A> v) { return v.display0(); }
10+
public static <A> Object[] getDisplay0(scala.collection.immutable.VectorIterator<A> p) { return p.display0(); }
911
public static <A> Object[] getDisplay1(scala.collection.immutable.Vector<A> v) { return v.display1(); }
12+
public static <A> Object[] getDisplay1(scala.collection.immutable.VectorIterator<A> p) { return p.display1(); }
1013
public static <A> Object[] getDisplay2(scala.collection.immutable.Vector<A> v) { return v.display2(); }
14+
public static <A> Object[] getDisplay2(scala.collection.immutable.VectorIterator<A> p) { return p.display2(); }
1115
public static <A> Object[] getDisplay3(scala.collection.immutable.Vector<A> v) { return v.display3(); }
16+
public static <A> Object[] getDisplay3(scala.collection.immutable.VectorIterator<A> p) { return p.display3(); }
1217
public static <A> Object[] getDisplay4(scala.collection.immutable.Vector<A> v) { return v.display4(); }
18+
public static <A> Object[] getDisplay4(scala.collection.immutable.VectorIterator<A> p) { return p.display4(); }
1319
public static <A> Object[] getDisplay5(scala.collection.immutable.Vector<A> v) { return v.display5(); }
20+
public static <A> Object[] getDisplay5(scala.collection.immutable.VectorIterator<A> p) { return p.display5(); }
1421
public static <A> scala.Tuple2< scala.Tuple2< scala.collection.Iterator<A>, Object >, scala.collection.Iterator<A> > trieIteratorSplit(scala.collection.Iterator<A> it) {
1522
if (it instanceof scala.collection.immutable.TrieIterator) {
1623
scala.collection.immutable.TrieIterator<A> trie = (scala.collection.immutable.TrieIterator<A>)it;

src/main/scala/scala/compat/java8/converterImpl/StepsLikeIndexed.scala

+3
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,19 @@ private[java8] abstract class StepsLikeIndexed[A, STA >: Null <: StepsLikeIndexe
3232
private[java8] abstract class StepsDoubleLikeIndexed[STD >: Null <: StepsDoubleLikeIndexed[_]](_i0: Int, _iN: Int)
3333
extends AbstractStepsLikeIndexed[DoubleStepper, STD](_i0, _iN)
3434
with DoubleStepper
35+
with java.util.Spliterator.OfDouble // Compiler wants this for mixin forwarder
3536
{}
3637

3738
/** Abstracts the operation of stepping over an indexable collection of Ints */
3839
private[java8] abstract class StepsIntLikeIndexed[STI >: Null <: StepsIntLikeIndexed[_]](_i0: Int, _iN: Int)
3940
extends AbstractStepsLikeIndexed[IntStepper, STI](_i0, _iN)
4041
with IntStepper
42+
with java.util.Spliterator.OfInt // Compiler wants this for mixin forwarder
4143
{}
4244

4345
/** Abstracts the operation of stepping over an indexable collection of Longs */
4446
private[java8] abstract class StepsLongLikeIndexed[STL >: Null <: StepsLongLikeIndexed[_]](_i0: Int, _iN: Int)
4547
extends AbstractStepsLikeIndexed[LongStepper, STL](_i0, _iN)
4648
with LongStepper
49+
with java.util.Spliterator.OfLong // Compiler wants this for mixin forwarder
4750
{}

src/main/scala/scala/compat/java8/converterImpl/StepsVector.scala

+52-6
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package scala.compat.java8.converterImpl
22

33
import scala.annotation.switch
44

5+
import scala.collection.immutable.VectorIterator
6+
57
import scala.compat.java8.collectionImpl._
68
import scala.compat.java8.runtime._
79

@@ -13,20 +15,27 @@ import Stepper._
1315

1416
private[java8] trait StepsVectorLike[A] {
1517
protected def myVector: Vector[A]
18+
protected def myVectorIterator: VectorIterator[A]
19+
protected def myVectorLength: Int
1620
protected var index: Int = 32
1721
protected var data: Array[AnyRef] = null
1822
protected var index1: Int = 32
1923
protected var data1: Array[AnyRef] = null
2024
protected final def advanceData(iX: Int): Unit = {
2125
index1 += 1
22-
if (index >= 32) initTo(iX)
26+
if (index >= 32) {
27+
if (myVector != null) initTo(iX)
28+
else initVpTo(iX)
29+
}
2330
else {
2431
data = data1(index1).asInstanceOf[Array[AnyRef]]
2532
index = 0
2633
}
2734
}
2835
protected final def initTo(iX: Int): Unit = {
29-
myVector.length match {
36+
// WARNING--initVpTo is an exact copy of this except for the type! If you change one you must change the other!
37+
// (Manually specialized this way for speed.)
38+
myVectorLength match {
3039
case x if x <= 0x20 =>
3140
index = iX
3241
data = CollectionInternals.getDisplay0(myVector)
@@ -52,12 +61,43 @@ private[java8] trait StepsVectorLike[A] {
5261
data = data1(index1).asInstanceOf[Array[AnyRef]]
5362
}
5463
}
64+
protected final def initVpTo(iX: Int): Unit = {
65+
// WARNING--this is an exact copy of initTo! If you change one you must change the other!
66+
// (Manually specialized this way for speed.)
67+
myVectorLength match {
68+
case x if x <= 0x20 =>
69+
index = iX
70+
data = CollectionInternals.getDisplay0(myVectorIterator)
71+
case x if x <= 0x400 =>
72+
index1 = iX >>> 5
73+
data1 = CollectionInternals.getDisplay1(myVectorIterator)
74+
index = iX & 0x1F
75+
data = data1(index1).asInstanceOf[Array[AnyRef]]
76+
case x =>
77+
var N = 0
78+
var dataN: Array[AnyRef] =
79+
if (x <= 0x8000) { N = 2; CollectionInternals.getDisplay2(myVectorIterator) }
80+
else if (x <= 0x100000) { N = 3; CollectionInternals.getDisplay3(myVectorIterator) }
81+
else if (x <= 0x2000000) { N = 4; CollectionInternals.getDisplay4(myVectorIterator) }
82+
else /*x <= 0x40000000*/{ N = 5; CollectionInternals.getDisplay5(myVectorIterator) }
83+
while (N > 2) {
84+
dataN = dataN((iX >>> (5*N))&0x1F).asInstanceOf[Array[AnyRef]]
85+
N -= 1
86+
}
87+
index1 = (iX >>> 5) & 0x1F
88+
data1 = dataN((iX >>> 10) & 0x1F).asInstanceOf[Array[AnyRef]]
89+
index = iX & 0x1F
90+
data = data1(index1).asInstanceOf[Array[AnyRef]]
91+
}
92+
}
5593
}
5694

5795
private[java8] class StepsAnyVector[A](underlying: Vector[A], _i0: Int, _iN: Int)
5896
extends StepsLikeIndexed[A, StepsAnyVector[A]](_i0, _iN)
5997
with StepsVectorLike[A] {
60-
protected def myVector = underlying
98+
protected val myVector = if (CollectionInternals.getDirt(underlying)) null else underlying
99+
protected val myVectorIterator = if (myVector == null) underlying.iterator else null
100+
protected val myVectorLength = underlying.length
61101
def next() = if (hasNext()) {
62102
index += 1
63103
if (index >= 32) advanceData(i0)
@@ -76,7 +116,9 @@ with StepsVectorLike[A] {
76116
private[java8] class StepsDoubleVector(underlying: Vector[Double], _i0: Int, _iN: Int)
77117
extends StepsDoubleLikeIndexed[StepsDoubleVector](_i0, _iN)
78118
with StepsVectorLike[Double] {
79-
protected def myVector = underlying
119+
protected val myVector = if (CollectionInternals.getDirt(underlying)) null else underlying
120+
protected val myVectorIterator = if (myVector == null) underlying.iterator else null
121+
protected val myVectorLength = underlying.length
80122
def nextDouble() = if (hasNext()) {
81123
index += 1
82124
if (index >= 32) advanceData(i0)
@@ -95,7 +137,9 @@ with StepsVectorLike[Double] {
95137
private[java8] class StepsIntVector(underlying: Vector[Int], _i0: Int, _iN: Int)
96138
extends StepsIntLikeIndexed[StepsIntVector](_i0, _iN)
97139
with StepsVectorLike[Int] {
98-
protected def myVector = underlying
140+
protected val myVector = if (CollectionInternals.getDirt(underlying)) null else underlying
141+
protected val myVectorIterator = if (myVector == null) underlying.iterator else null
142+
protected val myVectorLength = underlying.length
99143
def nextInt() = if (hasNext()) {
100144
index += 1
101145
if (index >= 32) advanceData(i0)
@@ -114,7 +158,9 @@ with StepsVectorLike[Int] {
114158
private[java8] class StepsLongVector(underlying: Vector[Long], _i0: Int, _iN: Int)
115159
extends StepsLongLikeIndexed[StepsLongVector](_i0, _iN)
116160
with StepsVectorLike[Long] {
117-
protected def myVector = underlying
161+
protected val myVector = if (CollectionInternals.getDirt(underlying)) null else underlying
162+
protected val myVectorIterator = if (myVector == null) underlying.iterator else null
163+
protected val myVectorLength = underlying.length
118164
def nextLong() = if (hasNext()) {
119165
index += 1
120166
if (index >= 32) advanceData(i0)

src/test/scala/scala/compat/java8/StreamConvertersTest.scala

+28
Original file line numberDiff line numberDiff line change
@@ -276,4 +276,32 @@ class StreamConvertersTest {
276276
val stepper2 = steppize2(coll2).stepper
277277
assertTrue(stepper2.getClass.getName.contains("StepsIntVector"))
278278
}
279+
280+
@Test
281+
def issue_87(): Unit = {
282+
// Vectors that are generated from other vectors tend _not_ to
283+
// have all their display vectors consistent; the cached vectors
284+
// are correct, but the higher-level vector does _not_ contain
285+
// the cached vector in the correct place (for efficiency)! This
286+
// is called being "dirty", and needs to be handled specially.
287+
val dirtyDisplayVector = Vector.fill(120)("a").slice(0, 40)
288+
val shouldNotNPE =
289+
dirtyDisplayVector.seqStream.collect(Collectors.toList())
290+
assertEq(shouldNotNPE.toArray(new Array[String](0)).toVector, dirtyDisplayVector, "Vector[Any].seqStream (with dirty display)")
291+
292+
val dirtyDisplayVectorInt = Vector.fill(120)(999).slice(0, 40)
293+
val shouldNotNPEInt =
294+
dirtyDisplayVectorInt.seqStream.sum()
295+
assertEq(shouldNotNPEInt, dirtyDisplayVectorInt.sum, "Vector[Int].seqStream (with dirty display)")
296+
297+
val dirtyDisplayVectorLong = Vector.fill(120)(99999999999L).slice(0, 40)
298+
val shouldNotNPELong =
299+
dirtyDisplayVectorLong.seqStream.sum()
300+
assertEq(shouldNotNPELong, dirtyDisplayVectorLong.sum, "Vector[Long].seqStream (with dirty display)")
301+
302+
val dirtyDisplayVectorDouble = Vector.fill(120)(0.1).slice(0, 40)
303+
val shouldNotNPEDouble =
304+
math.rint(dirtyDisplayVectorDouble.seqStream.sum() * 10)
305+
assertEq(shouldNotNPEDouble, math.rint(dirtyDisplayVectorDouble.sum * 10), "Vector[Double].seqStream (with dirty display)")
306+
}
279307
}

0 commit comments

Comments
 (0)