Skip to content

Commit f4868b6

Browse files
author
Sebastian Haracz
committed
Guard against polling during parse/enter in presentation compiler
Add a createIGlobal factory to ScexPresentationCompiler and a test that installs an IGlobal recording any checkForMoreWork call made while Global.parseAndEnter is on the stack. It fails deterministically if the signalParseProgress override is dropped or a scalac upgrade adds another check point inside parse/enter, unlike the timing-dependent original flake. Also make the signalParseProgress comment accurate: it defers everything pollForWork does (interrupts, work items, cancellation, shutdown and fresh-run requests, yielding) for every parse on the compiler thread.
1 parent 5306c2c commit f4868b6

3 files changed

Lines changed: 70 additions & 4 deletions

File tree

‎scex-core/src/main/scala/com/avsystem/scex/compiler/presentation/IGlobal.scala‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@ class IGlobal(settings: Settings, reporter: Reporter, val classLoader: ClassLoad
1818

1919
import definitions._
2020

21-
// Don't service interrupts (askForResponse) while parsing. Background compile resets a stale unit (clearing its
22-
// synthetics) and only then re-parses and re-enters it, so an interrupt run from the parser could force a lazy
23-
// accessor symbol left over from an aborted pass and crash with "no synthetic tree found for bean accessor".
21+
// Don't poll for work while parsing. Background compile resets a stale unit (clearing its synthetics) and only then
22+
// re-parses and re-enters it, so work run from the parser (e.g. our askForResponse calls) could force a lazy accessor
23+
// symbol left over from an aborted pass and crash with "no synthetic tree found for bean accessor".
24+
// This applies to every parse on the compiler thread and defers everything pollForWork does: interrupts, queued work
25+
// items, cancellation checks, shutdown and fresh-run requests, and yielding. All of it resumes at the next typer
26+
// check point. Scex sources are small, so the delay is negligible. Guarded by ParsePollingTest.
2427
override def signalParseProgress(pos: Position): Unit = ()
2528

2629
abstract class ScexMember extends Member {

‎scex-core/src/main/scala/com/avsystem/scex/compiler/presentation/ScexPresentationCompiler.scala‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ trait ScexPresentationCompiler extends ScexCompiler { compiler =>
2727
protected def isEnabled: Boolean = !settings.noPresentation.value
2828
protected def presentationCompilerSettings: Settings = settings
2929

30+
protected def createIGlobal(settings: Settings, reporter: Reporter, classLoader: ClassLoader): IGlobal =
31+
new IGlobal(settings, reporter, classLoader)
32+
3033
private var reporter: Reporter = _
3134
private var global: IGlobal = _
3235

@@ -35,7 +38,7 @@ trait ScexPresentationCompiler extends ScexCompiler { compiler =>
3538
if (isEnabled) {
3639
logger.info("Initializing Scala presentation compiler")
3740
reporter = new Reporter(presentationCompilerSettings)
38-
global = new IGlobal(presentationCompilerSettings, reporter, getSharedClassLoader)
41+
global = createIGlobal(presentationCompilerSettings, reporter, getSharedClassLoader)
3942
}
4043
}
4144

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
package com.avsystem.scex.compiler.presentation
2+
3+
import com.avsystem.scex.compiler.presentation.ScopeAndTypeCompletionTest.Root
4+
import com.avsystem.scex.compiler.{CompilationTest, ScexSettings}
5+
import com.avsystem.scex.japi.{DefaultJavaScexCompiler, JavaScexCompiler}
6+
import com.avsystem.scex.util.SimpleContext
7+
import org.scalatest.funsuite.AnyFunSuite
8+
9+
import java.util.concurrent.ConcurrentLinkedQueue
10+
import java.util.concurrent.atomic.AtomicInteger
11+
import scala.jdk.CollectionConverters._
12+
import scala.tools.nsc.Settings
13+
14+
/** Background compile resets a stale unit (clearing its synthetics) before re-parsing and re-entering it. If the
15+
* presentation compiler polled for work in between, a scex request could force an accessor symbol whose synthetic
16+
* tree is already gone ("no synthetic tree found for bean accessor"). This test fails deterministically if any
17+
* polling happens while a unit is being parsed and entered, regardless of whether the race actually triggers.
18+
*/
19+
class ParsePollingTest extends AnyFunSuite with CompilationTest {
20+
21+
private val pollCount = new AtomicInteger
22+
private val pollsDuringParseAndEnter = new ConcurrentLinkedQueue[String]
23+
24+
override protected def createCompiler: JavaScexCompiler = {
25+
val settings = new ScexSettings
26+
settings.classfileDirectory.value = "testClassfileCache"
27+
new DefaultJavaScexCompiler(settings) {
28+
override protected def createIGlobal(settings: Settings, reporter: Reporter, classLoader: ClassLoader) =
29+
new IGlobal(settings, reporter, classLoader) {
30+
override protected def checkForMoreWork(pos: Position): Unit = {
31+
pollCount.incrementAndGet()
32+
val inParseAndEnter = StackWalker.getInstance.walk { frames =>
33+
frames.anyMatch(f => f.getClassName == "scala.tools.nsc.interactive.Global" && f.getMethodName == "parseAndEnter")
34+
}
35+
if (inParseAndEnter) {
36+
pollsDuringParseAndEnter.add(Thread.currentThread.getStackTrace.mkString("\n "))
37+
}
38+
super.checkForMoreWork(pos)
39+
}
40+
}
41+
}
42+
}
43+
44+
val utils =
45+
"""
46+
|def utilMethod: Int = 42
47+
|val utilStuff = 5
48+
""".stripMargin
49+
50+
val profile = createProfile(utils = utils)
51+
52+
test("presentation compiler does not poll for work while parsing and entering a unit") {
53+
val completer = compiler.getCompleter[SimpleContext[Root], Any](profile, template = false)
54+
completer.getScopeCompletion
55+
completer.getTypeCompletion("_root.", 5)
56+
57+
assert(pollCount.get > 0, "custom IGlobal was not used")
58+
assert(pollsDuringParseAndEnter.isEmpty, pollsDuringParseAndEnter.asScala.headOption.getOrElse(""))
59+
}
60+
}

0 commit comments

Comments
 (0)