[winswitch] Terminating Xpra does not terminate spawned Xorg until Xpra becomes ready

[Troll Berserker]

On 01/11/18 08:53, Antoine Martin via shifter-users wrote:
> On 01/11/2018 10:36, Troll Berserker via shifter-users wrote:
>> Hi,
>>
>> I want to terminate Xpra if it didn't not become ready within some
>> period of time.
>> I've noticed that terminating Xpra (by sending SIGTERM or SIGINT) does
>> not terminate the already spawned Xorg server.
> It does, but there was a bug (not directly in xpra AFAICT) which caused
> the python cleanup hooks to fail to run, leaving the Xorg process alive.
> 
> For more details, see:
> https://xpra.org/trac/ticket/1943
> 
> Just make sure that you're up to date and everything will work fine.
>

Thank you, but I use Xpra 2.4.1 with this fix incorporated.
Prolly I was not clear enough.
Xorg gets killed if it assigns display number.
But if I kill the main process between Xorg start and display assignment, the Xorg process is not killed.

I've looked at
https://xpra.org/trac/browser/xpra/trunk/src/xpra/scripts/server.py?rev=20254#L774 and
https://xpra.org/trac/browser/xpra/trunk/src/xpra/scripts/server.py?rev=20254#L818
and I see why this happens.

The `kill_display` cleanup function is added to the list of cleanups only  after the function `start_Xvfb` returns.
And it does not return until the display number is assigned.

Possible fix? Instead of returning the list of `cleanups` from the `start_Xvfb`, pass the `add_cleanup` callback and add cleanups ASAP.
Also, move the code from https://xpra.org/trac/browser/xpra/trunk/src/xpra/scripts/server.py?rev=20254#L818 to the `start_Xvfb`, immediately after the  `subprocess.Popen`.

>> It wouldn't be a problem to kill the Xorg server if there were an easy
>> way to get its PID from Xpra.
>> But Xpra makes it more hard than necessary by using its main PID in the
>> Xorg.log name instead of Xorg's PID.
>> E.g. if Xpra is started with PID 300, it uses this PID in the Xorg.log
>> file name even though the PID of the child where the Xorg is exec()d is
>> 303.
> That's difficult to improve: the log filename is specified as an
> argument to the Xorg process. At that point, the new process ID is not
> known.
>

`subprocess.Popen` has the `preexec_fn` param, which you are using.
It is called after the fork, when the child PID is already known.
Isn't it possible to modify `xvfb_cmd` in the `preexec` function?