Opened 9 years ago

Closed 3 years ago

Last modified 11 months ago

#12534 closed bug (fixed)

Apps started via Shortcuts prefs don't get env variables

Reported by: humdinger Owned by: leavengood
Priority: normal Milestone: R1/beta4
Component: Servers/launch_daemon Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Platform: All

Description

This is hrev49919.

According to axel in IRC: the input_server is started by the app_server. Both are in the "system" hierarchy and are started without SetupUserEnvironment. Therefore there are almost no env available which are therefore not available for apps started thru the Shortcuts preferences.

Attachments (1)

Screenshot.png (20.6 KB ) - added by ilzu 11 months ago.

Download all attachments as: .zip

Change History (61)

comment:1 by humdinger, 9 years ago

Component: Servers/input_serverPreferences/Shortcuts
Owner: changed from korli to nobody

comment:2 by zephjc, 8 years ago

I will take a look at this

comment:3 by humdinger, 8 years ago

I've had that issue with "Pe". Pe always crashed when invoked with QuickLaunch or with a keycombo set in the Shortcuts prefs. Now, launching Pe works with either of those[[BR]] Can't say what commit has made the difference, but life just got a tiny bit better! Close the ticket?

comment:5 by humdinger, 8 years ago

Right! And the Pe package was updated January 10th. Still not sure if the ticket is still valid, because the underlying issue in Haiku is probably still there...

comment:6 by humdinger, 8 years ago

Another instance of this, as reported by diver as a QuickLaunch issue #15 is the app "qBittorrent" (currently only installable in 64bit Haiku). That one needs export QT_STYLE_OVERRIDE=haiku in ~/config/settings/boot/UserSetupEnvironment.

qBittorrent won't start from QuickLaunch, LaunchBox or with the Shortcuts prefs.

comment:7 by humdinger, 7 years ago

Another example is BurnItNow. When started from Terminal the cloning of an audio CD works. When started via double-click it doesn't. Reading in the audio CD with:

cdda2wav dev=10,1,0 paraopts=proof -vall cddb=0 -B -Owav /boot/home/Soundgarden/

fails with:

cdda2wav: Invalid Argument. Cannot open output fd 0.

comment:8 by humdinger, 7 years ago

To add, the cdda2wav error comes from here, if that's any help:
https://gist.github.com/humdingerb/7d70d30814d5d7a9865b01ccb18e5dce#file-gistfile1-txt-L140

A maybe similar issue: after having successfully read in a CD with cdda2wav by having started BurnItNow from Terminal, burning the wavs with cdrecord later fails with:

cdrecord: No such file or directory. Cannot open '/boot/system/cache/burnitnow_clone_wavs/*.wav'.

That's with the same cdrecord line that works when done 'manually' in Terminal:

