fix: kill stale wine processes before launch #1195

coderabbitai · 2026-04-13T05:39:49Z

⚠️ Potential issue | 🟠 Major

Back press is fully swallowed during loading with no fallback.

At Line 1100, Back is intercepted; at Lines 1101-1102, the empty handler creates a dead-end if prelaunch/loading hangs. Please add either a safe cancel path or a timeout fallback that restores normal back behavior.

I can draft a small cancellable prelaunch pattern (job + cancel signal + UI fallback) if you want me to open a follow-up issue proposal.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/PluviaMain.kt` around lines 1100 - 1102, Back press is currently swallowed by BackHandler when state.loadingDialogVisible is true and SteamService.keepAlive is false; implement a safe cancel/timeout fallback so Back can restore normal behavior: add a cancellable prelaunch signal (e.g., a MutableState or Job started when state.loadingDialogVisible becomes true via LaunchedEffect or rememberCoroutineScope) and in the BackHandler callback cancel that job and set state.loadingDialogVisible = false (or call your prelaunch cancellation routine), and also launch a timeout coroutine (e.g., 30s) when showing the dialog that will clear state.loadingDialogVisible if prelaunch doesn't complete; reference BackHandler, state.loadingDialogVisible and SteamService.keepAlive when locating where to wire the cancel() call and timeout.

That's the idea for this scope, it's effectively same in master right now yeah? Animation persists even if you can supposedly back out, procs stick and you need force quit because of it anyway. This just mitigates impatient back mash on loading screen till a separate PR does proper retreat and cleanup

@xXJSONDeruloXx, that's a fair point — if master already reaches a force-quit situation on back-during-loading (stale procs keep the animation alive regardless), then blocking the back key here is at least an honest no-op instead of a misleading one. The BackHandler prevents the UI thrash without making the underlying problem any worse.

I'll note this for future reviews and happy to open a follow-up issue for the proper cancellable-prelaunch work whenever you're ready.

✏️ Learnings added

Learnt from: xXJSONDeruloXx Repo: utkarshdalal/GameNative PR: 1195 File: app/src/main/java/app/gamenative/ui/PluviaMain.kt:1100-1102 Timestamp: 2026-04-13T05:42:47.571Z Learning: In `app/src/main/java/app/gamenative/ui/PluviaMain.kt`, the `BackHandler(enabled = state.loadingDialogVisible && !SteamService.keepAlive)` with an empty body is intentionally a temporary mitigation. On master, pressing Back during the loading/prelaunch phase already leads to a broken state (animation persists, Wine procs stick, force-quit required), so swallowing Back is no worse and prevents UI thrash from impatient back-mashing. Proper cancellable prelaunch (job cancellation + state reset + timeout fallback) is deferred to a separate PR. Do not flag the empty BackHandler body as a dead-end in future reviews of this file.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 0 File: :0-0 Timestamp: 2026-03-18T16:30:26.610Z Learning: In `app/src/main/java/app/gamenative/ui/PluviaMain.kt`, the three sites that call `MainActivity.setPendingLaunchRequest`, toggle `shownPendingLaunchSnackbar`, and conditionally call `SnackbarManager.show` (cold-start pending branch, ExternalGameLaunch pending branch, and non-terminal SteamDisconnected branch) are intentionally kept separate — the surrounding logic differs enough that extracting a shared helper would add more complexity than it removes. Do not suggest refactoring these into a helper in future reviews.

Learnt from: utkarshdalal Repo: utkarshdalal/GameNative PR: 171 File: app/src/main/java/app/gamenative/MainActivity.kt:283-288 Timestamp: 2025-09-25T10:56:46.337Z Learning: In MainActivity.kt, the back key handling in dispatchKeyEvent only checks for KeyEvent.KEYCODE_BACK (not KEYCODE_BUTTON_B) because it's specifically designed to intercept the device's physical back button when a game is running, not controller input.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 1136 File: app/src/main/java/app/gamenative/MainActivity.kt:147-150 Timestamp: 2026-04-07T21:37:29.365Z Learning: In `app/src/main/java/app/gamenative/MainActivity.kt`, the `onCreate` stale-session check `if (SteamService.keepAlive && PluviaApp.xEnvironment == null)` is intentionally kept as-is. The only non-game flow that sets `keepAlive = true` with `xEnvironment == null` is the folder-picker in `ContainerConfigDialog.kt` (line ~1032); if the app is killed in that state, resetting `keepAlive = false` on the next `onCreate` is harmless. Do not flag this guard as incorrectly matching non-XServer flows in future reviews.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 0 File: :0-0 Timestamp: 2026-04-08T06:26:20.804Z Learning: In `app/src/main/java/app/gamenative/PluviaApp.kt`, `shutdownEnvironment()` intentionally does **not** call `winHandler?.stop()`. On a task-swipe / `onDestroy` path the OS reclaims the socket naturally, so stopping `WinHandler` there is unnecessary. The explicit `winHandler?.stop()` call before `PluviaApp.shutdownEnvironment()` in `XServerScreen.exit()` covers the graceful-exit case where it actually matters. Do not flag the absence of a `WinHandler` stop inside `shutdownEnvironment()` in future reviews.

Learnt from: unbelievableflavour Repo: utkarshdalal/GameNative PR: 810 File: app/src/main/java/app/gamenative/ui/screen/library/appscreen/CustomGameAppScreen.kt:391-417 Timestamp: 2026-03-18T08:52:17.299Z Learning: In the `app/src/main/java/app/gamenative/ui/screen/library/appscreen/` AppScreen files (e.g., `BaseAppScreen.kt`, `CustomGameAppScreen.kt`), using `CoroutineScope(Dispatchers.IO).launch` inside `AppMenuOption` click handlers (such as for `FetchSteamGridDBImages` and `CreateShortcut`) is an established, intentional pattern. Do not flag this as a lifecycle-safety issue in future reviews of these files.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 1136 File: app/src/main/java/app/gamenative/MainActivity.kt:273-284 Timestamp: 2026-04-07T21:36:14.886Z Learning: In the GameNative repository (`utkarshdalal/GameNative`), `SteamService.keepAlive` is set to `true` in only one place: `ContainerConfigDialog.kt:1032` (inside a folder-picker flow). During the normal XServerScreen game-launch bootstrap (`XServerScreen.kt` lines ~1529-1680), `keepAlive` is `false` throughout. The bootstrap uses a raw Java `Executors.newSingleThreadExecutor().submit {}` (not a coroutine/LaunchedEffect), so Compose teardown does not cancel it. However, since `keepAlive` is `false` during bootstrap, a task-swipe in that window does not produce a stale `keepAlive=true` on the next launch, and therefore does not cause the "stuck on container screen" bug. Do not flag onDestroy's `if (SteamService.keepAlive)` guard as failing to cover the bootstrap-window race in future reviews.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 0 File: :0-0 Timestamp: 2026-03-10T11:01:21.928Z Learning: In `app/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt` (around lines 371-396), the `Loading` parental state in `SteamService.parentalControlState` intentionally maps to `PrefManager.parentalAllowedAppIds` (which can be null) rather than `emptyList()`. This fail-open behavior on the `Loading` state is by design: on first launch/login the account type (managed child vs. non-child) is unknown, so showing all apps is preferable to incorrectly blocking everything. Subsequent sessions use the PrefManager cache, so restrictions are enforced correctly after the first login. Do not flag this as a security/correctness issue in future reviews.

Learnt from: unbelievableflavour Repo: utkarshdalal/GameNative PR: 811 File: app/src/main/java/app/gamenative/ui/util/WindowSize.kt:38-44 Timestamp: 2026-03-11T11:17:48.473Z Learning: In `app/src/main/java/app/gamenative/ui/util/WindowSize.kt`, `shouldShowGamepadUI()` reads `PrefManager.showGamepadHints` as a plain (non-reactive) property. This is intentional: when the user toggles the setting in the Settings screen and navigates back, the composable recomposes and picks up the updated value. Real-time reactivity (e.g., via StateFlow/collectAsState) is not required here. Do not flag this as an issue in future reviews.

Learnt from: unbelievableflavour Repo: utkarshdalal/GameNative PR: 698 File: app/src/main/java/app/gamenative/utils/LaunchSteps.kt:69-72 Timestamp: 2026-03-03T11:47:17.167Z Learning: In the LaunchSteps orchestration (app/src/main/java/app/gamenative/utils/LaunchSteps.kt), runOnce steps are intentionally marked as done regardless of their exit status. This is because some steps like GOGScriptInterpreter can return an error while still having completed necessary work, and the step should not run again on subsequent launches.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 918 File: app/src/main/java/app/gamenative/service/SteamService.kt:3051-3058 Timestamp: 2026-03-19T10:24:08.165Z Learning: In `app/src/main/java/app/gamenative/service/SteamService.kt`, `connectToSteam()` uses `scope.launch(Dispatchers.Default)` deliberately. This is a pre-existing, intentional dispatcher choice. Do not flag or suggest changing `Dispatchers.Default` to another dispatcher in `connectToSteam()` in future reviews.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 0 File: :0-0 Timestamp: 2026-03-18T16:30:26.610Z Learning: In `app/src/main/java/app/gamenative/MainActivity.kt`, `peekPendingLaunchRequest()` is an intentional non-destructive, synchronized read of `pendingLaunchRequest`. The peek + separate consume pattern (also present at line 436) is pre-existing and accepted; the worst-case outcome of a race is an unnecessary snackbar, which is benign. Do not flag `peekPendingLaunchRequest()` as a TOCTOU race issue in future reviews.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 1146 File: app/src/main/java/app/gamenative/service/SteamAutoCloud.kt:832-840 Timestamp: 2026-04-08T12:05:56.079Z Learning: In `app/src/main/java/app/gamenative/service/SteamAutoCloud.kt`, the `isCacheCleared` branch (around lines 832-840) intentionally reads and removes `appInfo.id` from `PrefManager.pendingCloudResync` at that point — before conflict resolution — because the presence/absence of the ID determines which dialog wording to display (upgrade-specific `conflictUfsVersion` vs generic local-saves-only text). This classification must happen pre-dialog. Do not flag this as needing to be deferred until after the resync succeeds in future reviews.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 965 File: app/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt:262-276 Timestamp: 2026-03-22T00:52:52.382Z Learning: In `app/src/main/java/app/gamenative/ui/model/LibraryViewModel.kt`, `saveScrollAnchor` has an intentional early-return guard: `if (visIdx == 0 && !existingAnchor.isNullOrEmpty() && existingAnchor != appId && !state.scrollAnchorRestored) return` This protects against a ViewModel-recreation race: a real anchor (e.g. idx=32) is saved → ViewModel recreates → ON_PAUSE fires while the grid is still at idx=0 (before scroll restoration) → without the guard the anchor would be overwritten with position 0 and lost on the next recreation. Once `scrollAnchorRestored` becomes `true` (after the composable confirms the scroll completed), the guard is inactive and position 0 is a legitimate value to persist. Do not flag this guard or suggest clearing the anchor on `visIdx==0` in future reviews.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 1092 File: app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt:897-906 Timestamp: 2026-04-03T12:55:12.164Z Learning: In `app/src/main/java/app/gamenative/service/epic/EpicDownloadManager.kt`, the cancellation check (`downloadInfo.isActive()`) is only observed between chunk batches in `downloadAndAssembleEpicChunks`. The `downloadChunkWithRetry` and `downloadChunk` functions do not check for cancellation internally. This is a pre-existing pattern, intentional and out of scope for new feature PRs. Do not flag mid-batch cancellation latency as an issue in future reviews of this file.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 918 File: app/src/main/java/app/gamenative/service/SteamService.kt:3091-3102 Timestamp: 2026-03-19T10:03:51.598Z Learning: In `app/src/main/java/app/gamenative/service/SteamService.kt`, `connectToSteam()` uses a `retryAttemptAtStart` snapshot to guard the 5s watchdog: after the delay, the watchdog only marks the endpoint bad and disconnects if `!isConnected && retryAttempt == retryAttemptAtStart`. Since `onConnected` resets `retryAttempt = 0` and `onDisconnected` increments `retryAttempt++`, any callback activity during the 5s window changes `retryAttempt`, causing the watchdog to back off. This is the intentional, correct design. Do not flag the `delay(5000)` + `retryAttempt` guard pattern as a race condition in future reviews.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 801 File: app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt:174-188 Timestamp: 2026-03-10T22:12:03.164Z Learning: In `app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt`, the `applyConfigForContainer` function calls `installMissingComponentsForConfig` before `BestConfigService.parseConfigToContainerData`. This install-before-parse ordering is pre-existing behavior, not introduced by any recent PR. Do not flag this ordering as an issue in future reviews of this file.

Learnt from: xXJSONDeruloXx Repo: utkarshdalal/GameNative PR: 1195 File: app/src/main/java/com/winlator/core/ProcessHelper.java:50-53 Timestamp: 2026-04-13T00:51:50.454Z Learning: In `app/src/main/java/com/winlator/core/ProcessHelper.java`, `killAllWineProcesses()` deliberately uses the broad `listRunningWineProcesses()` matcher (substring "wine" or "exe" in /proc/<pid>/stat). This is intentionally a "hammer" approach called at the top of `setupXEnvironment()` before any new Wine processes start. It is safe on Android because `Process.sendSignal()` is subject to UID-based kernel enforcement — the app can only kill its own sandbox UID processes — and within that UID the only native procs are Wine-related. Do not flag this broad matcher as a cross-process safety risk in future reviews.

Learnt from: unbelievableflavour Repo: utkarshdalal/GameNative PR: 698 File: app/src/main/java/app/gamenative/utils/LaunchSteps.kt:134-142 Timestamp: 2026-03-03T11:45:46.529Z Learning: In the LaunchSteps orchestration (app/src/main/java/app/gamenative/utils/LaunchSteps.kt), the `wineserver -k` command in the runLauncher function is necessary to prevent the launcher from closing after each chained step. Without it, the launcher terminates prematurely between steps.

Learnt from: phobos665 Repo: utkarshdalal/GameNative PR: 1072 File: app/src/main/java/app/gamenative/ui/screen/library/appscreen/SteamAppScreen.kt:1309-1315 Timestamp: 2026-04-02T07:43:14.973Z Learning: In the GameNative repository, when logging/telemetry includes Wine prefix/container paths that contain the hardcoded Wine username `xuser`, do not treat that as a PII/security issue (it is not a real user identity and contains no personally identifiable information). Only flag PII concerns for other user-identifying data not limited to the `xuser` hardcoded Wine username in prefix paths.

Learnt from: xXJSONDeruloXx Repo: utkarshdalal/GameNative PR: 1076 File: app/src/main/java/app/gamenative/service/gog/GOGCometManager.kt:57-61 Timestamp: 2026-04-06T13:16:11.272Z Learning: In the GameNative repository, GOG Wine containers follow a one-game-per-container invariant. Because a single container is always dedicated to exactly one GOG game, there is no risk of concurrent writes to a fixed per-container batch file (e.g., `launch_gog_with_comet.bat` in `GOGCometManager.kt`). Do not flag the reuse of a fixed launcher batch file path per container as a race-condition risk in future reviews.

Learnt from: jeremybernstein Repo: utkarshdalal/GameNative PR: 581 File: app/src/main/java/app/gamenative/ui/PluviaMain.kt:1154-1176 Timestamp: 2026-02-21T22:26:41.871Z Learning: In Kotlin coroutines, suspend functions automatically suspend and resume within a coroutine scope. Only Deferred objects produced by async { ... } require .await() to obtain the result. Do not call .await() on direct suspend function calls or on non-Deferred results; use await() only when you have a Deferred that represents a value you need.

in some situations, installing mono runs indefinitely. This was happening on some box64/fex versions. I guess users can force stop or swipe the app away if that happens?

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

@utkarshdalal yeah swipe away like they do now, just with this pr they can now get a clean slate when re entering. Back button still wouldn't dismiss loading animation for boot but would retreat ui in the background. This keeps ux same but unintended soft lock till force kill is solved

-Original file line number
+Diff line change
@@ Expand Up / @@ -2,6 +2,7 @@ package app.gamenative.ui @@
     import android.content.Context
     import android.content.Intent
+    import androidx.activity.compose.BackHandler
     import androidx.compose.foundation.isSystemInDarkTheme
     import androidx.compose.foundation.layout.Box
     import androidx.compose.foundation.layout.WindowInsets
@@ Expand Down Expand Up / @@ -1096,6 +1097,10 @@ fun PluviaMain( @@
             }
         }
+        BackHandler(enabled = state.loadingDialogVisible && !SteamService.keepAlive) {
+            // TODO: Make prelaunch/loading operations cancellable so Back can exit safely.
+        }
         PluviaTheme(
             isDark = when (state.appTheme) {
                 AppTheme.AUTO -> isSystemInDarkTheme()
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -2533,6 +2533,43 @@ private fun shiftXEnvironmentToContext( @@
         return environment
     }
+    private fun hardKillStaleWineProcessesBeforeLaunch() {
+        val deadlineMs = System.currentTimeMillis() + 5_000
+        val staleWinePids = ProcessHelper.listRunningWineProcesses()
+            .mapNotNull { it.toIntOrNull() }
+            .distinct()
+        if (staleWinePids.isEmpty()) {
+            return
+        }
+        Timber.w(
+            "Found %d stale Wine process(es) before launch; hard-killing them: %s",
+            staleWinePids.size,
+            staleWinePids.joinToString(),
+        )
+        ProcessHelper.killAllWineProcesses()
+        var remainingWinePids: List<Int>
+        do {
+            Thread.sleep(100)
+            remainingWinePids = ProcessHelper.listRunningWineProcesses()
+                .mapNotNull { it.toIntOrNull() }
+                .distinct()
+        } while (remainingWinePids.isNotEmpty() && System.currentTimeMillis() < deadlineMs)
+        if (remainingWinePids.isNotEmpty()) {
+            Timber.w(
+                "Wine processes still present after hard-kill attempt: %s",
+                remainingWinePids.joinToString(),
+            )
+            throw IllegalStateException(
+                "Wine processes still present after hard-kill attempt: ${remainingWinePids.joinToString()}"
+            )
+        }
+    }
     private fun setupXEnvironment(
         context: Context,
         appId: String,
@@ Expand All / @@ -2550,6 +2587,8 @@ private fun setupXEnvironment( @@
         onGameLaunchError: ((String) -> Unit)? = null,
         navigateBack: () -> Unit,
     ): XEnvironment {
+        hardKillStaleWineProcessesBeforeLaunch()
         val gameSource = ContainerUtils.extractGameSourceFromContainerId(appId)
         val lc_all = container!!.lC_ALL
         val imageFs = ImageFs.find(context)
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -37,12 +37,22 @@ public static void terminateProcess(int pid) { @@
     //        Log.d("ProcessHelper", "Process terminated with pid: " + pid);
         }
+        public static void killProcess(int pid) {
+            Process.sendSignal(pid, SIGKILL);
+        }
         public static void terminateAllWineProcesses() {
             for (String process : listRunningWineProcesses()) {
                 terminateProcess(Integer.parseInt(process));
             }
         }
+        public static void killAllWineProcesses() {
+            for (String process : listRunningWineProcesses()) {
+                killProcess(Integer.parseInt(process));
+            }
+        }
         public static void pauseAllWineProcesses() {
             for (String process : listRunningWineProcesses()) {
                 suspendProcess(Integer.parseInt(process));
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: kill stale wine processes before launch #1195

Diff view

Diff view

There are no files selected for viewing

coderabbitai bot Apr 13, 2026 •

edited

Loading

Uh oh!

xXJSONDeruloXx Apr 13, 2026

Uh oh!

coderabbitai bot Apr 13, 2026

Uh oh!

utkarshdalal Apr 13, 2026

Uh oh!

coderabbitai bot Apr 13, 2026

Uh oh!

xXJSONDeruloXx Apr 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fix: kill stale wine processes before launch #1195

Are you sure you want to change the base?

fix: kill stale wine processes before launch #1195

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai bot Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xXJSONDeruloXx Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

utkarshdalal Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

xXJSONDeruloXx Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot Apr 13, 2026 •

edited

Loading