bug#35068: GDM crashes when it cannot find any .desktop files

bug#35068: GDM crashes when it cannot find any .desktop files

[Ludovic Courtès]

Hello,

I used to have a config (when using SLiM) where my system profile did
not contain any window manager, so no .desktop files.  Instead, the WM
would be installed in my profile and ~/.xsession would start it and a
couple of other things.

Unfortunately GDM crashes in that context.  More precisely, after
choosing the user name on the log-in screen, it enters an infinite loop
with an assertion failure:

  gdm_session_set_environment_variable: assertion 'value != NULL' failed

At that point you pretty much have to reboot since it becomes very hard
to type at the console.  (You can try it in a VM.)

The workaround is to add at least one window manager to the system’s
‘packages’ field.

It would be nice to fix it now that GDM is our default log-in manager.

Ludo’.

bug#35068: GDM crashes when it cannot find any .desktop files

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

Hi,

I just got hit by a similar problem.  I've got .xinitrc in my home directory
and no .xsession file.  The former just invokes startfluxbox.

Now after guix system reconfigure, gdm got selected as login manager.
After I log in at the gdm prompt,  it enters an endless loop, switching
terminals with almost no delay--which makes further debugging difficult.

I *think* I saw an authentication failure after I typed in my user name,
but I can't be sure.  It doesn't ever ask for my password.

I'm writing this from a recovery shell which I entered like the following:

* Reboot
* Let gdm start
* Do NOT log in (if I did, it would endless-loop the machine)
* Press Ctrl-Alt-F2
* on the text console, invoke "ps -ef |grep xorg" and type the command line
so shown again, but insert ":1" as the first argument.
* Press Ctrl-Alt-F3
* Invoke:
  DISPLAY=:1 startfluxbox
* Press Alt-F8

I have the following suggestions for improvements:

(1) Please back off after trying to login so one can actually debug things.
If the login doesn't work on the first try it won't work on the next 245 tries
in the next second either.
(2) Please have a failure fallback that at least returns you to gdm instead of
looping forever.
(3) If there is an error message plase show it in a dialog box--
there's xdialog and zenity that allows you to show a dialog box easily.
(4) Please honor .xsession and .xinitrc if it's there
(5) Please make gdm state the key to use in order to select a session type.
I've still got no idea how to select a different session type in gdm.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#35068: GDM crashes when it cannot find any .desktop files

[Ludovic Courtès]

Hi,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> I have the following suggestions for improvements:
>
> (1) Please back off after trying to login so one can actually debug things.
> If the login doesn't work on the first try it won't work on the next 245 tries
> in the next second either.
> (2) Please have a failure fallback that at least returns you to gdm instead of
> looping forever.
> (3) If there is an error message plase show it in a dialog box--
> there's xdialog and zenity that allows you to show a dialog box easily.
> (4) Please honor .xsession and .xinitrc if it's there
> (5) Please make gdm state the key to use in order to select a session type.
> I've still got no idea how to select a different session type in gdm.

We should identity how that GDM assertion failure is reached and patch
GDM to simply print an error or bail out.  Anything would be better than
the endless loop.

(Note that the zero-.desktop-files situation cannot happen when
installing from our installation image, so I would not consider it a 1.0
blocker; it’s definitely something we should fix, though.)

Thanks,
Ludo’.

bug#35068: GDM crashes when it cannot find any .desktop files

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 970 bytes --]

Further investigation:

diff -ru orig/gdm-3.26.2.1/daemon/gdm-session.c gdm-3.26.2.1/daemon/gdm-session.c
--- orig/gdm-3.26.2.1/daemon/gdm-session.c      1970-01-01 01:00:00.000000000 +0100
+++ gdm-3.26.2.1/daemon/gdm-session.c   2019-04-21 10:58:31.211164558 +0200
@@ -2483,6 +2483,10 @@
                                       const char *value)
 {
         g_return_if_fail (key != NULL);
+        if (value == NULL) {
+                g_warning ("XXX gdm: value == NULL for key == %s", key);
+                value = "";
+        }
         g_return_if_fail (value != NULL);
 
         g_hash_table_replace (self->priv->environment,

Yields:

Apr 21 11:23:57 localhost gdm: XXX gdm: value == NULL for key == PATH 
Apr 21 11:23:57 localhost gdm: Child process -1384 was already dead. 
Apr 21 11:23:57 localhost gdm: Child process 1381 was already dead. 
Apr 21 11:23:57 localhost gdm: Unable to kill session worker process 

and endless loop.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#35068: GDM crashes when it cannot find any .desktop files

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

Apr 21 11:38:51 localhost gdm: GdmSessionWorkerJob: Running session_worker_job process: gdm-session-worker [pam/gdm-launch-environment] /gnu/store/qz8413ijzxnh6kfkxr8syxjwvhsc6s3a-gdm-3.26.2.1/libexec/gdm-session-worker 
Apr 21 11:38:51 localhost gdm: GdmSessionWorkerJob: : SessionWorkerJob on pid 1463 
Apr 21 11:38:51 localhost gdm: Child process -1457 was already dead. 
Apr 21 11:38:51 localhost gdm: GdmSessionWorkerJob: Stopping job pid:1454 
Apr 21 11:38:51 localhost gdm: Child process 1454 was already dead. 
Apr 21 11:38:51 localhost gdm: Unable to kill session worker process 
[...]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#35068: GDM crashes when it cannot find any .desktop files

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 454 bytes --]

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> Apr 21 11:23:57 localhost gdm: XXX gdm: value == NULL for key == PATH 
> Apr 21 11:23:57 localhost gdm: Child process -1384 was already dead. 
> Apr 21 11:23:57 localhost gdm: Child process 1381 was already dead. 
> Apr 21 11:23:57 localhost gdm: Unable to kill session worker process 

Good catch!

Looks like setting PATH in GDM’s environment isn’t enough, though…

Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1642 bytes --]

diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index d4e73c13b4..26cfad6044 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -859,7 +859,11 @@ the GNOME desktop environment.")
                                   ;; can depend on GNOME Shell directly.
                                   (cons #$gnome-shell
                                         '#$(gdm-configuration-gnome-shell-assets
-                                            config)))))))))
+                                            config)))))
+
+                           ;; GDM wrecks havoc when PATH is unset:
+                           ;; <https://bugs.gnu.org/35068>.
+                           "PATH=/run/current-system/profile/bin"))))
          (stop #~(make-kill-destructor))
          (respawn? #t))))
 
diff --git a/gnu/system/examples/desktop.tmpl b/gnu/system/examples/desktop.tmpl
index 3931bad60d..e3afe0b8e2 100644
--- a/gnu/system/examples/desktop.tmpl
+++ b/gnu/system/examples/desktop.tmpl
@@ -62,12 +62,7 @@
   ;; by clicking the gear.  Use the "desktop" services, which
   ;; include the X11 log-in service, networking with
   ;; NetworkManager, and more.
-  (services (append (list (service gnome-desktop-service-type)
-                          (service xfce-desktop-service-type)
-                          (set-xorg-configuration
-                           (xorg-configuration
-                            (keyboard-layout keyboard-layout))))
-                    %desktop-services))
+  (services %desktop-services)
 
   ;; Allow resolution of '.local' host names with mDNS.
   (name-service-switch %mdns-host-lookup-nss))

bug#35068: GDM crashes when it cannot find any .desktop files

[Timothy Sample]

Hi Ludo and Danny,

Date: Thu, 25 Apr 2019 11:36:36 -0400

Ludovic Courtès <ludo@gnu.org> writes:

> Danny Milosavljevic <dannym@scratchpost.org> skribis:
>
>> Apr 21 11:23:57 localhost gdm: XXX gdm: value == NULL for key == PATH 
>> Apr 21 11:23:57 localhost gdm: Child process -1384 was already dead. 
>> Apr 21 11:23:57 localhost gdm: Child process 1381 was already dead. 
>> Apr 21 11:23:57 localhost gdm: Unable to kill session worker process 
>
> Good catch!
>
> Looks like setting PATH in GDM’s environment isn’t enough, though…

The PATH variable for GDM gets set in the package definition, too.  GDM
does a lot of environment clobbering, so it probably gets lost along the
way (it wants to follow the PAM default environment files rather than
propagate things down).  That being said, I don’t think this is the
issue.

GDM is hard-coded to fail if there are no “.desktop” files.  This
happens in the “get_fallback_session_name” function.  As an experiment,
I tried patching this out, which allows me to type in a password, but
fails later in the same way.  I’m not sure why.

I suppose we should add a default “.desktop” file to keep GDM from
exploding.  I did look at Danny’s patch (#35377), and it would work, but
it seems a little arbitrary.  Nothing understands the “Exec=custom”
line, and our “xinitrc” runs “~/.xsession” regardless of what desktop
entry is selected in the DM.

There are two workarounds.  The first is to keep Danny’s patch as-is,
but add logic to “xinitrc” so that it only uses “~/.xsession” when
passed “custom” as an argument (it’s kinda magic, but it seems to be a
common convention).  I have a slight preference for this option, because
it makes the whole “~/.xsession” thing more explicit.  However, it would
break other DMs that don’t install a “custom.desktop”.  Maybe we could
integrate it into all DMs at the service level.

The quicker workaround is to keep the current logic, and install the
following with GDM:

    [Desktop Entry]
    Encoding=UTF-8
    Type=Application
    Name=Fail
    Comment=This session always fails.
    NoDisplay=true
    Exec=false

This way, GDM fails cleanly when there are no “.desktop” files.  It
doesn’t show up in the list, either (“NoDisplay=true”), so everything
just kinda works as expected without any visible changes.

Danny, maybe you could adjust your patch to follow the second option and
apply it for the sake of everyone else who’s system profile doesn’t have
any “.desktop” files.  It might save people headaches in the short-term
regardless of what we settle on as a final solution.

Thoughts?


-- Tim

bug#35068: GDM crashes when it cannot find any .desktop files

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]

Hi Timothy,

On Thu, 25 Apr 2019 14:49:42 -0400
Timothy Sample <samplet@ngyro.com> wrote:

> exploding.  I did look at Danny’s patch (#35377), and it would work, but
> it seems a little arbitrary.  Nothing understands the “Exec=custom”
> line, and our “xinitrc” runs “~/.xsession” regardless of what desktop
> entry is selected in the DM.

gdm does know it.  It bundles gdm Xsession startup scripts and then runs
".xsession" (see data/Xsession.in) if available, otherwise ~/.Xclients.

> There are two workarounds.  The first is to keep Danny’s patch as-is,
> but add logic to “xinitrc” so that it only uses “~/.xsession” when

.xinitrc is not picked up by gdm.  Do you mean by startx?

> break other DMs that don’t install a “custom.desktop”.  Maybe we could
> integrate it into all DMs at the service level.

Well, the best way would be for gdm to support .xsession files like anyone
else (without desktop file)--but I'm not holding my breath.
 
> This way, GDM fails cleanly when there are no “.desktop” files.  It
> doesn’t show up in the list, either (“NoDisplay=true”), so everything
> just kinda works as expected without any visible changes.

I want it to show up in the list.  Maybe we are trying to reach different
goals here.  I have a ~/.xsession script for close to a decade now and I
want gdm to use it.  It's not only to keep gdm from crashing, it's so I
can get into my normal customized desktop.

Doesn't Exec=false make the login fail?  Or do you mean gdm will pick up
.xsession anyway and run it--and after the session is terminated, the
login will fail?  Why would it then fail?  Why not make it succeed?

> Danny, maybe you could adjust your patch to follow the second option and
> apply it for the sake of everyone else who’s system profile doesn’t have
> any “.desktop” files.  It might save people headaches in the short-term
> regardless of what we settle on as a final solution.

Sounds good in principle, as long as it actually allows me to log in to
my desktop.

FWIW, "Exec=custom" is the way gdm itself does it.

Still, ~/.xinitrc is not picked up either way--which is too bad.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#35068: GDM crashes when it cannot find any .desktop files

[Timothy Sample]

Hi Danny,

Oh my!  I explained things very poorly.  Sorry.  I’ll provide some more
context.

Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi Timothy,
>
> On Thu, 25 Apr 2019 14:49:42 -0400
> Timothy Sample <samplet@ngyro.com> wrote:
>
>> exploding.  I did look at Danny’s patch (#35377), and it would work, but
>> it seems a little arbitrary.  Nothing understands the “Exec=custom”
>> line, and our “xinitrc” runs “~/.xsession” regardless of what desktop
>> entry is selected in the DM.
>
> gdm does know it.  It bundles gdm Xsession startup scripts and then runs
> ".xsession" (see data/Xsession.in) if available, otherwise ~/.Xclients.

By default, GDM does not bundle an Xsession script.  You have to pass
“--enable-gdm-xsession” to the configure script for it to be installed.
I added this flag in 51bc8357e8457d5d7168d8837da6e14fa2d98485, but the
script doesn’t support “~/.xsession” scripts, so (after some discussion)
I removed it in 41fa9f1815685ede0d3fdc1c561d2a9cf0ffb158.  It seems that
most distros use their own scripts and in turn, the GDM folks don’t
really give much thought to “data/Xsession.in”.

Furthermore (I was surprised by this), even though the
“data/Xsession.in” script says that it will honour “custom” in a
comment, the code says otherwise.  There’s a commit where support for it
gets removed, but they must have forgotten about the comment.

>> There are two workarounds.  The first is to keep Danny’s patch as-is,
>> but add logic to “xinitrc” so that it only uses “~/.xsession” when
>
> .xinitrc is not picked up by gdm.  Do you mean by startx?

No.  We have our own script that we use to initialize X and it is bound
to “xinitrc” in “gnu/services/xorg.scm”.  You can override it from the
GDM configuration record.

>> break other DMs that don’t install a “custom.desktop”.  Maybe we could
>> integrate it into all DMs at the service level.
>
> Well, the best way would be for gdm to support .xsession files like anyone
> else (without desktop file)--but I'm not holding my breath.

We don’t need a “.desktop” file currently.  My personal opinion is that
having the existence of the “~/.xsession” script override your
explicitly stated session selection when logging in is a little
surprising.  But if that’s conventional, then I’m happy to follow
convention.

>> This way, GDM fails cleanly when there are no “.desktop” files.  It
>> doesn’t show up in the list, either (“NoDisplay=true”), so everything
>> just kinda works as expected without any visible changes.
>
> I want it to show up in the list.  Maybe we are trying to reach different
> goals here.  I have a ~/.xsession script for close to a decade now and I
> want gdm to use it.  It's not only to keep gdm from crashing, it's so I
> can get into my normal customized desktop.
>
> Doesn't Exec=false make the login fail?  Or do you mean gdm will pick up
> .xsession anyway and run it--and after the session is terminated, the
> login will fail?  Why would it then fail?  Why not make it succeed?

Currently, our “xinitrc” script will run your “~/.xsession” script if it
exists, regardless of what session you select when logging in.  However,
GDM will crash if you do not have any “.desktop” files.  You don’t
really need a “.desktop” file for anything, but GDM goes nuts if it
can’t find any.  So, we could fix GDM, but that looks pretty tricky.  In
lieu of that, we could provide a dummy “.desktop” that is invisible and
exists only to placate GDM when no other “.desktop” file exists.  With
the dummy file, if you have a “~/.xsession” script, you will be able to
log in even if you have no other “.desktop” files.  If you have no
“.desktop” files and no “~/.xsession” script, logging in will fail
(which seems reasonable, seeing how desperate the circumstances are).

>> Danny, maybe you could adjust your patch to follow the second option and
>> apply it for the sake of everyone else who’s system profile doesn’t have
>> any “.desktop” files.  It might save people headaches in the short-term
>> regardless of what we settle on as a final solution.
>
> Sounds good in principle, as long as it actually allows me to log in to
> my desktop.

According to all the tests I did this afternoon, it should!  :)

For people with other “.desktop” files, everything should be the same.
For people with no other “.desktop” files GDM will no longer go bananas,
but rather let you log in using “~/.xsession”.

> FWIW, "Exec=custom" is the way gdm itself does it.

It’s true that most distros honour “custom” in their scripts.  It
doesn’t happen at the GDM level, though (like I wrote above).

> Still, ~/.xinitrc is not picked up either way--which is too bad.

Isn’t that script specific to “startx”?  (That’s at least what a popular
question and answer site tells me.)


-- Tim

bug#35068: GDM crashes when it cannot find any .desktop files

[Danny Milosavljevic]

[-- Attachment #1: Type: text/plain, Size: 3025 bytes --]

Hi Timothy,

On Thu, 25 Apr 2019 21:02:53 -0400
Timothy Sample <samplet@ngyro.com> wrote:

> Furthermore (I was surprised by this), even though the
> “data/Xsession.in” script says that it will honour “custom” in a
> comment, the code says otherwise.  There’s a commit where support for it
> gets removed, but they must have forgotten about the comment.

> We don’t need a “.desktop” file currently.  My personal opinion is that
> having the existence of the “~/.xsession” script override your
> explicitly stated session selection when logging in is a little
> surprising.  But if that’s conventional, then I’m happy to follow
> convention.

I don't know what's conventional here.

But a CUSTOM session is just that:
A customer customized their session and presumably wants to enter that
customized session.  However, he should still be able to enter other
sessions by selecting a non-customized session in the DM.

If I wrote a DM, I'd scan for desktop files, add those to the session selection
menu, then scan for the file "~/.xsession" and, if that exists, add a
"custom" entry to the session selection menu.

The session that had been selected previously for that user should be the
default for that user the next time--unless he changes the selection again.

The first time a ~/.xsession shows up, the session manager should select it
(or ask).

In any case, let's just make gdm not crash and do a minimally invasive fix.
We are not upstream and they can clean up their own mess.

> >> This way, GDM fails cleanly when there are no “.desktop” files.  It
> >> doesn’t show up in the list, either (“NoDisplay=true”), so everything
> >> just kinda works as expected without any visible changes.  

Nice!

> Currently, our “xinitrc” script will run your “~/.xsession” script if it
> exists, regardless of what session you select when logging in.

Hmm, that's not what I thought it would do.  We could change it...

> However, GDM will crash if you do not have any “.desktop” files.

Yeah, there are a lot of places where it assumes that desktop files
have been read.  Even the "fallback" case requires desktop files.

>  With
> the dummy file, if you have a “~/.xsession” script, you will be able to
> log in even if you have no other “.desktop” files.  If you have no
> “.desktop” files and no “~/.xsession” script, logging in will fail
> (which seems reasonable, seeing how desperate the circumstances are).

That's fair.

> > Still, ~/.xinitrc is not picked up either way--which is too bad.  
> 
> Isn’t that script specific to “startx”?  (That’s at least what a popular
> question and answer site tells me.)

Yes, I had it from the time I still used startx.

So I guess it's okay if .xinitrc isn't used by gdm.

It would still be nice if .xsession was only used for the "custom session"
choice, which is what I wanted it to do.  I guess we could also adapt
our xinitrc script.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

bug#35068: GDM crashes when it cannot find any .desktop files

[Ludovic Courtès]

Hello,

Timothy Sample <samplet@ngyro.com> skribis:

> There are two workarounds.  The first is to keep Danny’s patch as-is,
> but add logic to “xinitrc” so that it only uses “~/.xsession” when
> passed “custom” as an argument (it’s kinda magic, but it seems to be a
> common convention).  I have a slight preference for this option, because
> it makes the whole “~/.xsession” thing more explicit.  However, it would
> break other DMs that don’t install a “custom.desktop”.  Maybe we could
> integrate it into all DMs at the service level.
>
> The quicker workaround is to keep the current logic, and install the
> following with GDM:
>
>     [Desktop Entry]
>     Encoding=UTF-8
>     Type=Application
>     Name=Fail
>     Comment=This session always fails.
>     NoDisplay=true
>     Exec=false
>
> This way, GDM fails cleanly when there are no “.desktop” files.  It
> doesn’t show up in the list, either (“NoDisplay=true”), so everything
> just kinda works as expected without any visible changes.

I have a slight preference for the hack you propose Timothy, at least in
the short term, because it seems less likely to break anything than
fiddling with “Exec=custom” and all that.

Anyway, it’d be great if one of you could push a fix by 1.0.  :-)

Thanks,
Ludo’.

bug#35068: GDM crashes when it cannot find any .desktop files

[Timothy Sample]

Hi Ludo and Danny,

Ludovic Courtès <ludo@gnu.org> writes:

> Hello,
>
> Timothy Sample <samplet@ngyro.com> skribis:
>
>> There are two workarounds.  The first is to keep Danny’s patch as-is,
>> but add logic to “xinitrc” so that it only uses “~/.xsession” when
>> passed “custom” as an argument (it’s kinda magic, but it seems to be a
>> common convention).  I have a slight preference for this option, because
>> it makes the whole “~/.xsession” thing more explicit.  However, it would
>> break other DMs that don’t install a “custom.desktop”.  Maybe we could
>> integrate it into all DMs at the service level.
>>
>> The quicker workaround is to keep the current logic, and install the
>> following with GDM:
>>
>>     [Desktop Entry]
>>     Encoding=UTF-8
>>     Type=Application
>>     Name=Fail
>>     Comment=This session always fails.
>>     NoDisplay=true
>>     Exec=false
>>
>> This way, GDM fails cleanly when there are no “.desktop” files.  It
>> doesn’t show up in the list, either (“NoDisplay=true”), so everything
>> just kinda works as expected without any visible changes.
>
> I have a slight preference for the hack you propose Timothy, at least in
> the short term, because it seems less likely to break anything than
> fiddling with “Exec=custom” and all that.
>
> Anyway, it’d be great if one of you could push a fix by 1.0.  :-)

Pushed as 8caa458953eeac783c73a0e5aaa72842fe3914c9.

I’m closing the bug and the patch.  If we want to change the “xinitrc”
script to understand “custom”, we can start a new discussion to figure
out the best way to do it after 1.0.


-- Tim

bug#35068: GDM crashes when it cannot find any .desktop files

[Ludovic Courtès]

Timothy Sample <samplet@ngyro.com> skribis:

> Pushed as 8caa458953eeac783c73a0e5aaa72842fe3914c9.

Thanks!

> I’m closing the bug and the patch.  If we want to change the “xinitrc”
> script to understand “custom”, we can start a new discussion to figure
> out the best way to do it after 1.0.

Sounds good.

Ludo’.