cdrecord -eject -sao dev=10,1,0 gracetime=2 -v -dao -useinfo -text /boot/system/cache/burnitnow_clone_wavs/*.wav

in reply to:  7 comment:9 by vidrep, 7 years ago

We can consider this a 64 bit specific failure. I was able to successfully copy an audioCD using BurnItNow with cdda2wav on x86_gcc2h without launching from terminal, and no error message.

Replying to humdinger:

Another example is BurnItNow. When started from Terminal the cloning of an audio CD works. When started via double-click it doesn't. Reading in the audio CD with:

cdda2wav dev=10,1,0 paraopts=proof -vall cddb=0 -B -Owav /boot/home/Soundgarden/

fails with:

cdda2wav: Invalid Argument. Cannot open output fd 0.

comment:10 by diver, 7 years ago

Did you launch it from Tracker or QuickLaunch?

comment:11 by vidrep, 7 years ago

double click

comment:12 by vidrep, 7 years ago

Reading in a CD with cdda2wav is now working on both 64 bit and gcc2h, but we are still left with the second problem: cdrecord: No such file or directory. Cannot open '/boot/system/cache/burnitnow_clone_wavs/*.wav'.

comment:13 by vidrep, 7 years ago

I'm going to move all cdda2wav associated issues to a new trac ticket.

https://dev.haiku-os.org/ticket/14038

comment:14 by humdinger, 6 years ago

Was this fixed recently? qBittorrent at least launches now with Shortcuts and QuickLaunch...

comment:15 by diver, 6 years ago

qBittorrent has seen quite a number of updates and was probably fixed in the meantime. However, this issue is still not fixed.

comment:16 by probono, 5 years ago

Fwiw, I can launch Clementine from Terminal but not from QuickLaunch. Is it related?

comment:17 by diver, 5 years ago

Yes, this is the same bug.

comment:18 by leavengood, 4 years ago

Component: Preferences/ShortcutsServers/launch_daemon
Owner: changed from nobody to leavengood
Status: newassigned

After some discussion on the forums I have an idea how this can be fixed in the launch_daemon. This affects any software started outside Terminal.

Changing component.

comment:19 by diver, 4 years ago

IIRC, input_server hasn't yet been converted to BServer and is launched by app_server instead of launch_daemon where we could add env vars.

Last edited 4 years ago by diver (previous) (diff)

comment:20 by X512, 4 years ago

input_server can inherit app_server environ that can be set by launch_daemon, can't it?

comment:21 by leavengood, 4 years ago

I will see what happens when I start testing, but I believe as X512 says input_server should inherit the environment from app_server, which app_server got from launch_daemon. If not then maybe this is a deeper problem. Either way the fix discussed in the forums should work for #7682.

comment:22 by pulkomandy, 4 years ago

I looked at #7682 and found only two variables that were not set in SetupEnvironment but only in profile: USER and GROUP. Are these causing a problem here? Are other variables needed? Is the problem in fact somewhere else?

comment:23 by leavengood, 4 years ago

The best test would be to define a unique variable in the ~/config/settings/boot/UserSetupEnvironment file, reboot, verify it is in the Terminal, then have a simple GUI app which reads the environment and outputs that variable (maybe in a little BSringView) and then ensure the new variable can be found when that test app is launched from Tracker, Deskbar, LaunchBox and then, of course Shortcuts.

Well that was my plan if I had managed to try to fix this.

If some of those ways of launching have this variable, but some don't, then that can narrow down where the problem is. You can know launch_daemon is fine but maybe something is not right in how input_server is launched, given apps started through Shortcuts are actually started by the input_server.

comment:24 by X512, 4 years ago

input_server is launched by app_server that is launched by root launch_daemon (see SystemManager screenshot). It don't call UserSetupEnvironment.

comment:25 by leavengood, 4 years ago

Well then that is the problem. I vaguely recall maybe Axel mentioning this, or maybe it was just you saying it before X512. I don't know enough about the plan for launch_daemon to know what is the fix here. app_server should be launched as part of the user session launch_daemon?

comment:26 by X512, 4 years ago

app_server should be launched as part of the user session launch_daemon?

One of easy solutions will be starting applications from Tracker process like Tracker start applications by double click. Tracker is already responsible for opening files with associated application.

comment:27 by X512, 4 years ago

Another idea is adding API for launch_daemon to run application from specified user.

comment:28 by vidrep, 4 years ago

Humdinger,

Has this commit resolved the issue with cloning audio CD's using BurnItNow!?

https://git.haiku-os.org/haiku/commit/?id=8aae4f21908e3e579aaac626d32f245185456735

comment:29 by humdinger, 4 years ago

I dunno, I don't have a CD drive handy...

comment:30 by vidrep, 4 years ago

Can you re-enable the function in BurnItNow!, or provide instructions on where I need to change the source code, or create a test build? I used to have some test builds which we were using before the last release, but not sure if I still have them.

Last edited 4 years ago by vidrep (previous) (diff)

comment:31 by humdinger, 4 years ago

You just have to revert this change. I've temporarily uploaded a 64bit version to http://0x0.st/-_YQ.zip

comment:32 by vidrep, 4 years ago

Humdinger,

That commit didn’t fix the inability of BurnItNow to find the cache folder and burn the .wav files. However, I found some other unrelated issues with cdda2wav that I fixed, and will address them on HaikuArchives. Thanks!

Last edited 4 years ago by vidrep (previous) (diff)

comment:33 by outsidecontext, 3 years ago

I tested what environment is available when starting from Tracker and from Shortcuts: When launching from Shortcuts only SAFEMODE and LC_TYPE are set, everything else is missing, including quite essential environment variables such as LIBRARY_PATH or PATH.

See

https://discuss.haiku-os.org/t/environment-variables-behavior-on-haiku/10454/32

comment:34 by outsidecontext, 3 years ago

As asked for by leavengood here is the app I used to dump the environment:

https://github.com/phw/HaikuEnvDump

That's really rough few lines code done with Paladin IDE. Don't look to close, I had 10 minutes spare time, haven't touched C++ for years and never before did any actual BeOS / Haiku SDK coding. It just barely does what I needed quickly :)

comment:35 by waddlesplash, 3 years ago

It would seem to me that if input_server is going to support add-ons, it probably should run on a per-user basis like app_server. When/if we implement multi-user, there should just be "hand offs" between input_servers for physical devices when switching users. So, for now, perhaps we can just start input_server as part of the user environment, and that will fix this issue, and we can add whatever else is needed to input_server for proper multi-user awareness at a later date.

comment:36 by X512, 3 years ago

I still don't understand why multiple per-user app_server is needed. It is just waste of resources and unnecessary complication. Current app_server already can create separate per-user desktops. I actually managed to get 2 desktops working with 2 separate users in test_app_server. The only problem that desktops require separate screen and screen switching is not implemented.

there should just be "hand offs" between input_servers

This also add unnecessary complication.

comment:37 by waddlesplash, 3 years ago

There need to be multiple app_servers per-user simply due to the 32-bit address space limitation, unless somehow we do not care about that. Not to mention that proper privilege separation in app_server is basically impossible and it is far easier to just run more than one of them.

Like I said above, if input_server is going to support user add-ons, then by definition it must run on a per-user basis. At least app_server could theoretically not allow user-installed add-ons, but input_server absolutely must. Or we have to add an input_addon_server, but that would add a lot of overhead and "unnecessary complication."

comment:38 by X512, 3 years ago

There need to be multiple app_servers per-user simply due to the 32-bit address space limitation

32 bit is basically obsolete. 32 bit mode in 64 bit OS should be implemented instead, some WIP work is available.

Not to mention that proper privilege separation in app_server is basically impossible

Why? Just add owner user to app_server objects and check permissions. Windows win32k.sys do that.

comment:39 by X512, 3 years ago

if input_server is going to support user add-ons, then by definition it must run on a per-user basis.

Separate processes should be preferred over input_server add-ons. input_server add-ons are primary supposed to deal with system-global things like input drivers and input methods. Bad behaving add-ons can also cause input_server instability, server crash and loss of user input.

Last edited 3 years ago by X512 (previous) (diff)

comment:40 by waddlesplash, 3 years ago

Yes, and there can be custom input methods on a per-user basis! Input filters like shortcut_catcher are a kind of "input method", too. Adding an input_addon_server would seriously increase latency for not much tangible benefit, and most of the misbehaviors aside from crashes would still be a factor.

comment:41 by waddlesplash, 3 years ago

Windows has had plenty of mixups when it comes to user objects. Also, it isn't just views and the like, app_server does font rendering, and there can be per-user fonts (or better, dynamically loaded fonts from web browsers), as well as global caches (e.g. clipping cache, font cache) where separating objects from different users will not be easy.

Why do you think it is so much simpler to not have multiple app_servers in a multi-user environment? The amount of code we would have to add to app_server in order to support that is ultimately, however you look at it, *much* larger and more complicated than simply starting separate app_servers for separate users on just about every level.

comment:42 by X512, 3 years ago

and there can be per-user fonts (or better, dynamically loaded fonts from web browsers)

As you mentioned, dynamic font management is already needed for web fonts even for single user. Per-user font handling will be not that difficult after that.

as well as global caches (e.g. clipping cache, font cache) where separating objects from different users will not be easy

Many caches can be shared globally between all users. It is big chance that multiple users will use the same font with the same size so rasterized font cache can be reused.

Why do you think it is so much simpler to not have multiple app_servers in a multi-user environment?

Single GUI server process is needed for global coordination (graphics drivers management, login screen, user switching).

The amount of code we would have to add to app_server in order to support that is ultimately, however you look at it

Multi-user will work just now after implementing screen and input stream switching. This is very small changes compared to complete redesign app_server. It is already working without any changes in test_app_server by opening each user desktop in separate window.

comment:43 by waddlesplash, 3 years ago

It is big chance that multiple users will use the same font with the same size so rasterized font cache can be reused.

Except at present, app_server presumes that fonts will be rendered with the same anti-aliasing settings. I don't think it presently supports rendering fonts with antialiasing enabled but different settings. So that is one area where massive changes would have to be made in order to support multiple users with different settings.

Similarly, desktop colors. Presently app_server presumes there is just one set of them. If different users have different colors, then app_server will have to be seriously reworked to handle that. Likewise for color changes, app_server just broadcasts them to all applications currently running. If one user changes colors, then app_server must know that and only broadcast to the one user's applications and not all the others. Likewise for font changes and other such settings, which now must be stored and broadcasted on a per-application basis.

comment:44 by waddlesplash, 3 years ago

Another one: decorators. Even two users using the same decorator with the same settings, if they use separate fonts or font sizes, the current decorator code does not handle that at all, and would need to be seriously reworked.

comment:45 by pulkomandy, 3 years ago

Another one: decorators.

This is solved by having two separate instances of the decorator object, that should be no problem at all and is not a reason to run two separate processes.

There may be good reasons to do it, but it looks like you are trying to fit all existing (or maybe even non-existing) problems to your preferred solution here.

The argument for using multiple process is mainly about security and making sure that an user can't access another's data. A possible second argument is resilience (if each user gets a separate input_server, an input_server crash could affect only one user and not everyone). Both of these are in the somewhat unlikely case that we even allow multiple users to use a machine at the same time, do we even want to go that way?

Running input_server and app_server as part of the user session is probably a good short/mid term solution to the problem here, while at the same time not being a good long term solution for future multiuser use cases. But we can make the change when we actually start doing multiuser, which as far as I know is still one of the things we put in the R2 roadmap precisely because we know it will require large changes to the way we do things.

comment:46 by waddlesplash, 3 years ago

This should be fixed in hrev55587, please test.

comment:47 by vidrep, 3 years ago

I tried to clone an audio disc with a reverted build of BurnItNow (see comment:31), but it behaves as before. I’ll try again tomorrow after updating to hrev55595.

comment:48 by outsidecontext, 3 years ago

@waddlesplash I updated to hrev55592 and things actually got worse, not better. If I run my HaikuEnvDump from above there are only 3 environment variables set: SAFEMODE, LC_TYPE and HOME. Nothing else.

And other than before this now also happens when running from Tracker. Previously it would have the full set of environment variables when running from Tracker, and a limited list only if running e.g. via Shortcuts (compare https://discuss.haiku-os.org/t/environment-variables-behavior-on-haiku/10454/31). Now it doesn't matter how I launch it, it always has nearly all environment variables missing.

That also means that applications that previously started via Tracker, but failed to start via Shortcuts due to missing environment variables, now also fail when starting via Tracker. That's e.g. true for MusicBrainz Picard, which fails to start due to missing LIBRARY_PATH variable (which should also affect other Python apps using ctypes).

So now environment variables are consistent between starting via Shortcuts and via Tracker, but only by breaking the Tracker case :(

comment:49 by diver, 3 years ago

Launching Clementine and VLC from QuickLaunch now work for me.

comment:50 by waddlesplash, 3 years ago

@outsidecontext: Yeah, hrev55595 is the real fix.

comment:51 by vidrep, 3 years ago

Tested BurnItNow with hrev55599. Same as before.

"cdrecord: No such file or directory. Cannot open '/boot/system/cache/burnitnow_clone_wavs/*.wav'."

comment:52 by waddlesplash, 3 years ago

cdrecord may require the XDG directories, which are not set except in profile.d.

I guess there are various ported applications which expect to be able to use those, too...

comment:53 by outsidecontext, 3 years ago

Yes, after upgrade to rev55599 everything works. The environment is the same when started from Tracker and Shortcuts. Thanks a lot, now finally I can fully use QuickLaunch :D

comment:54 by waddlesplash, 3 years ago

Milestone: UnscheduledR1/beta4
Resolution: fixed
Status: assignedclosed

comment:55 by ilzu, 11 months ago

I found out by accident that this is not completely fixed. I have ssh-agent running in UserSetupEnvironment and I have shortcut to open QuickLaunch. When opening an app through Quicklaunch opened by shortcut, it has different SSH_AGENT_PID for ssh-agent, so it doesn't have same ssh keys authenticated as the apps opened from Tracker or Deskbar. The other environment variables are correct.

comment:56 by waddlesplash, 11 months ago

If it has a different SSH_AGENT_PID, rather than no PID, then that means we have a mixup here between the "root" session, and then the "user" session (which is also for UID 0 aka. root, but is the part that actually launches the desktop.) That's a separate problem which deserves its own ticket.

comment:57 by ilzu, 11 months ago

Yup, different SSH_AGENT_PIDs. See screenshot, other terminal is opened from Deskbar, other from QuickLaunch started from keyboard shortcut.

by ilzu, 11 months ago

Attachment: Screenshot.png added

comment:58 by waddlesplash, 11 months ago

As I said above: a new ticket should be opened for this.

comment:59 by ilzu, 11 months ago

I'll open a new ticket, though I personally think it is not a different problem, but part of this one, but you probably know better.

Note: See TracTickets for help on using tickets.