Skip to content
This repository was archived by the owner on Sep 8, 2022. It is now read-only.

Add synchronization when sending events to sbt (fix ArrayIOBException). #78

Merged
merged 1 commit into from
Apr 10, 2017

Conversation

gourlaysama
Copy link
Contributor

Partest shows a single Task to sbt and does all the concurrency and
running subtasks itself, which isn't really how this should work, so sbt
doesn't expect the task to send back events concurrently:
EventHandler.handle adds events to an ArrayList when forking (see
ForkMain), and is called after each test, and if it happens on two
threads at exactly the wrong moment:

Caused by: sbt.ForkMain$ForkError: java.lang.ArrayIndexOutOfBoundsException: 15
	at java.util.ArrayList.add(ArrayList.java:459)
	at sbt.ForkMain$Run$2$1.handle(ForkMain.java:294)
	at scala.tools.partest.sbt.SBTRunner$$anon$1.onFinishTest(SBTRunner.scala:70)
	at scala.tools.partest.nest.SuiteRunner.runTest(Runner.scala:781)
	at scala.tools.partest.nest.SuiteRunner.$anonfun$runTestsForFiles$2(Runner.scala:788)
	at scala.tools.partest.package$$anon$2.call(package.scala:135)

See scala/scala#5663 (build log or the failed run).

Partest shows a single `Task` to sbt and does all the concurrency and
running subtasks itself, which isn't really how this should work, so sbt
doesn't expect the task to send back events concurrently:
`EventHandler.handle` adds events to an `ArrayList` when forking (see
[ForkMain][2]), and is called after each test, and if it happens on two
threads at exactly the wrong moment:

```
Caused by: sbt.ForkMain$ForkError: java.lang.ArrayIndexOutOfBoundsException: 15
	at java.util.ArrayList.add(ArrayList.java:459)
	at sbt.ForkMain$Run$2$1.handle(ForkMain.java:294)
	at scala.tools.partest.sbt.SBTRunner$$anon$1.onFinishTest(SBTRunner.scala:70)
	at scala.tools.partest.nest.SuiteRunner.runTest(Runner.scala:781)
	at scala.tools.partest.nest.SuiteRunner.$anonfun$runTestsForFiles$2(Runner.scala:788)
	at scala.tools.partest.package$$anon$2.call(package.scala:135)
```

See scala/scala#5663 ([build log][1] or the failed run).

[1]: https://scala-ci.typesafe.com/job/scala-2.12.x-validate-test/4300/consoleFull
[2]: https://github.com/sbt/sbt/blob/v0.13.13/testing/agent/src/main/java/sbt/ForkMain.java#L294
@gourlaysama
Copy link
Contributor Author

This could also be fixed on sbt's side, but it probably wouldn't benefit anyone else: a reasonable test framework's Task implementation either runs itself or returns subtasks that do, but it doesn't create a threadpool to run its own secret tasks that are not returned to sbt but that do return events to sbt.

I'm running locally the tests on the scala repo with this change, to see if it slows down anything...

@gourlaysama
Copy link
Contributor Author

And this was my "let's chase this interesting error" day 😄

@dwijnand
Copy link
Member

dwijnand commented Mar 6, 2017

Very interesting. Thank you for looking into this and sharing your analysis.

@adriaanm
Copy link
Contributor

Thanks! Sorry for the delay in merging this 😞

@adriaanm adriaanm merged commit 4344c94 into scala:1.1.x Apr 10, 2017
@gourlaysama gourlaysama deleted the array-iob-exception branch April 15, 2017 14:13
lrytz pushed a commit to lrytz/scala-partest that referenced this pull request May 9, 2018
Add synchronization when sending events to sbt (fix ArrayIOBException).
lrytz pushed a commit to lrytz/scala-partest that referenced this pull request May 9, 2018
Add synchronization when sending events to sbt (fix ArrayIOBException).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants