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

TreeHighlighter crash while generating an xml string at the console #18596

Closed
AlexITC opened this issue Sep 25, 2023 · 8 comments · Fixed by #19836
Closed

TreeHighlighter crash while generating an xml string at the console #18596

AlexITC opened this issue Sep 25, 2023 · 8 comments · Fixed by #19836
Assignees
Labels
area:parser area:repl good first issue Perfect for someone who wants to get started contributing itype:bug
Milestone

Comments

@AlexITC
Copy link

AlexITC commented Sep 25, 2023

I have tried sbt console as well as scala-cli console, in both cases, generating some valid xml strings crashes with a StackOverflowError. Running the same code without the console works as expected.

Also, generating a non-xml string just works ((1 to 200).foldRight("x") { case (_, n) => s"< $n >" }).

Previously, I was able to generate a different error which I can't reproduce again because I can't find which Scala version I used in that session:

error

Compiler version

3.3.0

Minimized example

(1 to 200).foldRight("x") { case (_, n) => s"<x>$n</x>" }

Output

Exception in thread "main" java.lang.StackOverflowError
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1545)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.traverseChildren(untpd.scala:809)
	at dotty.tools.dotc.printing.SyntaxHighlighting$TreeHighlighter$2$.traverse(SyntaxHighlighting.scala:123)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1545)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.traverseChildren(untpd.scala:809)
	at dotty.tools.dotc.printing.SyntaxHighlighting$TreeHighlighter$2$.traverse(SyntaxHighlighting.scala:123)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1545)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.traverseChildren(untpd.scala:809)
	at dotty.tools.dotc.printing.SyntaxHighlighting$TreeHighlighter$2$.traverse(SyntaxHighlighting.scala:123)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1557)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.traverseChildren(untpd.scala:809)
	at dotty.tools.dotc.printing.SyntaxHighlighting$TreeHighlighter$2$.traverse(SyntaxHighlighting.scala:123)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1545)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.traverseChildren(untpd.scala:809)
	at dotty.tools.dotc.printing.SyntaxHighlighting$TreeHighlighter$2$.traverse(SyntaxHighlighting.scala:123)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1551)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.traverseChildren(untpd.scala:809)
	at dotty.tools.dotc.printing.SyntaxHighlighting$TreeHighlighter$2$.traverse(SyntaxHighlighting.scala:123)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:808)
	at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1565)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.traverseChildren(untpd.scala:809)
	at dotty.tools.dotc.printing.SyntaxHighlighting$TreeHighlighter$2$.traverse(SyntaxHighlighting.scala:123)

Expectation

The console mustn't crash.

@AlexITC AlexITC added the stat:needs triage Every issue needs to have an "area" and "itype" label label Sep 25, 2023
@nicolasstucki nicolasstucki added area:repl area:parser itype:bug and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Sep 27, 2023
@nicolasstucki
Copy link
Contributor

When I tried it it got stuck in an infinite loop.

@AlexITC
Copy link
Author

AlexITC commented Oct 3, 2023

I played a bit with this and the behavior seems non-deterministic, for example, I have got the crash with 50 iterations but the next time it just works.

@mbovel
Copy link
Member

mbovel commented Feb 19, 2024

Reproduced today:

sbt:scala3> repl
[info] compiling 11 Scala sources to /Users/mbovel/dotty/out/bootstrap/scala3-compiler-bootstrapped/scala-3.4.2-RC1-bin-SNAPSHOT-nonbootstrapped/classes ...
[info] compiling 28 Scala sources to /Users/mbovel/dotty/out/bootstrap/scala3-compiler-bootstrapped/scala-3.4.2-RC1-bin-SNAPSHOT-nonbootstrapped/classes ...
Welcome to Scala 3.4.2-RC1-bin-SNAPSHOT-nonbootstrapped-git-678c16b (17.0.8, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                                                                                              
scala> (1 to 200).foldRight("x") { case (_, n) => s"<x>$n</x>" }
[error] java.lang.StackOverflowError
[error]         at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1665)
[error]         at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.traverseChildren(untpd.scala:814)
[error]         at dotty.tools.dotc.printing.SyntaxHighlighting$TreeHighlighter$2$.traverse(SyntaxHighlighting.scala:123)
[error]         at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:813)
[error]         at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.apply(untpd.scala:813)
[error]         at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1673)

