bugfix: fix reporting "no response error" after laptop in sleep mode

scalameta · Oct 5, 2023 · 2a80cae · 2a80cae
1 parent e3b108a
commit 2a80cae
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 34 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala b/metals/src/main/scala/scala/meta/internal/metals/ServerLivenessMonitor.scala
@@ -4,7 +4,6 @@ import java.util.concurrent.Executors
 import java.util.concurrent.ScheduledExecutorService
 import java.util.concurrent.ScheduledFuture
 import java.util.concurrent.TimeUnit
-import java.util.concurrent.atomic.AtomicReference
 
 import scala.concurrent.duration.Duration
 
@@ -60,9 +59,8 @@ class ServerLivenessMonitor(
     serverName: String,
     icons: Icons,
 ) {
-  private val state: AtomicReference[ServerLivenessMonitor.State] =
-    new AtomicReference(ServerLivenessMonitor.Idle)
   @volatile private var isServerResponsive = true
+  @volatile private var lastPing: Long = 0
   val scheduler: ScheduledExecutorService = Executors.newScheduledThreadPool(1)
   val connectedParams: MetalsStatusParams =
     ServerLivenessMonitor.connectedParams(serverName, icons)
@@ -74,31 +72,14 @@ class ServerLivenessMonitor(
 
   def runnable(): Runnable = new Runnable {
     def run(): Unit = {
-      def now = System.currentTimeMillis()
       def lastIncoming =
         requestMonitor.lastIncoming
           .map(now - _)
           .getOrElse(pingInterval.toMillis)
       def notResponding = lastIncoming > (pingInterval.toMillis * 2)
-      def metalsIsIdle =
-        requestMonitor.lastOutgoing
-          .map(lastOutgoing =>
-            (now - lastOutgoing) > metalsIdleInterval.toMillis
-          )
-          .getOrElse(true)
       if (!metalsIsIdle) {
-        val currState = state.getAndUpdate {
-          case ServerLivenessMonitor.Idle => ServerLivenessMonitor.FirstPing
-          case _ => ServerLivenessMonitor.Running
-        }
-        currState match {
-          case ServerLivenessMonitor.Idle =>
-            scribe.debug("setting server liveness monitor state to FirstPing")
-          case ServerLivenessMonitor.FirstPing =>
-            scribe.debug("setting server liveness monitor state to Running")
-          case _ =>
-        }
-        if (currState == ServerLivenessMonitor.Running) {
+        scribe.debug(s"server liveness monitor is last ping ok: $lastPingOk")
+        if (lastPingOk) {
           if (notResponding && isServerResponsive) {
             scribe.debug("server liveness monitor detected no response")
             client.metalsStatus(noResponseParams)
@@ -107,14 +88,21 @@ class ServerLivenessMonitor(
           isServerResponsive = !notResponding
         }
         scribe.debug("server liveness monitor: pinging build server...")
+        lastPing = now
         ping()
-      } else {
-        if (state.get() != ServerLivenessMonitor.Idle)
-          scribe.debug("setting server liveness monitor state to Idle")
-        state.set(ServerLivenessMonitor.Idle)
       }
     }
   }
+
+  private def now = System.currentTimeMillis()
+
+  def metalsIsIdle: Boolean =
+    requestMonitor.lastOutgoing
+      .map(lastOutgoing => (now - lastOutgoing) > metalsIdleInterval.toMillis)
+      .getOrElse(true)
+
+  def lastPingOk: Boolean = now - lastPing < (pingInterval.toMillis * 2)
+
   val scheduled: ScheduledFuture[_ <: Object] = scheduler.scheduleAtFixedRate(
     runnable(),
     pingInterval.toMillis,
@@ -131,7 +119,6 @@ class ServerLivenessMonitor(
     client.metalsStatus(ServerLivenessMonitor.disconnectedParams)
   }
 
-  def getState: ServerLivenessMonitor.State = state.get()
 }
 
 object ServerLivenessMonitor {

diff --git a/tests/unit/src/test/scala/tests/ServerLivenessMonitorSuite.scala b/tests/unit/src/test/scala/tests/ServerLivenessMonitorSuite.scala
@@ -32,19 +32,20 @@ class ServerLivenessMonitorSuite extends BaseSuite {
       Icons.default,
     )
     Thread.sleep(pingInterval.toMillis * 3 / 2)
-    assertEquals(livenessMonitor.getState, ServerLivenessMonitor.Idle)
+    assert(livenessMonitor.metalsIsIdle)
     server.sendRequest(false)
     Thread.sleep(pingInterval.toMillis * 2)
-    assertNotEquals(livenessMonitor.getState, ServerLivenessMonitor.Idle)
+    assert(!livenessMonitor.metalsIsIdle)
     Thread.sleep(pingInterval.toMillis * 5)
-    assertEquals(livenessMonitor.getState, ServerLivenessMonitor.Idle)
+    assert(livenessMonitor.metalsIsIdle)
     server.sendRequest(false)
     Thread.sleep(pingInterval.toMillis)
     server.sendRequest(false)
     server.sendRequest(false)
     Thread.sleep(pingInterval.toMillis * 2)
     server.sendRequest(false)
-    assertEquals(livenessMonitor.getState, ServerLivenessMonitor.Running)
+    assert(!livenessMonitor.metalsIsIdle)
+    assert(livenessMonitor.lastPingOk)
     assert(client.showMessageRequests == 0)
   }
 }
@@ -85,7 +86,10 @@ class CountMessageRequestsClient extends NoopLanguageClient {
   var showMessageRequests = 0
 
   override def metalsStatus(params: MetalsStatusParams): Unit =
-    if (params == ServerLivenessMonitor.disconnectedParams) {
-      showMessageRequests += 1
-    }
+    if (
+      params == ServerLivenessMonitor.noResponseParams(
+        "responsive-server",
+        Icons.default,
+      )
+    ) showMessageRequests += 1
 }