launch_daemon & BRoster: environment management problems

[waddlesplash]

Recently, in hrev55587 (and hrev55595), I moved app_server to launch in user context in order to fix #12534. However, even before the introduction of the launch_daemon, Haiku has had problems with managing environment variables. See e.g. #7682, which was fixed by editing SetupEnvironment (the script now invoked in hrev55595.)

shortcut_catcher uses BRoster to launch things, which of course is totally non-user-aware and simply launches them with the present environment. Actually, the distinction at present between "system" and "user" is somewhat artificial, as both are root.

What the "correct" solution is, I'm not entirely sure, and thus this ticket is here to discuss the matter.

[waddlesplash]

Someone also noted that programs run via SSH without a login shell mostly don't work because of missing PATH and the like, which is another facet of the same problem (and not solved by any of the above changes.)

Perhaps the thing to do is move most of SetupEnvironment into launch_daemon's _SetupEnvironment function, so that everything has access to those variables?

[pulkomandy]

I think the first step towards a solution is to make BRoster aware of these environments, so that we are able to specify which one to use when running an application. That would mean adding some extra parameter:

• Maybe keep it simple and have just two constants: B_USER_ENVIRONMENT and B_SYSTEM_ENVIRONMENT, this gets us covered for single-user systems
• Maybe make it more complicated and have some kind of session identifier. Maybe we can use UNIX sessions and process groups to some extent (setsid(), setpgid(), setpgrp()).

In UNIX, a single user can have multiple sessions, this matches well with what we need here (you can have a local session, and a separate ssh session maybe with a remote app_server, for the same user. And you can also run your system and user sessions with the same user).

Launch Daemon already calls setsid, so we already have the sessions in place, we just need to use them.

As to how to handle the parameter: currently there is a single BRoster which allow access to all apps in all sessions, and to run apps in the current session only. In a multiuser environment we could have multiple BRoster instances for each user (probably only in the system session, user sessions would not be able to see each other at all). So you could create a BRoster corresponding to a specific session, and then start apps in it.

The registrar would be aware of that and know how to list apps for a specific session, set up the correct environment in BRoster::Launch, make sure new apps connect to the correct session in app-server, etc.

[waddlesplash]

I think the first step towards a solution is to make BRoster aware of these environments, so that we are able to specify which one to use when running an application.

That may make sense, but for now, BRoster is basically always used to launch applications for the GUI (applications use posix_spawn or fork() instead if they want specific behaviors, etc.) So if we do add a parameter, the default should be B_USER_ENVIRONMENT, and it should remain that way.

However, another question is: should the GUI really run with less environment variables as a general rule? Because if BRoster is just going to inject whatever environment variables are missing, anyway, then all GUI apps besides those started initially by launch_daemon will have the full environment.

So, it would thus seem to make sense to just have all GUI apps have the same environment, and then the question about B_USER/B_SYSTEM/etc. environments becomes another thing we can worry about later, when we actually work on multiuser support.

[waddlesplash]

Basically, in short: while making BRoster environment/session-aware does seem to be a necessity for multiuser support, I don't know if we need to think about that right now so much, and just having applications all launch with the same environment one way or another (which is what the changes to launch_daemon so far have been about) makes the most sense to me.

[pulkomandy]

In the UNIX way, the environment is either inherited from the parent app (exec) or specified explicitly (execve, posix_spawn).

When using load_image, the environment is also specified explicitly as a parameter. So launching a process with a different environment is not the problem here.

This is, however, not currently exposed by BRoster::Launch. Sounds like an easy addition to the API to make a new version of BRoster::Launch where an environment can be specified.

Then there is the question of how we're going to use that, and in particular, what environment do we use and who defines it?

What is currently done with the changes you have recently merged is running app_server and input_server in the user session so they have the user environment. If we want to avoid that, we need to somehow get the correct environment. Currently this environment is known only by the launch daemon instance responsible for a particular user session. I see two options:

• Instead of calling load_image directly, tell LaunchRoster to launch an app in that session from us.
• Add an API to Launch Roster to be able to retrieve the environment from it, and then call load_image with that environment.

I think the first makes more sense, because it will easily allow to put the process in the correct session (not just the environment). For example this means we know we can kill that process when the user session ends (for example, if it was a remote app_server session and the user disconnects?). It might be possible to attach the process to that session after the fact, too (setsid, setpgrp?) but it seems easier to let launch daemon handle things?

That may make sense, but for now, BRoster is basically always used to launch applications for the GUI (applications use posix_spawn or fork() instead if they want specific behaviors, etc.) So if we do add a parameter, the default should be B_USER_ENVIRONMENT, and it should remain that way.

Yes, I wasn't very clear with what I meant by "parameter". After looking at the code, I think the way I would do it is be_roster would be the roster for the current session for the app (so, the system roster for system apps, and the user roster for user apps). And system apps would be allowed to create other BRoster instances, allowing them to manage apps in a specific session. So the API doesn't really need to change, if you wrote your app using be_roster, you're fine.

If we do it that way, it means be_roster would use the current app environment, but you could create BRoster instances targetting other sessions, and in that case it would get the environment from the corresponding launch_daemon, or somehow message the launch_daemon of that session and let it start the application, instead of doing it directly with load_image.

Generally this would allow to implement the "log out" operation (to terminate an user session) similarly to our existing shutdown process, asking all apps in a given session/BRoster instance to cleanly exit, without stopping any of the system things.

[X512]

It should be some function to launch application as specific user. It can be handled by launch_daemon and set environment variables for specified user. This should be easy to implement.

[X512]

Replying to pulkomandy:

• Maybe keep it simple and have just two constants: B_USER_ENVIRONMENT and B_SYSTEM_ENVIRONMENT, this gets us covered for single-user systems

I think that setting UID is better idea. Set it to root user and it will be system environment. Set is to current user (or user that currently have input attached for input_server) and it will be user environment.

[X512]

Launching application as root or another user may need confirmation dialog with password.