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

Avoid race condition in Reporter errorsBuilder #36

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ trait ScexCompiler extends LoggingUtils {

def displayPrompt(): Unit = {}

override def reset(): Unit = {
super.reset()
def clearErrors(): Unit = {
errorsBuilder.clear()
}
Comment on lines +73 to 75
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: add resetAndClearErrors method to avoid repeating subsequent reset and clearError calls in other places

}
Expand Down Expand Up @@ -221,9 +220,11 @@ trait ScexCompiler extends LoggingUtils {

protected final def withGlobal[T](code: ScexGlobal => T): T = underLock {
reporter.reset()
reporter.clearErrors()
val global = this.global
val result = try code(global) finally {
reporter.reset()
reporter.clearErrors()
}
result
}
Expand All @@ -241,6 +242,7 @@ trait ScexCompiler extends LoggingUtils {
val classfileDirectory = classLoader.classfileDirectory

reporter.reset()
reporter.clearErrors()

logger.debug(s"Compiling source file ${sourceFile.path} to $classfileDirectory:\n${new String(sourceFile.content)}")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,11 @@ trait ScexPresentationCompiler extends ScexCompiler { compiler =>

protected final def withIGlobal[T](code: IGlobal => T) = underLock {
reporter.reset()
reporter.clearErrors()
val global = compiler.global
val result = try code(global) finally {
reporter.reset()
reporter.clearErrors()
}
result
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.avsystem.scex.compiler

import com.avsystem.scex.compiler.presentation.TypeCompletionPrefixTest.Root
import com.avsystem.scex.japi.{DefaultJavaScexCompiler, JavaScexCompiler}
import com.avsystem.scex.util.SimpleContext
import org.scalatest.funsuite.AnyFunSuite

final class MultipleEvaluationsTest extends AnyFunSuite with CompilationTest {
override protected def createCompiler: JavaScexCompiler = {
val settings = new ScexSettings
settings.classfileDirectory.value = "testClassfileCache"

// for some reason enabling this setting fixes validation
settings.noGetterAdapters.value = false
new DefaultJavaScexCompiler(settings)
}

private val completer = compiler.getCompleter[SimpleContext[Root], Any](
createProfile(Nil),
template = false,
header = "",
)

test("Multiple evaluations of invalid expressions should always fail") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add test case with mixed correct and incorrect expressions

Also check corner case, when invalid expression is the last one before reaching settings.resetAfterCount and next expression is valid

(0 until 1000)
.map(i => s"invalidExpression $i") //added to avoid hitting the cache
.foreach(expr => if (completer.getErrors(expr).isEmpty) fail(s"Expression $expr should not compile"))
}
}
Loading