@mbovel
Copy link
Member

mbovel commented Feb 19, 2024

I believe there is nothing wrong per se in the implementation: the highlighter just overflows the stack if the expression is too deep.

Should we maybe catch StackOverflow exceptions at the top to make it more robust?

@mbovel
Copy link
Member

mbovel commented Feb 19, 2024

We could add a unit test for this:

diff --git a/compiler/test/dotty/tools/repl/ReplCompilerTests.scala b/compiler/test/dotty/tools/repl/ReplCompilerTests.scala
index 26092b73f1..4b4aca7dc0 100644
--- a/compiler/test/dotty/tools/repl/ReplCompilerTests.scala
+++ b/compiler/test/dotty/tools/repl/ReplCompilerTests.scala
@@ -421,6 +421,10 @@ object ReplCompilerTests:
 
 end ReplCompilerTests
 
+class ReplHighlightTests extends ReplTest(ReplTest.defaultOptions.filterNot(_.startsWith("-color")) :+ "-color:always"):
+  @Test def i18596: Unit = initially:
+    run("""(1 to 500).foldRight("x") { case (_, n) => s"<x>$n</x>" }""")
+
 class ReplXPrintTyperTests extends ReplTest(ReplTest.defaultOptions :+ "-Xprint:typer"):
   @Test def i9111 = initially {
     run("""|enum E {

To be run with:

sbt:scala3> scala3-compiler / Test / testOnly *ReplHighlightTests

@mbovel mbovel added the good first issue Perfect for someone who wants to get started contributing label Feb 19, 2024
@mbovel mbovel self-assigned this Feb 19, 2024
@AlexITC
Copy link
Author

AlexITC commented Feb 19, 2024

I believe there is nothing wrong per se in the implementation: the highlighter just overflows the stack if the expression is too deep.

While this makes sense, the behavior I experience was non-deterministic.

@mbovel
Copy link
Member

mbovel commented Feb 19, 2024

While this makes sense, the behavior I experience was non-deterministic.

Oh, I missed that, interesting. I just tried again and got different results depending on the sequence of expressions entered in the REPL.

For example, trying with to 60 directly seem to always fail for me:

➜  ~ scala-cli repl -S "3.nightly" --jvm 17
Welcome to Scala 3.4.2-RC1-bin-20240219-9a79c14-NIGHTLY-git-9a79c14 (17.0.6, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                                      
scala> (1 to 60).foldRight("x") { case (_, n) => s"<x>$n</x>" }
Exception in thread "main" java.lang.StackOverflowError
	at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1665)
	at dotty.tools.dotc.ast.untpd$UntypedTreeTraverser.traverseChildren(untpd.scala:814)
	at dotty.tools.dotc.printing.SyntaxHighlighting$TreeHighlighter$2$.traverse(SyntaxHighlighting.scala:123)
        ...

While trying with to 40 first and then with to 60 seem to always work:

➜  ~ scala-cli repl -S "3.3.2" --jvm 17    
Welcome to Scala 3.3.2 (17.0.6, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                                      
scala> (1 to 40).foldRight("x") { case (_, n) => s"<x>$n</x>" }
val res0: String = <x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x>x</x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x>
                                                                                                                      
scala> (1 to 60).foldRight("x") { case (_, n) => s"<x>$n</x>" }
val res1: String = <x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x><x>x</x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x></x>

So maybe the result is deterministic but dependent on previous steps? Or did you observe otherwise?

@AlexITC
Copy link
Author

AlexITC commented Feb 19, 2024

So maybe the result is deterministic but dependent on previous steps? Or did you observe otherwise?

I remember experiencing that, trying this right now leads to this (new session per line scala-cli console):

  1. 600 fails
  2. 500 then 600, both worh.
  3. 590 fails
  4. 590 works

Case 3 and 4 weren't simple to hit.

Valentin889 pushed a commit to mbovel/dotty that referenced this issue Feb 28, 2024
@Kordyjan Kordyjan added this to the 3.4.2 milestone Mar 28, 2024
@Kordyjan Kordyjan modified the milestones: 3.4.2, 3.5.0 May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:parser area:repl good first issue Perfect for someone who wants to get started contributing itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants