Various s-r-launcher-service fixes, enhancements
-
launch-client: Fix memory leak when receiving NameOwnerChanged
This bug was inherited from flatpak-spawn.
Forwarded: https://github.com/flatpak/flatpak-xdg-utils/pull/59
-
launch-client: Bind to a specific launcher-service instance
The launcher API is stateful: once we have asked an instance of the launcher to start a subprocess, we must send any subsequent Terminate() or SendSignal() messages to the original instance, not to a second instance that has taken over the same well-known bus name with
s-r-launcher-service --replace
. Similarly, we must listen for ProcessExited signals from the original instance, not a replacement.We can achieve this by resolving the well-known bus name (analogous to a DNS name) to a unique name (analogous to a dynamic IP address, but more so, since it is never reused during the lifetime of a dbus-daemon).
-
launch-client: Clarify that PWD is set after --clear-env takes effect
-
launcher-service: Fix documentation of --replace
-
launcher-service: Set $MAINPID in launched commands
If the launched process is run directly (as opposed to via a wrapper script that runs it as a subprocess or puts it in the background), then this is a convenient way to attach a debugging tool like strace or gdbserver to the launched process (typically a game).
The
MAINPID
variable name is borrowed from systemd.exec(5).Note that for tools that ptrace the game (like gdbserver and strace), this will not work on Ubuntu or other distributions that use YAMA, unless you set sysctl
kernel.yama.ptrace_scope=0
to remove the restriction on debugging non-child processes.main_pid remains set even after the main process has terminated, because this gives us a way to keep track of whether the process we are looking at was the main process (which we will want in a subsequent commit to clarify the message when implementing
terminate_after
), and to keep track of whether the main process was ever run (which we will want for steamrt/tasks#129).Resolves: steamrt/tasks#126
-
launcher-service: Clarify --terminate message
With the
terminate-after
option, processes that make us exit are not necessarily the "main" process. -
launcher-service: Clarify the name of an internal function
This wasn't particularly descriptive, and I now want to use the name pv_launcher_server_stop() for something else.
-
launcher-service: Fall back to kill() if killpg() fails
If the process we are terminating is not a process group leader, then killpg() will fail with ESRCH. This is particularly significant if we are terminating a wrapped process, for which we set
child_setup_data.keep_tty_session = TRUE
so that it will not give up its controlling terminal (if any).Before this change,
test_wrap_wait
in tests/pressure-vessel/launcher.py would leak itssleep 600
subprocess, which was not correctly terminated by callings-r-launch-client --terminate
. Replace that with acat
subprocess so that we can detect when it has been terminated, and replace the previous use of stdout (which duplicated similar code intest_wrap_info_fd
) to facilitate that.This will become more important when we keep better track of child processes in order to avoid exiting while there are still managed child processes running.
-
launcher-service: Don't exit on name loss if children exist
Instead of using a GMainLoop, it's often clearer what is going on if we explicitly loop until a condition becomes false, and that seems to be the case here.
Keeping better track of the circumstances under which we exit revealed that if we lose our D-Bus name (due to being replaced by a second instance with --replace), we would have exited after a short delay. This seems unfortunate, because we don't terminate our child processes in that situation, but exiting from the launcher-service process means that the launch-client processes cannot be informed when the child processes exit or given the opportunity to send signals to them, even if they are tracking the launcher-service by its unique name (as we started to do in a previous commit). Resolve this by continuing to not terminate the child processes, but also lingering until they have all exited, so that we can propagate their exit statuses back to the caller if it is watching our unique name.
Similarly, in signal_handler() and exit_on_readable_cb(), previously we would unconditionally stop the main loop immediately. Now, we wait for child processes to exit and for a possible queue of D-Bus calls to be processed.
Related to steamrt/tasks#128
-
launcher-service: Add test coverage for --replace, etc.
This exercises two recent changes: the launch-client binds to the launcher-server by its unique name, and the launcher-server doesn't exit while it has subprocesses running, even after losing its name.
-
launch-client: Exit 0 if Terminate() succeeds
Previously, both success and failure came out as exit status 125, contradicting the man page, which said a successful --terminate was exit status 0.
-
launch-client: Clarify behaviour of --terminate -- COMMAND
Previously, the synopsis implied that --terminate was only allowed when not specifying a COMMAND, and the exit status description implied that
s-r-launch-client --terminate -- false
would exit 0 (in fact it propagates the exit status from false(1), and exits 1). -
launcher-service: Implement a --[no-]stop-on-name-loss option
This can be used to keep the launcher service alive even if it has been replaced by another similarly-named launcher, for example when using a well-known name per Steam game.
Resolves: steamrt/tasks#128
-
launcher-service: Allow more than one well-known bus name
This might be a useful way to have both per-app and per-instance bus names, for example com.steampowered.App1234 and com.steampowered.App1234.Instance54321 on the same server.
-
launcher-service: Allow more than one well-known bus name
-
pv-wrap: Move _srt_get_steam_app_id() to library code
-
launcher-service: Add automatic choice of bus name
This might make sense to be the default later, but for now it has to be requested explicitly.
It currently only sets up com.steampowered.App1234. Additionally listening for ...App1234.Instance54321 could be added later if wanted.
-
launcher-service: Add --exec-fallback option
This could be useful if we're routinely wrapping games in the launcher service: if that's the case, then we'll want to fall back to running the game without the launcher service if setup fails, rather than just not running any games until it's fixed.
Resolves: steamrt/tasks#129
Automated tests pass, manual tests not done yet. This covers most of the nice-to-haves for this tool (steamrt/tasks#125 is considerably harder, steamrt/tasks#129 should be reasonably straightforward but I think it would make sense to get these in first).