X2Go Bug report logs - #541
Patch to close Bug #525 has undesirable side effects

Full log

Message #5 received at submit@bugs.x2go.org (full text, mbox, reply):

Received: (at submit) by bugs.x2go.org; 8 Jul 2014 22:19:45 +0000
From newsgroups.mail2@stefanbaur.de  Wed Jul  9 00:19:44 2014
X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on
	ymir.das-netzwerkteam.de
X-Spam-Level: 
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham
	version=3.3.2
Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.17.13])
	by ymir.das-netzwerkteam.de (Postfix) with ESMTPS id E520F5DB24
	for <submit@bugs.x2go.org>; Wed,  9 Jul 2014 00:19:43 +0200 (CEST)
Received: from appserv1.stefanbaur.home (dslb-188-105-141-120.pools.arcor-ip.net [188.105.141.120])
	by mrelayeu.kundenserver.de (node=mreue102) with ESMTP (Nemesis)
	id 0LtX6a-1WdAiZ1NEi-010uuQ; Wed, 09 Jul 2014 00:19:43 +0200
Message-ID: <53BC6E79.1030906@stefanbaur.de>
Date: Wed, 09 Jul 2014 00:19:37 +0200
From: Stefan Baur <newsgroups.mail2@stefanbaur.de>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.6.0
MIME-Version: 1.0
To: submit@bugs.x2go.org
Subject: Patch to close Bug #525 has undesirable side effects
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
X-Provags-ID: V02:K0:taIeLHQWDAC3YFXphUxwQtmj1YNt2nR9uembHP0UPvW
 ezG4fOZZpmfwqPNXsuSnRTT4V/aucaomXLVXXN0bM2j1UzS/a4
 SPpp+GL8XMZS9AsL9W8kF5FacKZcfydPENeGYqvF3fA5pEOY/2
 uk7Io105vFwOTjlGlLRVoOuW1DHibf4qlyjcclvNlVYo2XCNUm
 UwYnlJ4YSkAZUx+3QZgb6LmSCNd164/Y0ZTyAgXbwBQ+8wbtYu
 EW2wV650RUrVDtBv0gP7vqIdEqV7fWOeapmd/6sJ+AamVpbhPT
 bjzf8jYTHGHN4Lh5TwEWbYGx+3TAsiCWZSi9dc1vieYKveAsM4
 aqfD7Eb7XgNPct8pc4RgLJqVbC3jXzeDJ5oEDt57/ZotdZ4OyS
 VBUhC3c1IaLeQ==

Package: x2goclient
Version: 4.0.2.1
Severity: grave

Hi,

Alex' latest patches introduce a new and more severe bug.

His idea, assuming that Mike#1. Mike#2, Mihai and me understood him correctly, is, that on Windows, users shouldn't start x2goclient.exe any more, but rather the new x2gohelper.exe. 

==>Alex, if we're misunderstanding what you wrote, please reply and explain your intentions.


Assuming we're right, your plan breaks backward compatibility when it comes to desktop shortcuts on Windows, in two places.

The smaller issue is that clicking the arrow on the session tile will create a desktop shortcut that is hardcoded to x2goclient.exe instead of x2gohelper.exe.
This could be fixed with a small patch. However ...


The bigger issue is that *all* existing links users may have on their desktops are broken, since they point to x2goclient.exe and can't easily be changed!
I have at least one customer that makes heavy use of that feature, where it would mean manually updating over 600 desktop shortcuts. That is not going to be fun!
We need to find a way to achieve what you were trying to achieve without breaking backward compatibility like that.

From what I understand so far, you're trying to build a "reaper" to clean up after a dying x2golient.exe, which may have left pulseaudio, xserver and ssh daemon running.

I haven't tried to figure out what your current x2gohelper.exe is trying to do, and if you made changes to x2goclient.exe as well where functionality from x2goclient.exe is moved to x2gohelper.exe.  If you moved it, rather than duplicating it, that's not going to help, because it might just as well be x2gohelper.exe that dies first and you'd again leave sshd, pulseaudio, and xserver dangling.

Duplicating it might work, similar to how certain malware on Windows works (two processes checking on each other and re-spawning whichever gets killed first).

So you'd have x2goclient.exe on the one hand permanently checking if x2gohelper.exe is running, respawning it when necessary, and upon shutdown, "trying" to clean up behind itself by terminating sshd, pulseaudio, xserver, and finally itself. While x2gohelper.exe would be checking if x2goclient.exe is still running - if not, it will terminate sshd, pulseaudio, xserver, (if any of them are still running) and finally itself.

Before you start coding, I'd like to ask this question, though:

Do we really want to have a reaper mechanism that tries to clean up after a malfunctioning x2goclient.exe, or wouldn't it be better to find out why x2goclient.exe is malfunctioning, fix these issues / catch those exceptions and deal with them appropriately (i.e. properly shutting down sshd, pulseaudio and xserver before going belly-up)? The latter would mean we need more bug reports and it requires analyzing what exactly went wrong, but it would be less disturbing to x2goclient development and roll-out.

Whichever path you choose, Alex, please do keep in mind that we have a release planned in the near future and your latest commits aren't exactly helpful in getting 4.0.2.1 stabilized (and I *need* 4.0.2.1 out soon, my customers are desperately waiting for it because of the improved sound that PA5 brings).

It would be really really cool if you could either do your development on a separate branch that could later become 4.0.2.2, or hold back until we have released 4.0.2.1.

If you don't know how to do create your own branch for development, I'm sure there is more than just one Dev on the list that will help you.

Kind Regards,
Stefan

Send a report that this bug log contains spam.