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

bugfix: Try to make ending taks more reliable #6793

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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 @@ -177,7 +177,7 @@ final class ForwardingMetalsBuildClient(
compilations.remove(target).foreach(_.end())

val name = info.getDisplayName
val token =
val (_, token) =
workDoneProgress.startProgress(
s"Compiling $name",
withProgress = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.ScheduledExecutorService
import java.util.concurrent.ScheduledFuture
import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicBoolean

import scala.concurrent.ExecutionContext
import scala.concurrent.Future
import scala.util.control.NonFatal

import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals.WorkDoneProgress.Token
Expand All @@ -32,6 +34,7 @@ class WorkDoneProgress(
maybeProgress: Option[TaskProgress],
) {
val timer = new Timer(time)
val wasFinished = new AtomicBoolean(false)
def additionalMessage: Option[String] =
if (showTimer) {
val seconds = timer.elapsedSeconds
Expand Down Expand Up @@ -79,15 +82,14 @@ class WorkDoneProgress(
withProgress: Boolean = false,
showTimer: Boolean = true,
onCancel: Option[() => Unit] = None,
): Future[Token] = {
): (Task, Future[Token]) = {
val uuid = UUID.randomUUID().toString()
val token = messages.Either.forLeft[String, Integer](uuid)

val optProgress = Option.when(withProgress)(TaskProgress.empty)
val task = Task(onCancel, showTimer, optProgress)
taskMap.put(token, task)

client
val tokenFuture = client
.createProgress(new WorkDoneProgressCreateParams(token))
.asScala
.map { _ =>
Expand All @@ -105,8 +107,10 @@ class WorkDoneProgress(
begin
)
client.notifyProgress(new ProgressParams(token, notification))
taskMap.put(token, task)
token
}
(task, tokenFuture)
}

def notifyProgress(
Expand All @@ -130,16 +134,22 @@ class WorkDoneProgress(
}

private def notifyProgress(token: Token, task: Task): Unit = {
val report = new WorkDoneProgressReport()
task.maybeProgress.foreach { progress =>
report.setPercentage(progress.percentage)
// make sure we don't update if a task was finished
if (task.wasFinished.get()) {
endProgress(Future.successful(token))
taskMap.remove(token)
} else {
val report = new WorkDoneProgressReport()
task.maybeProgress.foreach { progress =>
report.setPercentage(progress.percentage)
}
task.additionalMessage.foreach(report.setMessage)
val notification =
messages.Either.forLeft[WorkDoneProgressNotification, Object](
report
)
client.notifyProgress(new ProgressParams(token, notification))
}
task.additionalMessage.foreach(report.setMessage)
val notification =
messages.Either.forLeft[WorkDoneProgressNotification, Object](
report
)
client.notifyProgress(new ProgressParams(token, notification))
}

def endProgress(token: Future[Token]): Future[Unit] =
Expand All @@ -151,8 +161,11 @@ class WorkDoneProgress(
messages.Either.forLeft[WorkDoneProgressNotification, Object](end)
client.notifyProgress(new ProgressParams(token, params))
}
.recover { case _: NullPointerException =>
// no such value in the task map, task already ended or cancelled
.recover {
case _: NullPointerException =>
// no such value in the task map, task already ended or cancelled
case NonFatal(e) =>
scribe.error("Could not end a progress task", e)
}

def trackFuture[T](
Expand All @@ -161,16 +174,22 @@ class WorkDoneProgress(
onCancel: Option[() => Unit] = None,
showTimer: Boolean = true,
)(implicit ec: ExecutionContext): Future[T] = {
val token =
val (task, token) =
startProgress(message, onCancel = onCancel, showTimer = showTimer)
value.onComplete(_ => endProgress(token))
value.onComplete { _ =>
task.wasFinished.set(true)
endProgress(token)
}
value
}

def trackBlocking[T](message: String)(thunk: => T): T = {
val token = startProgress(message)
val (task, token) = startProgress(message)
try thunk
finally endProgress(token)
finally {
task.wasFinished.set(true)
endProgress(token)
}
}

def canceled(token: Token): Unit =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,15 @@ class WorksheetProvider(
val interruptThread = new Runnable {
def run(): Unit = {
if (!result.isDone()) {
val token = workDoneProgress.startProgress(
val (task, token) = workDoneProgress.startProgress(
s"Evaluating worksheet '${path.filename}'",
onCancel = Some(cancellable.cancel),
)

result.asScala.onComplete(_ => workDoneProgress.endProgress(token))
result.asScala.onComplete { _ =>
task.wasFinished.set(true)
workDoneProgress.endProgress(token)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ final class AutoImportsProvider(
sym.name.dropLocal.decoded == name

symbols.result().collect {
case sym
if isExactMatch(sym, name) && context.isAccessible(sym, sym.info) =>
case sym if isExactMatch(sym, name) =>
val pkg = sym.owner.fullName
val edits = importPosition match {
// if we are in import section just specify full name
Expand Down
Loading