gamescope icon indicating copy to clipboard operation
gamescope copied to clipboard

gamescopereaper does not kill child winedevice

Open R1kaB3rN opened this issue 1 year ago • 1 comments

Description

After forcefully closing the X connection for an application that is run through wine, it's observed gamescope fails to kill the winedevice subprocess. As a result, users need to manually kill the process for both the app window and the gamescope instance to close. Moreover, applications that depend on gamescope functionality need to implement custom logic to handle killing the winedevice.

Expectation

Gamescope properly kills all children when the X connection closes.

Steps to reproduce

  1. Install gamescope that supports gamescopereaper
  2. Install wine
  3. Open a terminal emulator
  4. Run gamescope -- winecfg
  5. Click the OK button or send ctrl+c from terminal window
  6. Observe that the app window is still visible
  7. Run ps -e | grep winedevice to observe that winedevice has not been killed

Gamescope version: 3.14.29-1 Distribution: Arch Linux Desktop: sway Window system: wayland

R1kaB3rN avatar Aug 20 '24 19:08 R1kaB3rN

Same here... winedevice.exe remains after wine app closes. This only happens with gamescope. image Is it possible to disable gamescopereaper?

EDIT: Here is a workaround. Disable winedevice with WINEDLLOVERRIDES="winedevice.exe=d"

EDIT2: Hmm this workaround isn't very good as it disables gamepad in most games... :-( So we need a general fix.

DocMAX avatar Sep 08 '24 11:09 DocMAX

is it possible to disable gamescopereaper at all? i mean the game is allready isolated by "reaper" from steam

DocMAX avatar Sep 30 '24 13:09 DocMAX

Actually i found another workaround. You can have wineserver run in background permanently when launching games. Winedevice.exe closes normaly. I created a systemd user service like this: In homedir: .config/systemd/user/wineserver.service

[Unit]
Description=Wineserver Daemon

[Service]
ExecStart=/usr/bin/wineserver -p -f
Restart=on-failire

[Install]
WantedBy=default.target

No idea why this is working.

DocMAX avatar Sep 30 '24 13:09 DocMAX

is it possible to disable gamescopereaper at all?

Not in runtime. But if you build gamescope from source, you can use this patch:

