fix: stop workspace if server process was stopped (#23579) (#203)

adietish · web-flow · commit c26a281229ff · 2025-11-01T01:01:59.000+01:00
* fix: stop workspace if server process was stopped (#23579)
* added tests for RemoteIDEServer

Signed-off-by: Andre Dietisheim &lt;adietish@redhat.com&gt;
Assisted-by: gemini-cli

---------

Signed-off-by: Andre Dietisheim &lt;adietish@redhat.com&gt;
diff --git a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesConnection.kt
@@ -11,14 +11,17 @@
  */
 package com.redhat.devtools.gateway
 
-import com.intellij.openapi.diagnostic.thisLogger
 import com.jetbrains.gateway.thinClientLink.LinkedClientManager
 import com.jetbrains.gateway.thinClientLink.ThinClientHandle
 import com.jetbrains.rd.util.lifetime.Lifetime
 import com.redhat.devtools.gateway.openshift.DevWorkspaces
 import com.redhat.devtools.gateway.openshift.Pods
 import com.redhat.devtools.gateway.server.RemoteIDEServer
 import io.kubernetes.client.openapi.ApiException
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
+import java.io.Closeable
 import java.io.IOException
 import java.net.ServerSocket
 import java.net.URI
@@ -46,21 +49,14 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) {
         onDisconnected: () -> Unit
     ): ThinClientHandle {
         val workspace = devSpacesContext.devWorkspace
+        devSpacesContext.addWorkspace(workspace)
 
-        synchronized(devSpacesContext.activeWorkspaces) {
-            if (devSpacesContext.activeWorkspaces.contains(workspace)) {
-                throw IllegalStateException("Workspace '${workspace.name}' is already connected.")
-            }
-            devSpacesContext.activeWorkspaces.add(workspace)
-        }
-
-        var client: ThinClientHandle? = null
-        var forwarder: AutoCloseable? = null
+        var remoteIdeServer: RemoteIDEServer? = null
+        var forwarder: Closeable? = null
 
-        try {
+        return try {
             startAndWaitDevWorkspace()
-
-            val remoteIdeServer = RemoteIDEServer(devSpacesContext)
+            remoteIdeServer = RemoteIDEServer(devSpacesContext)
             val remoteIdeServerStatus = remoteIdeServer.getStatus()
             val joinLink = remoteIdeServerStatus.joinLink
                 ?: throw IOException("Could not connect, remote IDE is not ready. No join link present.")
@@ -72,7 +68,7 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) {
 
             val effectiveJoinLink = joinLink.replace(":5990", ":$localPort")
 
-            client = LinkedClientManager
+            val client = LinkedClientManager
                 .getInstance()
                 .startNewClient(
                     Lifetime.Eternal,
@@ -82,101 +78,40 @@ class DevSpacesConnection(private val devSpacesContext: DevSpacesContext) {
                     false
                 )
 
-            client.run {
-                lifetime.onTermination {
-                    try {
-                        forwarder.close()
-                    } catch (_: Exception) {
-                        // ignore cleanup errors
-                    }
-                }
-
-                lifetime.onTermination {
-                    try {
-                        if (remoteIdeServer.waitServerTerminated()) {
-                            DevWorkspaces(devSpacesContext.client)
-                                .stop(
-                                    devSpacesContext.devWorkspace.namespace,
-                                    devSpacesContext.devWorkspace.name
-                                )
-                            onDevWorkspaceStopped() // UI refresh through callback
-                        }
-                    } finally {
-                        synchronized(devSpacesContext.activeWorkspaces) {
-                            devSpacesContext.activeWorkspaces.remove(workspace)
-                        }
-                        onDisconnected()
-                    }
-                }
-
-                lifetime.onTermination {
-                    onDisconnected() //  UI refresh through callback
-                }
+            client.clientClosed.advise(client.lifetime) {
+                onClientClosed(onDisconnected , onDevWorkspaceStopped, remoteIdeServer, forwarder)
             }
 
-            return client
+            client
         } catch (e: Exception) {
-            try {
-                disconnectAndCleanup(client, forwarder, onDevWorkspaceStopped, onDisconnected) // Cancel if started
-            } catch (_: Exception) {}
-
-            try {
-                forwarder?.close()
-            } catch (_: Exception) {}
-
-            synchronized(devSpacesContext.activeWorkspaces) {
-                devSpacesContext.activeWorkspaces.remove(workspace)
-            }
-
-            // Make sure UI refresh still happens on failure
-            onDisconnected()
-
+            onClientClosed(onDisconnected, onDevWorkspaceStopped, remoteIdeServer, forwarder)
             throw e
         }
     }
 
-    private fun disconnectAndCleanup(
-        client: ThinClientHandle?,
-        forwarder: AutoCloseable?,
+    private fun onClientClosed(
+        onDisconnected: () -> Unit,
         onDevWorkspaceStopped: () -> Unit,
-        onDisconnected: () -> Unit
+        remoteIdeServer: RemoteIDEServer?,
+        forwarder: Closeable?
     ) {
-        if (client == null) {
-            onDisconnected()
-            return
-        }
-
-        try {
-            // Close the port forwarder first
+        CoroutineScope(Dispatchers.IO).launch {
+            val currentWorkspace = devSpacesContext.devWorkspace
             try {
+                onDisconnected.invoke()
+                if (true == remoteIdeServer?.waitServerTerminated()) {
+                    DevWorkspaces(devSpacesContext.client)
+                        .stop(
+                            devSpacesContext.devWorkspace.namespace,
+                            devSpacesContext.devWorkspace.name
+                        )
+                        .also { onDevWorkspaceStopped() }
+                }
                 forwarder?.close()
-            } catch (e: Exception) {
-                thisLogger().debug("Failed to close port forwarder: ${e.message}")
-            }
-
-            // Stop workspace cleanly
-            val devWorkspaces = DevWorkspaces(devSpacesContext.client)
-            val workspace = devSpacesContext.devWorkspace
-
-            try {
-                devWorkspaces.stop(workspace.namespace, workspace.name)
-                onDevWorkspaceStopped()
-            } catch (e: Exception) {
-                thisLogger().debug("Workspace stop failed: ${e.message}")
-            }
-
-            // Remove from active list and update state
-            synchronized(devSpacesContext.activeWorkspaces) {
-                devSpacesContext.activeWorkspaces.remove(workspace)
-            }
-            onDisconnected()
-
-        } catch (e: Exception) {
-            thisLogger().debug("Error while terminating client: ${e.message}")
-            synchronized(devSpacesContext.activeWorkspaces) {
-                devSpacesContext.activeWorkspaces.remove(devSpacesContext.devWorkspace)
+            } finally {
+                devSpacesContext.removeWorkspace(currentWorkspace)
+                onDisconnected()
             }
-            onDisconnected()
         }
     }
 
diff --git a/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesContext.kt b/src/main/kotlin/com/redhat/devtools/gateway/DevSpacesContext.kt
@@ -17,5 +17,21 @@ import io.kubernetes.client.openapi.ApiClient
 class DevSpacesContext {
     lateinit var client: ApiClient
     lateinit var devWorkspace: DevWorkspace
-    var activeWorkspaces = mutableSetOf<DevWorkspace>()  // Global or companion-level variable
+    var activeWorkspaces = mutableSetOf<DevWorkspace>()
+
+    fun addWorkspace(workspace: DevWorkspace) {
+        synchronized(activeWorkspaces) {
+            if (activeWorkspaces.contains(workspace)) {
+                return
+            }
+            activeWorkspaces.add(workspace)
+        }
+    }
+
+    fun removeWorkspace(currentWorkspace: DevWorkspace) {
+        synchronized(activeWorkspaces) {
+            activeWorkspaces.remove(currentWorkspace)
+        }
+    }
+
 }
diff --git a/src/main/kotlin/com/redhat/devtools/gateway/openshift/Pods.kt b/src/main/kotlin/com/redhat/devtools/gateway/openshift/Pods.kt
@@ -170,6 +170,7 @@ class Pods(private val client: ApiClient) {
                     logger.info("Attempt #${attempt + 1}: Connecting $localPort -> $remotePort...")
                     val portForward = PortForward(client)
                     forwardResult = portForward.forward(pod, listOf(remotePort))
+                    logger.info("forward successful: $localPort -> $remotePort...")
                     copyStreams(clientSocket, forwardResult, remotePort)
                     return
                 } catch (e: Exception) {
diff --git a/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt b/src/main/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServer.kt
@@ -19,8 +19,10 @@ import com.redhat.devtools.gateway.DevSpacesContext
 import com.redhat.devtools.gateway.openshift.Pods
 import io.kubernetes.client.openapi.models.V1Container
 import io.kubernetes.client.openapi.models.V1Pod
-import org.bouncycastle.util.Arrays
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.withTimeoutOrNull
 import java.io.IOException
+import kotlin.time.Duration.Companion.seconds
 
 /**
  * Represent an IDE server running in a CDE.
@@ -68,8 +70,8 @@ class RemoteIDEServer(private val devSpacesContext: DevSpacesContext) {
     }
 
     @Throws(IOException::class)
-    fun waitServerReady() {
-        doWaitServerState(true)
+    suspend fun waitServerReady() {
+        doWaitServerProjectExists(true)
             .also {
                 if (!it) throw IOException(
                     "Remote IDE server is not ready after $readyTimeout seconds.",
@@ -78,19 +80,39 @@ class RemoteIDEServer(private val devSpacesContext: DevSpacesContext) {
     }
 
     @Throws(IOException::class)
-    fun waitServerTerminated(): Boolean {
-        return doWaitServerState(false)
+    suspend fun waitServerTerminated(): Boolean {
+        return doWaitServerProjectExists(false)
     }
 
-    @Throws(IOException::class)
-    fun doWaitServerState(isReadyState: Boolean): Boolean {
-        return try {
-            val status = getStatus()
-            isReadyState == !Arrays.isNullOrEmpty(status.projects)
-        } catch (e: Exception) {
-            thisLogger().debug("Failed to check remote IDE server state.", e)
-            false
-        }
+    /**
+     * Waits for the server to have or not have projects according to the given parameter.
+     * Times out the wait if the expected state is not reached within 10 seconds.
+     *
+     * @param expected True if projects are expected, False otherwise,
+     * @return True if the expected state is achieved within the timeout, False otherwise.
+     */
+    private suspend fun doWaitServerProjectExists(expected: Boolean): Boolean {
+        val timeout = 10.seconds
+
+        return withTimeoutOrNull(timeout) {
+            while (true) {
+                val hasProjects = try {
+                    val status = getStatus()
+                    status.projects.isNotEmpty()
+                } catch (e: Exception) {
+                    thisLogger().debug("Failed to check remote IDE server state.", e)
+                    null
+                }
+
+                if (expected == hasProjects) {
+                    return@withTimeoutOrNull true
+                }
+
+                delay(500L)
+            }
+            @Suppress("UNREACHABLE_CODE")
+            null
+        } ?: false
     }
 
     @Throws(IOException::class)
diff --git a/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt b/src/test/kotlin/com/redhat/devtools/gateway/server/RemoteIDEServerTest.kt
