typelevel / scalacheck

Property-based testing for Scala
http://www.scalacheck.org
BSD 3-Clause "New" or "Revised" License
1.94k stars 404 forks source link

Stack overflow caused by retryUntil #232

Open dabd opened 8 years ago

dabd commented 8 years ago

The following generator causes a stack overflow error:

`import org.scalacheck.Arbitrary.arbBigInt

arbBigInt.arbitrary.retryUntil(x => x >= BigInt(200) && x <= BigInt(300)).sample`

will cause java.lang.StackOverflowError at scala.collection.immutable.RedBlackTree$.doFrom(scratch_34:250) at scala.collection.immutable.RedBlackTree$.doFrom(scratch_34:249) at scala.collection.immutable.RedBlackTree$.doFrom(scratch_34:249) at scala.collection.immutable.RedBlackTree$.from(scratch_34:71) at scala.collection.immutable.TreeMap.from(scratch_34:59) at org.scalacheck.Gen$$anonfun$frequency$1.apply(scratch_34:390) at org.scalacheck.Gen$$anonfun$frequency$1.apply(scratch_34:390) at org.scalacheck.Gen$$anonfun$flatMap$1$$anonfun$apply$1.apply(scratch_34:49) at org.scalacheck.Gen$$anonfun$flatMap$1$$anonfun$apply$1.apply(scratch_34:49) at org.scalacheck.Gen$R$class.flatMap(scratch_34:171) at org.scalacheck.Gen$R$$anon$4.flatMap(scratch_34:157) at org.scalacheck.Gen$$anonfun$flatMap$1.apply(scratch_34:49) at org.scalacheck.Gen$$anonfun$flatMap$1.apply(scratch_34:48) at org.scalacheck.Gen$$anon$6.doApply(scratch_34:182) at org.scalacheck.Gen$$anon$2.doApply(scratch_34:74) at org.scalacheck.Gen$$anonfun$flatMap$1.apply(scratch_34:49) at org.scalacheck.Gen$$anonfun$flatMap$1.apply(scratch_34:48) at org.scalacheck.Gen$$anon$6.doApply(scratch_34:182) at org.scalacheck.Gen$$anonfun$flatMap$1$$anonfun$apply$1.apply(scratch_34:49) at org.scalacheck.Gen$$anonfun$flatMap$1$$anonfun$apply$1.apply(scratch_34:49) at org.scalacheck.Gen$R$class.flatMap(scratch_34:171) at org.scalacheck.Gen$R$$anon$4.flatMap(scratch_34:157) at org.scalacheck.Gen$$anonfun$flatMap$1.apply(scratch_34:49) at org.scalacheck.Gen$$anonfun$flatMap$1.apply(scratch_34:48) at org.scalacheck.Gen$$anon$6.doApply(scratch_34:182) at org.scalacheck.Gen$$anonfun$flatMap$1$$anonfun$apply$1.apply(scratch_34:49) at org.scalacheck.Gen$$anonfun$flatMap$1$$anonfun$apply$1.apply(scratch_34:49) at org.scalacheck.Gen$R$class.flatMap(scratch_34:171) at org.scalacheck.Gen$R$$anon$4.flatMap(scratch_34:157) at org.scalacheck.Gen$$anonfun$flatMap$1.apply(scratch_34:49) at org.scalacheck.Gen$$anonfun$flatMap$1.apply(scratch_34:48) at org.scalacheck.Gen$$anon$6.doApply(scratch_34:182) at org.scalacheck.Gen$$anonfun$flatMap$1$$anonfun$apply$1.apply(scratch_34:49) at org.scalacheck.Gen$$anonfun$flatMap$1$$anonfun$apply$1.apply(scratch_34:49) Output exceeds cutoff limit.

It seems to be very dependent on lower and upper limit parameters and the usage of retryUntil. For example with lower = 100 and upper = 200 it doesn't crash, nor if using suchThat.

non commented 7 years ago

So this is pretty easy to fix. The real issue here is that the Gen[BigInt] in question is very unlikely to generate values in the 200-300 range. (For example, when I tested a "fix" to this bug it ran for over 33 million iterations without generating a value in this range.)

I agree that a StackOverflowException is pretty unfriendly. However, I don't think users will be happier to have code that just hangs forever. In general it's safer to stay away from this kind of filtering logic.

One possible fix here would be to remove the stack unsafety, but put in a manual cutoff at 100K iterations (or some other limit we can agree on), and throw a more descriptive error. What do you think @rickynils?