vncsession: Please follow the recommendations of daemon(7) or at least provide it as an option

[zmudc]

Is your feature request related to a problem? Please describe.
The problem is configurability of the TigerVNC server. If I understand correctly (and please correct me if I misunderstand), currently vncsession always forks and becomes an orphaned process, and that is why Type=forking and a PID file is needed in the systemd unit configuration. But this is not the recommended way to build a modern daemon, according to systemd's daemon(7) man page. See, for example, what the current systemd daemon(7) man page says about this here:

https://github.com/systemd/systemd/blob/main/man/daemon.xml#L484

Describe the solution you'd like
Please provide the option to run vncsession so it does not fork and become an orphan. This could be implemented by adding an optional argument to the vncsession command. Currently, vncsession takes exactly two arguments: the user and the display. A third optional argument, maybe something like -D, could be implemented to support a Type=simple or Type=exec systemd unit and also would have the additional side benefit of making the TigerVNC server more compatible with other non-systemd systems that are not compatible with the Type=forking daemon. All this with (IMHO) not much extra cost.

Basically, I am asking that vncsession have the same -D option that sshd has:

-D When this option is specified, sshd will not detach and does not become a daemon. This allows easy monitoring of sshd.

Describe alternatives you've considered
The only alternative I have considered is to implement my own non-forking version of vncsession and maintain it myself.

Additional context
I think one can reduce this issue to a question: Why does the TigerVNC server use a Type=forking systemd service and not provide the option of running vncsession without forking and detaching?

[CendioOssman]

vncsession was indeed designed according to the old SysV style of doing things. This was explicitly done to maximize compatibility, as the hope was that it would be possible to use on classical init systems and not just systemd.

Your report doesn't really state a problem with this approach, beyond that it isn't the most modern way of doing things. Do you encounter any practical issues, or is this a cosmetic concern?

[CendioOssman]

And sorry for the very late response.

[zmudc]

vncsession was indeed designed according to the old SysV style of doing things. This was explicitly done to maximize compatibility, as the hope was that it would be possible to use on classical init systems and not just systemd.

Your report doesn't really state a problem with this approach, beyond that it isn't the most modern way of doing things. Do you encounter any practical issues, or is this a cosmetic concern?

So if the goal is maximum compatibility why not have the SysV style of forking services as the default with an option to make TigerVNC compatible with systems that use neither systemd nor old SysV.? I am thinking of runit on Void as one example of a distro that cannot easily be compatible with the old SysV style forking services. For example, sshd from OpenBSD uses SysV style forking as the default but provides -D as the option to stay in the foreground, and that works great with runit on Void. Why cannot TigerVNC use the same approach for maimum compatibility?

[zmudc]

OK, I am glad you have responded now. Thanks.

[zmudc]

vncsession was indeed designed according to the old SysV style of doing things. This was explicitly done to maximize compatibility, as the hope was that it would be possible to use on classical init systems and not just systemd.

Your report doesn't really state a problem with this approach, beyond that it isn't the most modern way of doing things. Do you encounter any practical issues, or is this a cosmetic concern?

I think I can try again to explain the practical issue: It is this response of the Void developers to my PR for their repository (now closed) to update their tigervnc package and add a runit service directory (the runit equivalent of a systemd unit):

void-linux/void-packages#45307 (comment)

So the Void developers are not going to accept a patch to make the TigerVNC vncsession serivce compatible with Void's runit system unless TigerVNC accepts a patch to not daemonize the vncsession process. That's the practical issue.

I closed that Void PR because the Void developers seemed to make a very final decision that if there was going to be a patch to add an option to not daemonize vncsession, it would need to be accepted upstream by the TigerVNC developers.

I thought of another solution for a distro such as Void that does not work easily with a service that daemonizes. Instead of adding the complexity of the -D command line option, implement a compile time option that disables daemonizing so a distro such as Void can compile out the code that implements daemonizing in its build of vncsession. That would be a simpler patch to vncsession than what I proposed in #1651.

[CendioOssman]

Great! So runit compatibility is a very tangible feature that we can weigh the changes against. Every feature we have adds a bit to the maintenance burden, so we want to know why we have different features.