--- a/src/steamcompmgr.cpp
+++ b/src/steamcompmgr.cpp
@@ -7367,7 +7367,7 @@ void LaunchNestedChildren( char **ppPrimaryChildArgv )
 
 	if ( ppPrimaryChildArgv && *ppPrimaryChildArgv )
 	{
-		pid_t nPrimaryChildPid = gamescope::Process::SpawnProcessInWatchdog( ppPrimaryChildArgv, false );
+		pid_t nPrimaryChildPid = gamescope::Process::SpawnProcess( ppPrimaryChildArgv );
 
 		std::thread waitThread([ nPrimaryChildPid ]()
 		{
@@ -7385,7 +7385,7 @@ void LaunchNestedChildren( char **ppPrimaryChildArgv )
 	if ( g_bLaunchMangoapp )
 	{
 		char *ppMangoappArgv[] = { (char *)"mangoapp", NULL };
-		gamescope::Process::SpawnProcessInWatchdog( ppMangoappArgv, true );
+		gamescope::Process::SpawnProcess( ppMangoappArgv );
 	}
 }
 

Maybe later I will come up with a PR implementing runtime CLI switch for that.

HanabishiRecca avatar Oct 01 '24 14:10 HanabishiRecca

ok, something like "--disable-reaper" switch would be great, thanks.

DocMAX avatar Oct 02 '24 08:10 DocMAX

has nothing to do with gamescope. gamescopereaper is the problem! launch "gamescopereaper -- wine explorer" and see. winedevice.exe hangs! this has to be fixed!

DocMAX avatar Dec 11 '24 13:12 DocMAX

@DocMAX I know you asked me to look into this issue I will eventually, but I’m busy this week. Also, I get the feeling that this issue may be trickier to investigate, since it’s quite possible that this hang is due to the combined quirks of both gamescopereaper and winedevice

sharkautarch avatar Dec 11 '24 14:12 sharkautarch

yeah could me challenging. thank you! maybe gamescopereaper has similarities with reaper from steam (/home/docmax/.local/share/Steam/ubuntu12_32/reaper) this one is exiting normally, which leads me to my new workaround: sudo ln -s $HOME/.local/share/Steam/ubuntu12_32/reaper /usr/local/bin/gamescopereaper works :-)

https://github.com/ValveSoftware/gamescope/blob/master/src/Apps/gamescopereaper.cpp vs. https://github.com/Plagman/reaper/blob/main/reaper.cpp

DocMAX avatar Dec 11 '24 14:12 DocMAX

@DocMAX

LOOOOL, i just compiled gamescopereaper and it WORKS! WTF??? And i have no idea what the code does. Are programmers really obsolete or what? This world is crazy.

Yes, let's let the magic black box try to fix random stuff. Surely that could never lead to regressions in the future

I'm 99% sure that it works because:

  1. gamescope's current gamescopereaper.cpp waits on all pids, but exits the waiting loop once it sees that the main child pid was returned by waitpid(). ChatGPT decided that was unnecessary, but I doubt that ChatGPT considers much of the risks of this change, insofar as you could argue that this change could increase the risk of gamescope hanging at exit.
  2. ChatGPT decided to add the WNOHANG option to waitpid(): https://linux.die.net/man/2/waitpid

WNOHANG return immediately if no child has exited.

IMO, that seems like a pretty inefficient solution, which (if ChatGPT didn't decide to "fix" the inefficiency by adding a 1s wait between waitpid() calls) will increase battery drainage, and make steamdeck users extremely annoyed (IIRC gamescopereaper starts up after gamescope starts up, so it's running for the entire session). Then ChatGPT added in a sleep(1);, which will add a whole second to the shutdown time for gamescope, which might not seem like a lot, but will definitely annoy some steamdeck users who like to switch from gaming mode to desktop mode for some games (or to use the deck as a normal PC)

This revised gamescopereaper.cpp implements the following fixes to address the hanging winedevice.exe issue:

Subreaper Setting: Uses prctl(PR_SET_CHILD_SUBREAPER) to ensure orphaned processes are correctly adopted and reaped. Signal Handling: Handles SIGTERM and SIGINT for graceful termination. Child Process Reaping: Regularly cleans up child processes using waitpid(-1, ...). Process Termination: Ensures proper termination of child processes by sending SIGTERM and SIGKILL if necessary

This is not directed at you, DocMax, more towards AI companies that exagerrate the ability of AI to perform all of a programmer's tasks. I hate this so much, because all of these revisions that ChatGPT claimed are what "fixed" this issue, are things that the current gamescopereaper already does. So not even ChatGPT can tell us what changes it made that actually "fixed" the issue...

sharkautarch avatar Dec 11 '24 18:12 sharkautarch

I am sorry, i did a mistake. It is NOT working as i expected. But the reaper bin from Steam works. Thank you for analyzing ChatGPTs output anyway :-)

DocMAX avatar Dec 11 '24 18:12 DocMAX

this is reproducible now on SteamOS 3.7 if you sudo pacman -S wine and follow the steps in the original post while on on plasma-wayland.

matte-schwartz avatar Dec 13 '24 20:12 matte-schwartz

please guys, any solution? can't even find a workaround like trap pkill winedevice.exe wrapped in a script...

DocMAX avatar Dec 14 '24 21:12 DocMAX

please guys, any solution? can't even find a workaround like trap pkill winedevice.exe wrapped in a script...

this is workaround i use as startup script

wineserver -p -f &
wineserver_pid=$!
gamescope [gamescope arguments] -- wine "[game executable]"
kill -KILL $wineserver_pid

it would probably cause issues when you have multiple games but i dont run multiple games at a time usually so that's not a problem for me

Kitten-boop avatar Dec 14 '24 22:12 Kitten-boop

I ended here twice due to two diferent bugs, one is the winedevice issue, and the other is that the gamescopereaper don't catch the orphan process created by the parent. A workaround that worked to me was making a subreaper and use it with gamescope, this subreaper waits until all the children ended. Here is the code for whoever needs it, adapt it to your needs:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/prctl.h>
#include <sys/wait.h>

void wait_descendents(void) {
	while(1) {
		int pid = wait(NULL);
		if(pid == -1) break;
	}
}

int main(int argc, char **argv) {
	int pid;
	if ((pid = fork()) == 0 ) {
		// Start bash as a child process
		execvp("bash", (char*[]){ "bash", "-c", "WINEDLLOVERRIDES=\"d3d9=n,b\" wine ./Soku/th123e.exe", NULL });
		perror("execvp failed");
		return EXIT_FAILURE;
	}

	// Set this process as a child subreaper
	if (prctl(PR_SET_CHILD_SUBREAPER, 1) != 0) {
		perror("prctl failed");
		return EXIT_FAILURE;
	}

	wait_descendents();

	return EXIT_SUCCESS;
}

and to use it:

gamescope -- ./subreaper

NemuiSen avatar Dec 26 '24 08:12 NemuiSen