improvement: start liveness monitor only for main bsp connection

metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala

@@ -77,15 +77,18 @@ class BspConnector(
       shellRunner: ShellRunner,
   )(implicit ec: ExecutionContext): Future[Option[BspSession]] = {
     def connect(
-        workspace: AbsolutePath
+        workspace: AbsolutePath,
+        addLivenessMonitor: Boolean,
     ): Future[Option[BuildServerConnection]] = {
       scribe.info("Attempting to connect to the build server...")
       resolve() match {
         case ResolvedNone =>
           scribe.info("No build server found")
           Future.successful(None)
         case ResolvedBloop =>
-          bloopServers.newServer(workspace, userConfiguration).map(Some(_))
+          bloopServers
+            .newServer(workspace, userConfiguration, addLivenessMonitor)
+            .map(Some(_))
         case ResolvedBspOne(details)
             if details.getName() == SbtBuildTool.name =>
           tables.buildServers.chooseServer(SbtBuildTool.name)
@@ -94,7 +97,11 @@ class BspConnector(
             for {
               _ <- SbtBuildTool(workspace, () => userConfiguration)
                 .ensureCorrectJavaVersion(shellRunner, workspace, client)
-              connection <- bspServers.newServer(workspace, details)
+              connection <- bspServers.newServer(
+                workspace,
+                details,
+                addLivenessMonitor,
+              )
               _ <-
                 if (shouldReload) connection.workspaceReload()
                 else Future.successful(())
@@ -104,7 +111,9 @@ class BspConnector(
             .map(Some(_))
         case ResolvedBspOne(details) =>
           tables.buildServers.chooseServer(details.getName())
-          bspServers.newServer(workspace, details).map(Some(_))
+          bspServers
+            .newServer(workspace, details, addLivenessMonitor)
+            .map(Some(_))
         case ResolvedMultiple(_, availableServers) =>
           val distinctServers = availableServers
             .groupBy(_.getName())
@@ -136,26 +145,33 @@ class BspConnector(
                 )
               )
             _ = tables.buildServers.chooseServer(item.getName())
-            conn <- bspServers.newServer(workspace, item)
+            conn <- bspServers.newServer(
+              workspace,
+              item,
+              addLivenessMonitor,
+            )
           } yield Some(conn)
       }
     }
 
-    connect(workspace).flatMap { possibleBuildServerConn =>
-      possibleBuildServerConn match {
-        case None => Future.successful(None)
-        case Some(buildServerConn)
-            if buildServerConn.isBloop && buildTools.isSbt =>
-          // NOTE: (ckipp01) we special case this here since sbt bsp server
-          // doesn't yet support metabuilds. So in the future when that
-          // changes, re-work this and move the creation of this out above
-          val metaConns = sbtMetaWorkspaces(workspace).map(connect(_))
-          Future
-            .sequence(metaConns)
-            .map(meta => Some(BspSession(buildServerConn, meta.flatten)))
-        case Some(buildServerConn) =>
-          Future(Some(BspSession(buildServerConn, List.empty)))
-      }
+    connect(workspace, addLivenessMonitor = true).flatMap {
+      possibleBuildServerConn =>
+        possibleBuildServerConn match {
+          case None => Future.successful(None)
+          case Some(buildServerConn)
+              if buildServerConn.isBloop && buildTools.isSbt =>
+            // NOTE: (ckipp01) we special case this here since sbt bsp server
+            // doesn't yet support metabuilds. So in the future when that
+            // changes, re-work this and move the creation of this out above
+            val metaConns = sbtMetaWorkspaces(workspace).map(
+              connect(_, addLivenessMonitor = false)
+            )
+            Future
+              .sequence(metaConns)
+              .map(meta => Some(BspSession(buildServerConn, meta.flatten)))
+          case Some(buildServerConn) =>
+            Future(Some(BspSession(buildServerConn, List.empty)))
+        }
     }
   }
 

metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala

@@ -68,6 +68,7 @@ final class BspServers(
   def newServer(
       projectDirectory: AbsolutePath,
       details: BspConnectionDetails,
+      addLivenessMonitor: Boolean,
   ): Future[BuildServerConnection] = {
 
     def newConnection(): Future[SocketConnection] = {
@@ -140,6 +141,7 @@ final class BspServers(
       tables.dismissedNotifications.ReconnectBsp,
       config,
       details.getName(),
+      addLivenessMonitor,
     )
   }
 

metals/src/main/scala/scala/meta/internal/metals/BloopServers.scala

@@ -80,6 +80,7 @@ final class BloopServers(
   def newServer(
       workspace: AbsolutePath,
       userConfiguration: UserConfiguration,
+      addLivenessMonitor: Boolean,
   ): Future[BuildServerConnection] = {
     val bloopVersion = userConfiguration.currentBloopVersion
     BuildServerConnection
@@ -92,6 +93,7 @@ final class BloopServers(
         tables.dismissedNotifications.ReconnectBsp,
         config,
         name,
+        addLivenessMonitor,
       )
   }
 

metals/src/main/scala/scala/meta/internal/metals/BuildServerConnection.scala

@@ -34,14 +34,15 @@ import ch.epfl.scala.bsp4j._
 import com.google.gson.Gson
 import org.eclipse.lsp4j.jsonrpc.JsonRpcException
 import org.eclipse.lsp4j.jsonrpc.Launcher
+import org.eclipse.lsp4j.jsonrpc.MessageConsumer
 import org.eclipse.lsp4j.jsonrpc.MessageIssueException
 import org.eclipse.lsp4j.services.LanguageClient
 
 /**
  * An actively running and initialized BSP connection
  */
 class BuildServerConnection private (
-    reestablishConnection: () => Future[
+    setupConnection: () => Future[
       BuildServerConnection.LauncherConnection
     ],
     initialConnection: BuildServerConnection.LauncherConnection,
@@ -54,7 +55,13 @@ class BuildServerConnection private (
     extends Cancelable {
 
   @volatile private var connection = Future.successful(initialConnection)
-  initialConnection.onConnectionFinished(reconnect)
+  initialConnection.setReconnect(() => reconnect().ignoreValue)
+  private def reestablishConnection(
+      original: Future[BuildServerConnection.LauncherConnection]
+  ) = {
+    original.foreach(_.optLivenessMonitor.foreach(_.shutdown()))
+    setupConnection()
+  }
 
   private val isShuttingDown = new AtomicBoolean(false)
   private val onReconnection =
@@ -70,7 +77,7 @@ class BuildServerConnection private (
 
   def version: String = _version.get()
 
-  // the name is set before when establishing conenction
+  // the name is set before when establishing connection
   def name: String = initialConnection.socketConnection.serverName
   private def capabilities: BuildServerCapabilities =
     initialConnection.capabilities
@@ -125,7 +132,7 @@ class BuildServerConnection private (
         if (isShuttingDown.compareAndSet(false, true)) {
           conn.server.buildShutdown().get(2, TimeUnit.SECONDS)
           conn.server.onBuildExit()
-          conn.livenessMonitor.shutdown()
+          conn.optLivenessMonitor.foreach(_.shutdown())
           scribe.info("Shut down connection with build server.")
           // Cancel pending compilations on our side, this is not needed for Bloop.
           cancel()
@@ -300,13 +307,15 @@ class BuildServerConnection private (
     ongoingCompilations.cancel()
   }
 
-  private def askUser(): Future[BuildServerConnection.LauncherConnection] = {
+  private def askUser(
+      original: Future[BuildServerConnection.LauncherConnection]
+  ): Future[BuildServerConnection.LauncherConnection] = {
     if (config.askToReconnect) {
       if (!reconnectNotification.isDismissed) {
         val params = Messages.DisconnectedServer.params()
         languageClient.showMessageRequest(params).asScala.flatMap {
           case response if response == Messages.DisconnectedServer.reconnect =>
-            reestablishConnection()
+            reestablishConnection(original)
           case response if response == Messages.DisconnectedServer.notNow =>
             reconnectNotification.dismiss(5, TimeUnit.MINUTES)
             connection
@@ -317,7 +326,7 @@ class BuildServerConnection private (
         connection
       }
     } else {
-      reestablishConnection()
+      reestablishConnection(original)
     }
   }
 
@@ -327,11 +336,11 @@ class BuildServerConnection private (
       synchronized {
         // if the future is different then the connection is already being reestablished
         if (connection eq original) {
-          connection = askUser().map { conn =>
+          connection = askUser(original).map { conn =>
             // version can change when reconnecting
             _version.set(conn.version)
             ongoingRequests.addAll(conn.cancelables)
-            conn.onConnectionFinished(reconnect)
+            conn.setReconnect(() => reconnect().ignoreValue)
             conn
           }
           connection.foreach(_ => onReconnection.get()(this))
@@ -404,9 +413,12 @@ class BuildServerConnection private (
     CancelTokens.future(_ => actionFuture)
   }
 
-  def isBuildServerResponsive: Future[Boolean] = {
+  def isBuildServerResponsive: Future[Option[Boolean]] = {
     val original = connection
-    original.map(_.livenessMonitor.isBuildServerResponsive)
+    original.map(
+      _.optLivenessMonitor
+        .map(_.isBuildServerResponsive)
+    )
   }
 
 }
@@ -428,6 +440,7 @@ object BuildServerConnection {
       reconnectNotification: DismissedNotifications#Notification,
       config: MetalsServerConfig,
       serverName: String,
+      addLivenessMonitor: Boolean = false,
       retry: Int = 5,
       supportsWrappedSources: Option[Boolean] = None,
   )(implicit
@@ -437,16 +450,20 @@ object BuildServerConnection {
     def setupServer(): Future[LauncherConnection] = {
       connect().map { case conn @ SocketConnection(_, output, input, _, _) =>
         val tracePrinter = Trace.setupTracePrinter("BSP", workspace)
-        val requestMonitor = new RequestMonitorImpl
-        val launcher = new Launcher.Builder[MetalsBuildServer]()
-          .traceMessages(tracePrinter.orNull)
-          .setOutput(output)
-          .setInput(input)
-          .setLocalService(localClient)
-          .setRemoteInterface(classOf[MetalsBuildServer])
-          .setExecutorService(ec)
-          .wrapMessages(requestMonitor.wrapper(_))
-          .create()
+        val requestMonitor =
+          if (addLivenessMonitor) Some(new RequestMonitorImpl) else None
+        val wrapper: MessageConsumer => MessageConsumer =
+          requestMonitor.map(_.wrapper).getOrElse(identity)
+        val launcher =
+          new Launcher.Builder[MetalsBuildServer]()
+            .traceMessages(tracePrinter.orNull)
+            .setOutput(output)
+            .setInput(input)
+            .setLocalService(localClient)
+            .setRemoteInterface(classOf[MetalsBuildServer])
+            .setExecutorService(ec)
+            .wrapMessages(wrapper(_))
+            .create()
         val listening = launcher.startListening()
         val server = launcher.getRemoteProxy
         val stopListening =
@@ -461,21 +478,27 @@ object BuildServerConnection {
               scribe.error("Timeout waiting for 'build/initialize' response")
               throw e
           }
+
+        val optServerLivenessMonitor =
+          requestMonitor.map {
+            new ServerLivenessMonitor(
+              _,
+              () => server.workspaceBuildTargets(),
+              languageClient,
+              result.getDisplayName(),
+              config.metalsToIdleTime,
+              config.pingInterval,
+            )
+          }
+
         LauncherConnection(
           conn,
           server,
           result.getDisplayName(),
           stopListening,
           result.getVersion(),
           result.getCapabilities(),
-          new ServerLivenessMonitor(
-            requestMonitor,
-            () => server.workspaceBuildTargets(),
-            languageClient,
-            result.getDisplayName(),
-            config.metalsToIdleTime,
-            config.pingInterval,
-          ),
+          optServerLivenessMonitor,
         )
       }
     }
@@ -503,6 +526,7 @@ object BuildServerConnection {
             reconnectNotification,
             config,
             serverName,
+            addLivenessMonitor,
             retry - 1,
           )
         } else {
@@ -566,16 +590,17 @@ object BuildServerConnection {
       cancelServer: Cancelable,
       version: String,
       capabilities: BuildServerCapabilities,
-      livenessMonitor: ServerLivenessMonitor,
+      optLivenessMonitor: Option[ServerLivenessMonitor],
   ) {
 
     def cancelables: List[Cancelable] =
       cancelServer :: socketConnection.cancelables
 
-    def onConnectionFinished(
-        f: () => Unit
+    def setReconnect(
+        reconnect: () => Future[Unit]
     )(implicit ec: ExecutionContext): Unit = {
-      socketConnection.finishedPromise.future.foreach(_ => f())
+      optLivenessMonitor.foreach(_.setReconnect(reconnect))
+      socketConnection.finishedPromise.future.foreach(_ => reconnect())
     }
 
     /**

metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala

@@ -1642,7 +1642,9 @@ class MetalsLspService(
     val shutdownBsp =
       bspSession match {
         case Some(session) if session.main.isBloop =>
-          Future.successful(bloopServers.shutdownServer())
+          for {
+            _ <- disconnectOldBuildServer()
+          } yield bloopServers.shutdownServer()
         case Some(session) if session.main.isSbt =>
           for {
             currentBuildTool <- supportedBuildTool
@@ -2135,7 +2137,11 @@ class MetalsLspService(
     (for {
       _ <- disconnectOldBuildServer()
       maybeSession <- timerProvider.timed("Connected to build server", true) {
-        bspConnector.connect(folder, userConfig(), shellRunner)
+        bspConnector.connect(
+          folder,
+          userConfig(),
+          shellRunner,
+        )
       }
       result <- maybeSession match {
         case Some(session) =>