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

Ensure slow shutdown of LS always kicks off hooks #6665

Merged
merged 1 commit into from
May 15, 2023
Merged
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 @@ -297,6 +297,9 @@ class LanguageServerController(
s"Received client ($clientId) disconnect request during shutdown. Ignoring."
)

case ShutDownServer =>
logger.debug(s"Received shutdown request during shutdown. Ignoring.")

case m: StartServer =>
// This instance has not yet been shut down. Retry
context.parent.forward(Retry(m))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.enso.projectmanager.infrastructure.languageserver

import akka.actor.{Actor, ActorRef, Cancellable, PoisonPill, Props, Terminated}
import com.typesafe.scalalogging.LazyLogging
import org.enso.projectmanager.event.ProjectEvent.ProjectClosed
import org.enso.projectmanager.infrastructure.languageserver.LanguageServerController.ShutDownServer
import org.enso.projectmanager.infrastructure.languageserver.LanguageServerKiller.KillTimeout
import org.enso.projectmanager.infrastructure.languageserver.LanguageServerProtocol.{
Expand All @@ -10,6 +11,7 @@ import org.enso.projectmanager.infrastructure.languageserver.LanguageServerProto
}
import org.enso.projectmanager.util.UnhandledLogging

import java.util.UUID
import scala.concurrent.duration.FiniteDuration

/** An actor that shuts all running language servers. It orchestrates all
Expand All @@ -20,7 +22,7 @@ import scala.concurrent.duration.FiniteDuration
* @param shutdownTimeout a shutdown timeout
*/
class LanguageServerKiller(
controllers: List[ActorRef],
controllers: Map[UUID, ActorRef],
shutdownTimeout: FiniteDuration
) extends Actor
with LazyLogging
Expand All @@ -34,20 +36,22 @@ class LanguageServerKiller(
context.stop(self)
} else {
logger.info("Killing all servers [{}].", controllers)
controllers.foreach(context.watch)
controllers.foreach(_ ! ShutDownServer)
controllers.foreach { case (_, ref) =>
context.watch(ref)
ref ! ShutDownServer
}
val cancellable =
context.system.scheduler.scheduleOnce(
shutdownTimeout,
self,
KillTimeout
)
context.become(killing(controllers.toSet, cancellable, sender()))
context.become(killing(controllers.map(_.swap), cancellable, sender()))
}
}

private def killing(
liveControllers: Set[ActorRef],
liveControllers: Map[ActorRef, UUID],
cancellable: Cancellable,
replyTo: ActorRef
): Receive = {
Expand All @@ -63,7 +67,13 @@ class LanguageServerKiller(
}

case KillTimeout =>
liveControllers.foreach(_ ! PoisonPill)
logger.warn(
s"Not all language servers' controllers finished on time. Forcing termination."
)
liveControllers.foreach { case (actorRef, projectId) =>
actorRef ! PoisonPill
context.system.eventStream.publish(ProjectClosed(projectId))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If one was to reduce this fix to a single line, then this is it.

}
context.stop(self)
}

Expand All @@ -80,7 +90,7 @@ object LanguageServerKiller {
* @return a configuration object
*/
def props(
controllers: List[ActorRef],
controllers: Map[UUID, ActorRef],
shutdownTimeout: FiniteDuration
): Props = Props(new LanguageServerKiller(controllers, shutdownTimeout))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class LanguageServerRegistry(
case msg @ KillThemAll =>
val killer = context.actorOf(
LanguageServerKiller.props(
serverControllers.values.toList,
serverControllers,
timeoutConfig.shutdownTimeout
),
"language-server-killer"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,36 @@ class ShutdownHookActivator[F[+_, +_]: Exec: CovariantFlatMap]

private def running(
hooks: Map[UUID, List[ShutdownHook[F]]] =
Map.empty.withDefaultValue(List.empty)
Map.empty.withDefaultValue(List.empty),
scheduled: List[UUID] = Nil
): Receive = {
case RegisterShutdownHook(projectId, hook) =>
val realHook = hook.asInstanceOf[ShutdownHook[F]]
val updated = hooks.updated(projectId, realHook :: hooks(projectId))
context.become(running(updated))
context.become(running(updated, scheduled))

case ProjectClosed(projectId) =>
val projectHooks = hooks(projectId)
if (projectHooks.nonEmpty) {
context.actorOf(
ShutdownHookRunner.props[F](projectId, projectHooks.reverse)
)
context.become(running(hooks - projectId, projectId :: scheduled))
} else if (scheduled.contains(projectId)) {
logger.debug(
s"Request for starting shutdown hooks has already been filed for project ${projectId}. Ignoring."
)
} else {
logger.warn(
s"Shutdown hook activator has no recollection of project ${projectId}. Either it was closed already or it never existed. Ignoring."
)
}

case ShutdownHooksFired(projectId) =>
context.become(running(hooks - projectId))
context.become(running(hooks, scheduled.filter(_ != projectId)))

case ArePendingShutdownHooks =>
val arePending = hooks.values.map(_.size).sum != 0
val arePending = hooks.values.map(_.size).sum != 0 || scheduled.nonEmpty
sender() ! arePending
}

Expand Down