Does runit have no support at all for forking services? Or is it just very difficult or annoying to handle them?

[zmudc]

Great! So runit compatibility is a very tangible feature that we can weigh the changes against. Every feature we have adds a bit to the maintenance burden, so we want to know why we have different features.

Does runit have no support at all for forking services? Or is it just very difficult or annoying to handle them?

I get mixed messages from the Void developers. At best, they say it is "cumbersome" to use runit with services that fork. Practically, there is a program called fghack that comes as part of the toolset with runit to deal with the problem of forking services to keep a service that forks under the supervision of runit. I searched the entire void-packages repo and there is no instance of the Void developers accepting fghack as a solution to this problem. Putting hack in the name of the tool says it best. At best it is an ugly hack.

At worst, the Void developers say that runit "requires" that services run in the foreground.

So I invite @ahesford and @classabbyamp from the VoidLinux project to weigh in here and clarify the matter.

[ahesford]

Void does ship a daemontools package that includes fghack, but it isn't part of any standard installation. The runit package provides chpst to offer some of the functionality of the daemontools functionality; fghack is not part of that. Personally, I think fghack is a pretty ugly hack indeed.

Runit is nice and simple, and people seem to love it for that, but it also has some serious limitations. Among these are an inability to manage forking processes or properly fire "one-shot" services. Void ships a few "one-shot" services that use the pause executable to run some commands and then sleep forever. We generally advise against this and I'm not aware of any abuse of pause to attept to manage daemonizing programs in official Void services. I'd be loath to accept such a solution in this case, although I'd probably advise a user looking to create a custom service to start there. Perhaps we might consider including an example service in /usr/share/examples/sv that does so for TigerVNC if it is thoughtfully implemented. (There are a lot of signal-handling edge cases that would need to be considered to make this viable.)

I see potential value in a no-daemon flag for the TigerVNC server; it would help people debug their configurations and the server in general. However, while I use the TigerVNC client on occasion, I am not a user of the server and can't attest to any personal interest in the addition.

[zmudc]

... Perhaps we might consider including an example service in /usr/share/examples/sv that does so for TigerVNC if it is thoughtfully implemented. (There are a lot of signal-handling edge cases that would need to be considered to make this viable.)

Could properly considering the many signal-handling edge cases be implemented solely in an example service in /usr/share/examples/sv and without any patches to the upstream vncsession.c?

[zmudc]

... Perhaps we might consider including an example service in /usr/share/examples/sv that does so for TigerVNC if it is thoughtfully implemented. (There are a lot of signal-handling edge cases that would need to be considered to make this viable.)

Could properly considering the many signal-handling edge cases be implemented solely in an example service in /usr/share/examples/sv and without any patches to the upstream vncsession.c?

Here is what is in the signal-handling code of vncsession.c, if that helps answer my question about whether or not the edge signal-handling cases can be handled without patching vncsession.c (script is pid of the vncserver perl script that vncsession controls):

`sighandler(int sig)
{
    (void)sig;
    if (script > 0) {
        kill(script, SIGTERM);
    }
}`

 `memset(&act, 0, sizeof(act));
  act.sa_handler = sighandler;
  sigemptyset(&act.sa_mask);
  act.sa_flags = 0;
  sigaction(SIGHUP, &act, NULL);
  sigaction(SIGINT, &act, NULL);
  sigaction(SIGTERM, &act, NULL);
  sigaction(SIGQUIT, &act, NULL);
  sigaction(SIGPIPE, &act, NULL);'

[ahesford]

I'm talking about properly handling signals sent by runsv to the run script; the run script isn't doing anything useful, so some extra logic will be required in the service to make sure the detached vncserver is manipulated appropriately.

[zmudc]

I'm talking about properly handling signals sent by runsv to the run script; the run script isn't doing anything useful, so some extra logic will be required in the service to make sure the detached vncserver is manipulated appropriately.

The other big problem I saw with unpatched vncsession.c was that when vncsession was fully detached and the first forked process exits, the logic of runit causes runsv (or maybe sv, I am no expert) to consider that the service exited and runsv (or sv) restarts it when it does not need to be restarted and an endless loop ensues of start->exit->start...

[zmudc]

I'm talking about properly handling signals sent by runsv to the run script; the run script isn't doing anything useful, so some extra logic will be required in the service to make sure the detached vncserver is manipulated appropriately.

The other big problem I saw with unpatched vncsession.c was that when vncsession was fully detached and the first forked process exits, the logic of runit causes runsv (or maybe sv, I am no expert) to consider that the service exited and runsv (or sv) restarts it when it does not need to be restarted and an endless loop ensues of start->exit->start...

So I think the question to ask: @ahesford would you help me work on a solution for this 'forking service issue' that we could fix on the Void side instead of asking TigerVNC to add the no-daemonize flag? I think over the long term that would be the best option for Void - extend runit capabilities so it works well with forking services.

[ahesford]

You've misunderstood my point. There is nothing to "extend" in runit; its inability to properly supervise daemonized processes is a fundamental consequence of its design. What I'm suggesting you do is write run, finish and (as necessary) control/* scripts to background the vncserver process, track its PID, and work whatever magic is appropriate to signal that process when runsv wants to control the service. You should read and understand the runsv(8) manual page before even thinking about this task.

Because I am not a vncserver user, I have no interest in trying to hack around its lack of a foreground option.

[zmudc]

You've misunderstood my point. There is nothing to "extend" in runit; its inability to properly supervise daemonized processes is a fundamental consequence of its design. What I'm suggesting you do is write run, finish and (as necessary) control/* scripts to background the vncserver process, track its PID, and work whatever magic is appropriate to signal that process when runsv wants to control the service. You should read and understand the runsv(8) manual page before even thinking about this task.

Because I am not a vncserver user, I have no interest in trying to hack around its lack of a foreground option.

I think I just have a broader idea of what extending runit means. I am not necessarily proposing fundamental changes to runsv, sv, etc., but coming up with a general solution to the forking service problem using specially designed run, finish, and control scripts as you propose for the case of forking services. I will read and understand runsv(8) if that is what is necessary.

[zmudc]

Because I am not a vncserver user, I have no interest in trying to hack around its lack of a foreground option.

IIUC, you are emphatically saying "NO!" to my request for your help with fixing this issue in Void. Correct?

[classabbyamp]

The proper solution is to have an operating mode where vncserver runs in the foreground and doesn't fork, not make every service manager try to corral forked processes (nodaemon/foreground/whatever you want to call it is a common functionality for daemonised programs of all kinds). The fact that this doesn't already exist is somewhat baffling to me, as this seems like a basic debugging functionality (e.g. run the thing and watch the output in the terminal)

but I also don't use tigervnc so I have no skin in this game.

[zmudc]

Great! So runit compatibility is a very tangible feature that we can weigh the changes against. Every feature we have adds a bit to the maintenance burden, so we want to know why we have different features.

Does runit have no support at all for forking services? Or is it just very difficult or annoying to handle them?

After some feedback from Void developers, I think the answer is that it is very difficult or annoying for runit to handle forking services. Furthermore, the Void developers appear unwilling to do the work of accommodating forking services by fixing this issue on the Void side because they strongly disagree with using forking services at all in Void.

N.B.: I am not a Void developer, just a Void user who only recently started trying out Void, so I definitely do not speak in any way for the Void project or for the Void developers.

[zmudc]

...

but I also don't use tigervnc so I have no skin in this game.

You are a core team member of a Linux distribution that provides the tigervnc server and you have "no skin in this game?" And you don't really care about the quality of the software that is packaged in Void except for the software in Void that you use? Sorry about that, it is a bit harsh.

Still, that response of "I have no skin in this game" does not send the kind of message I would want to send if I were a member of the core Void Team. Can't you at least say you have some skin in the game even if you don't use tigervnc because the Linux distribution you work so hard on ships it in its packages repository?

[ahesford]

Sigh. I thought my responses have been pretty clear, but ongoing commentary seems to be muddling the issue. Unequivocally: runit will not manage processes that fork away from their parents. Period. Its design REQUIRES that the run script that defines a service NOT return, or runit will just keep restarting the service.

My speculative workaround is effectively crafting a separate, single-purpose process manager IN SHELL to do a task that runit will not. It would be a terrible hack and, as I indicated before, an unprecedented one. We would never ship such a solution as a standard system service. Providing an example service that does not live among standard, supported services requires clearing a slightly lower bar, although I think it unlikely that a proposal of this nature would clear that bar. I DO NOT RECOMMEND FOLLOWING THIS APPROACH and already regret bringing it up.

If you want to manage a VNC server, convince TigerVNC to implement a foreground option or seek an alternative process supervisor.

[zmudc]

Thanks, @ahesford for the clarification!

[zmudc]

If you want to manage a VNC server, convince TigerVNC to implement a foreground option or seek an alternative process supervisor.

For my part, I will work on making the requested changes to the PR I have proposed here on the TigerVNC GitHub site, and after an updated PR is sufficiently tested, I will update the PR, cross my fingers and hope that the TigerVNC developers will be willing to consider my PR to implement a foreground option for vncsession.

[zmudc]

Sigh. I thought my responses have been pretty clear, but ongoing commentary seems to be muddling the issue. Unequivocally: runit will not manage processes that fork away from their parents. Period. Its design REQUIRES that the run script that defines a service NOT return, or runit will just keep restarting the service.

My speculative workaround is effectively crafting a separate, single-purpose process manager IN SHELL to do a task that runit will not. It would be a terrible hack and, as I indicated before, an unprecedented one. We would never ship such a solution as a standard system service. Providing an example service that does not live among standard, supported services requires clearing a slightly lower bar, although I think it unlikely that a proposal of this nature would clear that bar. I DO NOT RECOMMEND FOLLOWING THIS APPROACH and already regret bringing it up.

@ahesford I totally accept and understand your recent decision to close void-linux/void-packages#45522 as not a bug in Void. I was not inclined to speak on Void's behalf about runit here on the TigerVNC issues page, so I invited you and @classabbyamp to state the official answer of Void to the question @CendioOssman recently posed about runit:

Does runit have no support at all for forking services? Or is it just very difficult or annoying to handle them?

Best regards

[CendioOssman]

I see potential value in a no-daemon flag for the TigerVNC server; it would help people debug their configurations and the server in general. However, while I use the TigerVNC client on occasion, I am not a user of the server and can't attest to any personal interest in the addition.

For posterity, running vncsession manually is not terribly useful as a way to debug things. The reason being that it often doesn't start the user session correctly if it's already in a user session (e.g. run manually from a terminal).

In other words, the lack of a "foreground" flag can to some extent be seen as a feature.

That said, I don't consider that fact a blocker for improving support for runit.

[zmudc]

For posterity, running vncsession manually is not terribly useful as a way to debug things. The reason being that it often doesn't start the user session correctly if it's already in a user session (e.g. run manually from a terminal).

In other words, the lack of a "foreground" flag can to some extent be seen as a feature.

OK. I think the way to proceed is by adding the foreground flag because it is clear from users and developers of runit on at least one actively developed Linux distribution (Void Linux) that a design requirement of runit is that the services it supervises must run in the foreground.

Before I write the updated PR, I have one question: Do you think it is helpful or necessary to indicate in the updated vncsession man page that the flag is not intended to add support for running vncsession in an ordinary user session from a terminal or maybe better, to say the flag is not intended to be a useful way to debug vncsession from a user session on a terminal?

[CendioOssman]

Given the history of vncserver, that might be helpful, yes. We could simply state that the purpose is to start the service from service managers that don't want services to fork off in to the background.

[zmudc]

Given the history of vncserver, that might be helpful, yes. We could simply state that the purpose is to start the service from service managers that don't want services to fork off in to the background.

Thanks for the guidance on the man page. Now I can write the updated PR #1651 and make the requested changes. I plan to test it carefully so it will be a few days before I push it to GitHub.

[eternal-sorrow]

For me vncsession does not create a PID file at all for some unknown reason, so I'm unable to start it with systemd.