Bug 8245

Summary:	Children of a subprocess may continue to run after the subprocess is terminated
Product:	ThinLinc	Reporter:	Alexander Zeijlon <alexander.zeijlon>
Component:	VSM Server	Assignee:	Bugzilla mail exporter <bugzilla-qa>
Status:	NEW ---
Severity:	Normal
Priority:	P2
Version:	trunk
Target Milestone:	LowPrio
Hardware:	PC
OS:	Linux
See Also:	https://bugzilla.cendio.com/show_bug.cgi?id=8232
Acceptance Criteria:

Description Alexander Zeijlon cendio

2023-10-26 14:52:29 CEST

When the output asyncio.Future from extproc.subprocess_run [1] is cancelled, we make sure to terminate the process, but we do not kill or terminate any potential child processes.

[1] See bug 8232.

A situation like this can occur in loginhandler_common.py where we attach a timeout to the subprocess_run output with asyncio.wait_for when running session scripts present in /opt/thinlinc/etc/session*.d/.

This means that a session script could be written to start a process that runs forever, or until the vsmserver.service is restarted, which should be outside the scope of scripts meant to be run to completion before a user session is started.

We should therefore make sure to terminate lingering children of the process we create with subprocess_run when a timeout occurs.

Comment 1 Alexander Zeijlon cendio

2023-10-26 14:54:22 CEST

Maybe a realistic example of this could be if the session script starts a process that hangs indefinitely.

Comment 2 Alexander Zeijlon cendio

2023-10-26 15:00:52 CEST

The TAG states:
> Scripts in /opt/thinlinc/etc/sessionstartup.d and
> /opt/thinlinc/etc/sessionreconnect.d are run by the root user, on the
> VSM Server. Session information will not be sent back to the client
> until these scripts have completed. This makes it possible to ensure
> that commands have been run before the client connects to the VSM Agent.
Which makes it sound like these scripts are not expected to continue running after a timeout has triggered termination of the parent process.

Comment 3 Pierre Ossman cendio

2023-10-31 13:18:48 CET

It's unclear if this is a bug or feature as some cases might want the children to live on as background processes. We'll need to gather some more real world cases to see if there are any issues with the current approach.