Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception in JUnit test after disabling JVM forking #112

Merged
merged 1 commit into from
Nov 3, 2016

Conversation

ashawley
Copy link
Member

@ashawley ashawley commented Oct 4, 2016

Disabling JVM forking while working on #110 causes a JUnit test to fail.

$ sbt
> testOnly scala.xml.pull.XMLEventReaderTest 

This unit test hasn't been an issue in the past, but was found while trying to troubleshoot a different Travis build error after adding a Scalacheck test suite to scala-xml. It's not clear if this is the actual issue that adding Scalacheck introduce. Very likely, it could just be a false lead after desperately trying to understand the original problem.

@@ -250,7 +250,6 @@ private[scala] trait MarkupParserCommon extends TokenTests {
else if (ch == SU || eof)
truncatedError("") // throws TruncatedXMLControl in compiler

sb append ch
Copy link
Member Author

@ashawley ashawley Oct 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the line in scala-xml that in turn causes the OOM exception to be thrown by java.util.Arrays by way of java.lang.StringBuilder. Even after removing the line in question from scala-xml, see above, it still causes the test suite to run out of memory. Which scala-xml is it running that causes the memory overflow? It doesn't seem to be the code under test.

@ashawley
Copy link
Member Author

Removing the line that emits the exception, still causes the exception.

This can be verified from the Travis build:

Exception in thread "XMLEventReader" java.lang.OutOfMemoryError: Java heap space
    at java.util.Arrays.copyOf(Arrays.java:3332)
    at java.lang.AbstractStringBuilder.expandCapacity(AbstractStringBuilder.java:137)
    at java.lang.AbstractStringBuilder.ensureCapacityInternal(AbstractStringBuilder.java:121)
    at java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:622)
    at java.lang.StringBuilder.append(StringBuilder.java:202)
    at scala.collection.mutable.StringBuilder.append(StringBuilder.scala:267)
    at scala.xml.parsing.MarkupParserCommon.xTakeUntil(MarkupParserCommon.scala:253)
    at scala.xml.parsing.MarkupParserCommon.xTakeUntil$(MarkupParserCommon.scala:238)
    at scala.xml.pull.XMLEventReader$Parser.xTakeUntil(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.xCharData(MarkupParser.scala:379)
    at scala.xml.parsing.MarkupParser.xCharData$(MarkupParser.scala:373)
    at scala.xml.pull.XMLEventReader$Parser.xCharData(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.content1(MarkupParser.scala:424)
    at scala.xml.parsing.MarkupParser.content1$(MarkupParser.scala:419)
    at scala.xml.pull.XMLEventReader$Parser.content1(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.content(MarkupParser.scala:459)
    at scala.xml.parsing.MarkupParser.content$(MarkupParser.scala:442)
    at scala.xml.pull.XMLEventReader$Parser.content(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.element1(MarkupParser.scala:588)
    at scala.xml.parsing.MarkupParser.element1$(MarkupParser.scala:573)
    at scala.xml.pull.XMLEventReader$Parser.element1(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.content1(MarkupParser.scala:433)
    at scala.xml.parsing.MarkupParser.content1$(MarkupParser.scala:419)
    at scala.xml.pull.XMLEventReader$Parser.content1(XMLEventReader.scala:60)
    at scala.xml.parsing.MarkupParser.document(MarkupParser.scala:247)
    at scala.xml.parsing.MarkupParser.document$(MarkupParser.scala:225)
    at scala.xml.pull.XMLEventReader$Parser.document(XMLEventReader.scala:60)
    at scala.xml.pull.XMLEventReader$Parser.$anonfun$run$1(XMLEventReader.scala:96)
    at scala.xml.pull.XMLEventReader$Parser$$Lambda$14/510432178.apply(Unknown Source)
    at scala.xml.pull.ProducerConsumerIterator.interruptibly(XMLEventReader.scala:125)
    at scala.xml.pull.ProducerConsumerIterator.interruptibly$(XMLEventReader.scala:125)
    at scala.xml.pull.XMLEventReader.interruptibly(XMLEventReader.scala:27)

@SethTisue
Copy link
Member

this is kind of a wild guess, but XMLEventReader uses a background thread. perhaps that thread needs to be explicitly shut down?

@ashawley
Copy link
Member Author

Thanks for pointing that out. That's curious. I hadn't noticed it.

I'll try calling stop in the unit tests.

@ashawley
Copy link
Member Author

Unfortunately, no joy after adding stop to the tests at least.

@gourlaysama
Copy link
Member

You cannot disable JVM test forking when working on a scala module that is distributed with the compiler (which is the case for scala-xml and scala-parser-combinators) because of an SBT classloader leaking issue (#20).

Basically, when not forking, the test are compiled against the locally built classes, but they are run against the ones bundled with the compiler (the ones from the bootclasspath, or from the special scala-toolconfiguration that contains the compiler and its dependencies).

@ashawley
Copy link
Member Author

@gourlaysama Thanks for this. I was losing my mind understanding what was amiss.

I had seen @jsuereth branch, wip/fix-build-classpath, but didn't know what it was about until you mentioned the issue it was associated with.

@ashawley
Copy link
Member Author

I've reworked this PR to add an explicit setting of fork in the build.sbt file, and added @gourlaysama's words and mentioned both this issue and the original one.

Add a comment with a description of the issue by Antoine Gourlay and
refer to the relevant GitHub issues.
@SethTisue
Copy link
Member

LGTM

@ashawley ashawley mentioned this pull request Oct 27, 2016
@SethTisue
Copy link
Member

final review/merge by @biswanaths ?

@biswanaths biswanaths merged commit 150a237 into scala:master Nov 3, 2016
@biswanaths
Copy link
Member

@SethTisue words are final though.

Thank you @SethTisue for review and @ashawley for the PR.

@ashawley ashawley deleted the disable-test-forking branch November 11, 2016 06:36
ashawley added a commit to ashawley/scala-xml that referenced this pull request Dec 17, 2016
* build.sbt (fork): SBT was leaking scala-xml from scala-compiler
because of an incorrect library dependency exclude rule.
ashawley added a commit to ashawley/scala-xml that referenced this pull request Dec 25, 2016
* build.sbt (fork): SBT was leaking scala-xml from scala-compiler
because of an incorrect library dependency exclude rule.
ashawley added a commit to ashawley/scala-xml that referenced this pull request Feb 7, 2017
* build.sbt (fork): SBT was leaking scala-xml from scala-compiler
because of an incorrect library dependency exclude rule.
ashawley added a commit to ashawley/scala-xml that referenced this pull request Feb 15, 2017
* build.sbt (fork): SBT was leaking scala-xml from scala-compiler
because of an incorrect library dependency exclude rule.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants