Skip to content

[JetBrains] Improve ports listener leaking #20872

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

Closed
wants to merge 4 commits into from
Closed
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 @@ -38,6 +38,7 @@ project(":") {
if (properties("platformType") == "RD") {
print("Rider: exclude unnecessary files")
sourceSets["main"].kotlin.exclude("**/GitpodForceUpdateMavenProjectsActivity.kt")
sourceSets["main"].kotlin.exclude("**/GitpodPortForwardingServiceImpl.kt")
sourceSets["main"].kotlin.exclude("**/maven.xml")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ import io.grpc.stub.ClientCallStreamObserver
import io.grpc.stub.ClientResponseObserver
import kotlinx.coroutines.*
import kotlinx.coroutines.future.asDeferred
import kotlinx.coroutines.flow.MutableSharedFlow
import org.apache.http.client.utils.URIBuilder
import java.util.*
import java.util.concurrent.CompletableFuture
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext

@Suppress("UnstableApiUsage")
abstract class AbstractGitpodPortForwardingService : GitpodPortForwardingService {
Expand All @@ -34,8 +37,26 @@ abstract class AbstractGitpodPortForwardingService : GitpodPortForwardingService
private val perClientPortForwardingManager = service<PerClientPortForwardingManager>()
private val ignoredPortsForNotificationService = service<GitpodIgnoredPortsForNotificationService>()
private val lifetime = Lifetime.Eternal.createNested()
private val portStatusFlow = MutableSharedFlow<Status.PortsStatusResponse>()
Copy link
Preview

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

The default MutableSharedFlow has no buffer and will suspend emit until a collector is ready. Consider configuring an extraBufferCapacity or using MutableSharedFlow(replay=0, extraBufferCapacity=N) to avoid potential deadlocks if bursts of events arrive before collection.

Suggested change
private val portStatusFlow = MutableSharedFlow<Status.PortsStatusResponse>()
private val portStatusFlow = MutableSharedFlow<Status.PortsStatusResponse>(replay = 0, extraBufferCapacity = 64)

Copilot uses AI. Check for mistakes.


init {
// Start collecting port status updates
runJob(lifetime) {
portStatusFlow
.let { flow -> applyThrottling(flow) }
.collect { response ->
withContext(Dispatchers.IO) {
syncPortsListWithClient(response)
}
}
}

init { start() }
start()
}

protected abstract fun runJob(lifetime: Lifetime, block: suspend CoroutineScope.() -> Unit): Job;

protected abstract suspend fun <T> applyThrottling(flow: kotlinx.coroutines.flow.Flow<T>): kotlinx.coroutines.flow.Flow<T>

private fun start() {
if (application.isHeadlessEnvironment) return
Expand All @@ -47,8 +68,6 @@ abstract class AbstractGitpodPortForwardingService : GitpodPortForwardingService
observePortsListWhileProjectIsOpen()
}

protected abstract fun runJob(lifetime: Lifetime, block: suspend CoroutineScope.() -> Unit): Job;

private fun observePortsListWhileProjectIsOpen() = runJob(lifetime) {
while (isActive) {
try {
Expand Down Expand Up @@ -86,7 +105,11 @@ abstract class AbstractGitpodPortForwardingService : GitpodPortForwardingService
}

override fun onNext(response: Status.PortsStatusResponse) {
application.invokeLater { syncPortsListWithClient(response) }
application.invokeLater {
runJob(lifetime) {
portStatusFlow.emit(response)
}
}
}

override fun onCompleted() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ package io.gitpod.jetbrains.remote

import com.jetbrains.rd.util.lifetime.Lifetime
import com.jetbrains.rd.util.threading.coroutines.launch
import io.gitpod.jetbrains.remote.AbstractGitpodPortForwardingService
import kotlinx.coroutines.CoroutineScope
import fleet.util.async.throttleLatest
import kotlinx.coroutines.flow.Flow

@Suppress("UnstableApiUsage")
class GitpodPortForwardingServiceImpl : AbstractGitpodPortForwardingService() {
override fun runJob(lifetime: Lifetime, block: suspend CoroutineScope.() -> Unit) = lifetime.launch { block() }

override suspend fun <T> applyThrottling(flow: Flow<T>): Flow<T> = flow.throttleLatest(1000)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) 2025 Gitpod GmbH. All rights reserved.
// Licensed under the GNU Affero General Public License (AGPL).
// See License.AGPL.txt in the project root for license information.

package io.gitpod.jetbrains.remote.optional

import com.jetbrains.rd.util.lifetime.Lifetime
import com.jetbrains.rd.util.threading.coroutines.launch
import io.gitpod.jetbrains.remote.AbstractGitpodPortForwardingService
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow

@Suppress("UnstableApiUsage")
class GitpodRiderPortForwardingService : AbstractGitpodPortForwardingService() {
override fun runJob(lifetime: Lifetime, block: suspend CoroutineScope.() -> Unit) = lifetime.launch { block() }

override suspend fun <T> applyThrottling(flow: Flow<T>): Flow<T> = flow
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,8 @@
<!--suppress PluginXmlValidity -->
<idea-plugin>
<extensions defaultExtensionNs="com.intellij">
<applicationService serviceInterface="io.gitpod.jetbrains.remote.GitpodPortForwardingService"
serviceImplementation="io.gitpod.jetbrains.remote.GitpodPortForwardingServiceImpl"
client="controller" preload="true"/>
</extensions>
</idea-plugin>
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
description="Disable the forced update of Maven projects when IDE opens."
restartRequired="true"/>

<applicationService serviceInterface="io.gitpod.jetbrains.remote.GitpodPortForwardingService"
serviceImplementation="io.gitpod.jetbrains.remote.optional.GitpodRiderPortForwardingService"
client="controller" preload="true" />

<applicationService serviceInterface="io.gitpod.jetbrains.remote.GitpodIgnoredPortsForNotificationService"
serviceImplementation="io.gitpod.jetbrains.remote.internal.GitpodIgnoredPortsForNotificationServiceImpl"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,8 @@
<!--suppress PluginXmlValidity -->
<idea-plugin>
<extensions defaultExtensionNs="com.intellij">
<applicationService serviceInterface="io.gitpod.jetbrains.remote.GitpodPortForwardingService"
serviceImplementation="io.gitpod.jetbrains.remote.GitpodPortForwardingServiceImpl"
client="controller" preload="true"/>
</extensions>
</idea-plugin>
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@
preload="true"/>


<applicationService serviceInterface="io.gitpod.jetbrains.remote.GitpodPortForwardingService"
serviceImplementation="io.gitpod.jetbrains.remote.GitpodPortForwardingServiceImpl"
client="controller" preload="true"/>
<gateway.customization.performance id="gitpodMetricsControl" order="before cpuControl"
implementation="io.gitpod.jetbrains.remote.GitpodMetricControlProvider"/>
</extensions>
Expand Down
7 changes: 7 additions & 0 deletions dev/preview/workflow/preview/patch-ide-configmap.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ function replaceImage(image) {
const imagesBuildFromMain = [
"jb-backend-plugin:commit-2d67254d5aa110bc2c76cd807b85b272e3d54d97-latest",
"jb-backend-plugin:commit-4c69ad0670cc4cfbf43910e1db700ad90acd5ac6",
"jb-backend-plugin:commit-8cdc2a1a45c66ac9e003a97af8c57fad42d923f6-rider",
"jb-backend-plugin:commit-8cdc2a1a45c66ac9e003a97af8c57fad42d923f6-rider-latest",
];

// TODO(hw): remove me
Expand Down Expand Up @@ -43,6 +45,11 @@ for (let ide in json.ideOptions.options) {
json.ideOptions.options[ide].imageLayers = json.ideOptions.options[ide].imageLayers.map(replaceImage2);
}

if (["rider"].includes(ide)) {
Copy link
Preview

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

[nitpick] Using an array for a single value is overkill. You can simplify this to if (ide === "rider") for clarity.

Suggested change
if (["rider"].includes(ide)) {
if (ide === "rider") {

Copilot uses AI. Check for mistakes.

json.ideOptions.options[ide].pluginImage = replaceImage2(json.ideOptions.options[ide].pluginImage);
json.ideOptions.options[ide].imageLayers = json.ideOptions.options[ide].imageLayers.map(replaceImage2);
}

if (["code", "code1_85"].includes(ide)) {
json.ideOptions.options[ide].image = replaceImage(json.ideOptions.options[ide].image);
json.ideOptions.options[ide].imageLayers = json.ideOptions.options[ide].imageLayers.map(replaceImage);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,12 @@ func GenerateIDEConfigmap(ctx *common.RenderContext) (*ide_config.IDEConfig, err
CodeHelperImage: ctx.ImageName(ctx.Config.Repository, ide.CodeHelperIDEImage, ctx.VersionManifest.Components.Workspace.CodeHelperImage.Version),
CodeWebExtensionImage: ctx.ImageName(ctx.Config.Repository, ide.CodeWebExtensionImage, ctx.VersionManifest.Components.Workspace.CodeWebExtensionImage.Version),

JetBrainsPluginImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, ctx.VersionManifest.Components.Workspace.DesktopIdeImages.JetBrainsBackendPluginImage.Version),
JetBrainsPluginLatestImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, ctx.VersionManifest.Components.Workspace.DesktopIdeImages.JetBrainsBackendPluginLatestImage.Version),
JetBrainsPluginRiderImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, ctx.VersionManifest.Components.Workspace.DesktopIdeImages.JetBrainsBackendPluginRiderImage.Version),
JetBrainsPluginLatestRiderImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, ctx.VersionManifest.Components.Workspace.DesktopIdeImages.JetBrainsBackendPluginLatestRiderImage.Version),
JetBrainsPluginImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, ctx.VersionManifest.Components.Workspace.DesktopIdeImages.JetBrainsBackendPluginImage.Version),
JetBrainsPluginLatestImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, ctx.VersionManifest.Components.Workspace.DesktopIdeImages.JetBrainsBackendPluginLatestImage.Version),

// Pin Rider's backend-plugin version as we don't plan to upgrade Rider yet
JetBrainsPluginRiderImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, "commit-8cdc2a1a45c66ac9e003a97af8c57fad42d923f6-rider"),
JetBrainsPluginLatestRiderImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, "commit-8cdc2a1a45c66ac9e003a97af8c57fad42d923f6-rider-latest"),
Comment on lines +82 to +83
Copy link
Preview

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

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

[nitpick] This hardcoded commit string acts as a magic value. Consider extracting it into a named constant or configuration property to make future updates clearer.

Suggested change
JetBrainsPluginRiderImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, "commit-8cdc2a1a45c66ac9e003a97af8c57fad42d923f6-rider"),
JetBrainsPluginLatestRiderImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, "commit-8cdc2a1a45c66ac9e003a97af8c57fad42d923f6-rider-latest"),
JetBrainsPluginRiderImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, RiderCommit),
JetBrainsPluginLatestRiderImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsBackendPluginImage, RiderLatestCommit),

Copilot uses AI. Check for mistakes.

JetBrainsLauncherImage: ctx.ImageName(ctx.Config.Repository, ide.JetBrainsLauncherImage, ctx.VersionManifest.Components.Workspace.DesktopIdeImages.JetBrainsLauncherImage.Version),
ResolvedJBImageLatest: JBImages{
IntelliJ: resolveLatestImage(ide.IntelliJDesktopIDEImage, "latest", ctx.VersionManifest.Components.Workspace.DesktopIdeImages.IntelliJLatestImage),
Expand Down
Loading