Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SetIdleHint /SetLockedHint false, but property remains true #14053

Closed
edrex opened this issue Nov 17, 2019 · 6 comments · Fixed by #14501
Closed

SetIdleHint /SetLockedHint false, but property remains true #14053

edrex opened this issue Nov 17, 2019 · 6 comments · Fixed by #14501
Labels

Comments

@edrex
Copy link

edrex commented Nov 17, 2019

systemd 243.78-2 Arch x86_64

IdleHint and LockedHint properties are true.

Calling

gdbus call -y -d org.freedesktop.login1 -o /org/freedesktop/login1/session/auto -m org.freedesktop.login1.Session.SetIdleHint false
gdbus call -y -d org.freedesktop.login1 -o /org/freedesktop/login1/session/auto -m org.freedesktop.login1.Session.SetLockedHint false

doesn't change their values, and

gdbus monitor -y -d org.freedesktop.login1 -o /org/freedesktop/login1/session/auto

doesn't report any property changes, which would seem to rule out some other caller setting the value back.

method_set_idle_hint seems very straightforward, don't see a code path that could lead to this result.

@progandy
Copy link

progandy commented Dec 10, 2019

A console session with IdleHint=false falls back to checking the tty access time, so it will always be idle when the tty does not receive any input events for IdleActionSec seconds. A graphical environment intercepts these and the tty will never receive any input.

int session_get_idle_hint(Session *s, dual_timestamp *t) {
usec_t atime = 0, n;
int r;
assert(s);
/* Explicit idle hint is set */
if (s->idle_hint) {
if (t)
*t = s->idle_hint_timestamp;
return s->idle_hint;
}
/* Graphical sessions should really implement a real
* idle hint logic */
if (SESSION_TYPE_IS_GRAPHICAL(s->type))
goto dont_know;
/* For sessions with an explicitly configured tty, let's check
* its atime */
if (s->tty) {
r = get_tty_atime(s->tty, &atime);
if (r >= 0)
goto found_atime;
}
/* For sessions with a leader but no explicitly configured
* tty, let's check the controlling tty of the leader */
if (pid_is_valid(s->leader)) {
r = get_process_ctty_atime(s->leader, &atime);
if (r >= 0)
goto found_atime;
}
dont_know:
if (t)
*t = s->idle_hint_timestamp;
return 0;
found_atime:
if (t)
dual_timestamp_from_realtime(t, atime);
n = now(CLOCK_REALTIME);
if (s->manager->idle_action_usec <= 0)
return 0;
return atime + s->manager->idle_action_usec <= n;
}

You have to explicitly mark sessions as "graphical" to stop logind from doing that. This can be done only during the session creation in the pam_systemd module with the environment variable XDG_SESSION_TYPE or the type parameter for the pam_systemd module in the pam configuration. If you have custom code to create the session, then you need to set the type paramter of the CreateSession dbus call.

Currently there are no provisions to change the session type of an existing session or to disable atime detection for console sessions. A display manager is one way to create the pam session with XDG_SESSION_TYPE set to x11, mir or wayland (those three are recognized as graphical by systemd). Another option is to set it explicitly in your pam configuration. It should also be possible to create a systemd unit that starts your display server with a pam session that has the environment variable set as well for autologin purposes.

I have not looked into the LockedHint, it may be a consequence of being idle.

@edrex
Copy link
Author

edrex commented Dec 10, 2019

Thanks for the knowledge. I'll have to dig in and do some experimentation to digest all that. I see that

❯ echo $XDG_SESSION_TYPE 
tty

Does sway started from tty inherit the tty session rather than create one? Sway project recommends starting manually from TTY so

I don't understand how this explains not being able to change the IdleHint dbus property via SetIdleHint.

I just noticed the values from by loginctl and dbus don't match:

❯ gdbus call -y -d org.freedesktop.login1 -o /org/freedesktop/login1/session/auto -m org.freedesktop.DBus.Properties.Get org.freedesktop.login1.Session IdleHint
(<true>,)
❯ loginctl show-session 
...
IdleHint=no

@progandy
Copy link

Does sway started from tty inherit the tty session rather than create one?

For a given value of "inherit". It is started inside of the tty session, so it will belong to that session. No application can create its own session without special privileges (e.g. root).

I don't understand how this explains not being able to change the IdleHint dbus property via SetIdleHint.

The internal idle_hint can be changed, but the dbus property returns the result of the session_get_idle_hint method, so you'll never see that when the atime detection spits out an idle status.

@poettering
Copy link
Member

So I thought a bit about this. But this kind of "upgrading" of sessions is quite problematic, clients should really create new sessions, so that there's a context that is referenced and when the client goes away can be destroyed again and the state returned to the status quo ante...

I thought about making the idle hint fully overridable, in both ways (currently you can mark a tty session idle, but never unmark it) but then there'd be no concept of reverting to the original state when your display manager dies again, i.e. if you leave it the last idle state will be frozen and never be updated again.

If your display manager wants the idle logic to work and operate on a tty it can of course keep touching the tty atime... But really it should just register a proper session instead that doesn't claim to be tty.

poettering added a commit to poettering/systemd that referenced this issue Jan 6, 2020
Previously we'd allow marking TTY sessions as idle, but when the user
tried to unmark it we'd just revert to automatic TTY atime idle
detection, thus making it impossible to mark the session as non-idle,
unless its tty is touched all the time. But of course, marking a session
as idle is pretty much fatal if you never can mark it as non-idle again.

This change is triggred by bug reports such as this:

systemd#14053

With this patch we will now output a clean, clear error message if this
is attempted.

I considered allowing the idle state to be overriden both ways for tty
sessions but that's problematic: for sessions that are temporarily
upgraded from tty to graphical and thus suddenly want to manage their
own idle state we'd need to a way to detect when the upgrade goes away
and thus we should revert to old behaviour. Without reverting to the
previous TTY idle auto-magic we'd otherwise be stuck in an eternally
idle or eternally non-idle state, with really bad effects in case
auto-suspend is used. Thus, let's instead generate a proper error
message, saying clearly we don't support it.

Closes systemd#14053
@tmccombs
Copy link
Contributor

tmccombs commented Jan 6, 2020

clients should really create new sessions

But as @progandy pointed out, unless the client has elevated privileges, it isn't able to create a new session. If I understand correctly, in this issue, there is no display manager, sway was started after logging in to a TTY.

Setting XDG_SESSION_TYPE in .pam_environment works if always starting a graphical session after logging in, but would probably cause unwanted behaviour if a graphical session is started in one tty and a text console is in another tty. For example, I have this in my .profile:

if [ -z "$DISPLAY" ] && [ "$XDG_VTNR" -eq 1 ]; then
  exec startx
fi

So that if I log in on tty 1, then I start X, but I can still get a normal terminal if I log in to another tty as a fallback.

poettering added a commit to poettering/systemd that referenced this issue Jan 7, 2020
Previously we'd allow marking TTY sessions as idle, but when the user
tried to unmark it as idle again it we'd just revert to automatic TTY
atime idle detection, thus making it impossible to mark the session as
non-idle, unless its TTY is atime-touched all the time. But of course,
marking a session as idle is pretty much fatal if you never can mark it
as non-idle again.

This change is triggred by bug reports such as this:

systemd#14053

With this patch we will now output a clean, clear error message if a
client tries to manipulate the idle state of a non-graphical session.
This means we now have clear rules: "manual" idle logic for graphical
sessions, and TTY based ones for all others that have a TTY of some
form.

I considered allowing the idle state to be overriden both ways for tty
sessions but that's problematic: for sessions that are temporarily
upgraded from tty to graphical and thus suddenly want to manage their
own idle state we'd need to a way to detect when the upgrade goes away
and thus we should revert to old behaviour. Without reverting to the
previous TTY idle auto-magic we'd otherwise be stuck in an eternally
idle or eternally non-idle state, with really bad effects in case
auto-suspend is used. Thus, let's instead generate a proper error
message, saying clearly we don't support it.

(Also includes some other fixes and clean-ups in related code)

Closes: systemd#14053
@poettering
Copy link
Member

I posted #14501 which makes this more discoverable...

"Upgrading" of tty sessions to graphical is still not better supported with that (because that's a hard problem, and not sure if even desirable), but at least the failure becomes clearly discoverable. But issues around are better tracked in #14489 or so.

poettering added a commit to poettering/systemd that referenced this issue Jan 8, 2020
Previously we'd allow marking TTY sessions as idle, but when the user
tried to unmark it as idle again it we'd just revert to automatic TTY
atime idle detection, thus making it impossible to mark the session as
non-idle, unless its TTY is atime-touched all the time. But of course,
marking a session as idle is pretty much fatal if you never can mark it
as non-idle again.

This change is triggred by bug reports such as this:

systemd#14053

With this patch we will now output a clean, clear error message if a
client tries to manipulate the idle state of a non-graphical session.
This means we now have clear rules: "manual" idle logic for graphical
sessions, and TTY based ones for all others that have a TTY of some
form.

I considered allowing the idle state to be overriden both ways for tty
sessions but that's problematic: for sessions that are temporarily
upgraded from tty to graphical and thus suddenly want to manage their
own idle state we'd need to a way to detect when the upgrade goes away
and thus we should revert to old behaviour. Without reverting to the
previous TTY idle auto-magic we'd otherwise be stuck in an eternally
idle or eternally non-idle state, with really bad effects in case
auto-suspend is used. Thus, let's instead generate a proper error
message, saying clearly we don't support it.

(Also includes some other fixes and clean-ups in related code)

Closes: systemd#14053
keszybz pushed a commit that referenced this issue Jan 14, 2020
Previously we'd allow marking TTY sessions as idle, but when the user
tried to unmark it as idle again it we'd just revert to automatic TTY
atime idle detection, thus making it impossible to mark the session as
non-idle, unless its TTY is atime-touched all the time. But of course,
marking a session as idle is pretty much fatal if you never can mark it
as non-idle again.

This change is triggred by bug reports such as this:

#14053

With this patch we will now output a clean, clear error message if a
client tries to manipulate the idle state of a non-graphical session.
This means we now have clear rules: "manual" idle logic for graphical
sessions, and TTY based ones for all others that have a TTY of some
form.

I considered allowing the idle state to be overriden both ways for tty
sessions but that's problematic: for sessions that are temporarily
upgraded from tty to graphical and thus suddenly want to manage their
own idle state we'd need to a way to detect when the upgrade goes away
and thus we should revert to old behaviour. Without reverting to the
previous TTY idle auto-magic we'd otherwise be stuck in an eternally
idle or eternally non-idle state, with really bad effects in case
auto-suspend is used. Thus, let's instead generate a proper error
message, saying clearly we don't support it.

(Also includes some other fixes and clean-ups in related code)

Closes: #14053
Yamakuzure pushed a commit to elogind/elogind that referenced this issue Feb 1, 2020
Previously we'd allow marking TTY sessions as idle, but when the user
tried to unmark it as idle again it we'd just revert to automatic TTY
atime idle detection, thus making it impossible to mark the session as
non-idle, unless its TTY is atime-touched all the time. But of course,
marking a session as idle is pretty much fatal if you never can mark it
as non-idle again.

This change is triggred by bug reports such as this:

systemd/systemd#14053

With this patch we will now output a clean, clear error message if a
client tries to manipulate the idle state of a non-graphical session.
This means we now have clear rules: "manual" idle logic for graphical
sessions, and TTY based ones for all others that have a TTY of some
form.

I considered allowing the idle state to be overriden both ways for tty
sessions but that's problematic: for sessions that are temporarily
upgraded from tty to graphical and thus suddenly want to manage their
own idle state we'd need to a way to detect when the upgrade goes away
and thus we should revert to old behaviour. Without reverting to the
previous TTY idle auto-magic we'd otherwise be stuck in an eternally
idle or eternally non-idle state, with really bad effects in case
auto-suspend is used. Thus, let's instead generate a proper error
message, saying clearly we don't support it.

(Also includes some other fixes and clean-ups in related code)

Closes: #14053
keszybz pushed a commit to systemd/systemd-stable that referenced this issue Feb 5, 2020
Previously we'd allow marking TTY sessions as idle, but when the user
tried to unmark it as idle again it we'd just revert to automatic TTY
atime idle detection, thus making it impossible to mark the session as
non-idle, unless its TTY is atime-touched all the time. But of course,
marking a session as idle is pretty much fatal if you never can mark it
as non-idle again.

This change is triggred by bug reports such as this:

systemd/systemd#14053

With this patch we will now output a clean, clear error message if a
client tries to manipulate the idle state of a non-graphical session.
This means we now have clear rules: "manual" idle logic for graphical
sessions, and TTY based ones for all others that have a TTY of some
form.

I considered allowing the idle state to be overriden both ways for tty
sessions but that's problematic: for sessions that are temporarily
upgraded from tty to graphical and thus suddenly want to manage their
own idle state we'd need to a way to detect when the upgrade goes away
and thus we should revert to old behaviour. Without reverting to the
previous TTY idle auto-magic we'd otherwise be stuck in an eternally
idle or eternally non-idle state, with really bad effects in case
auto-suspend is used. Thus, let's instead generate a proper error
message, saying clearly we don't support it.

(Also includes some other fixes and clean-ups in related code)

Closes: #14053
(cherry picked from commit be2bb14)
(cherry picked from commit 45d52c7)
keszybz pushed a commit to systemd/systemd-stable that referenced this issue Feb 5, 2020
Previously we'd allow marking TTY sessions as idle, but when the user
tried to unmark it as idle again it we'd just revert to automatic TTY
atime idle detection, thus making it impossible to mark the session as
non-idle, unless its TTY is atime-touched all the time. But of course,
marking a session as idle is pretty much fatal if you never can mark it
as non-idle again.

This change is triggred by bug reports such as this:

systemd/systemd#14053

With this patch we will now output a clean, clear error message if a
client tries to manipulate the idle state of a non-graphical session.
This means we now have clear rules: "manual" idle logic for graphical
sessions, and TTY based ones for all others that have a TTY of some
form.

I considered allowing the idle state to be overriden both ways for tty
sessions but that's problematic: for sessions that are temporarily
upgraded from tty to graphical and thus suddenly want to manage their
own idle state we'd need to a way to detect when the upgrade goes away
and thus we should revert to old behaviour. Without reverting to the
previous TTY idle auto-magic we'd otherwise be stuck in an eternally
idle or eternally non-idle state, with really bad effects in case
auto-suspend is used. Thus, let's instead generate a proper error
message, saying clearly we don't support it.

(Also includes some other fixes and clean-ups in related code)

Closes: #14053
(cherry picked from commit be2bb14)
Yamakuzure pushed a commit to elogind/elogind that referenced this issue Feb 12, 2020
Previously we'd allow marking TTY sessions as idle, but when the user
tried to unmark it as idle again it we'd just revert to automatic TTY
atime idle detection, thus making it impossible to mark the session as
non-idle, unless its TTY is atime-touched all the time. But of course,
marking a session as idle is pretty much fatal if you never can mark it
as non-idle again.

This change is triggred by bug reports such as this:

systemd/systemd#14053

With this patch we will now output a clean, clear error message if a
client tries to manipulate the idle state of a non-graphical session.
This means we now have clear rules: "manual" idle logic for graphical
sessions, and TTY based ones for all others that have a TTY of some
form.

I considered allowing the idle state to be overriden both ways for tty
sessions but that's problematic: for sessions that are temporarily
upgraded from tty to graphical and thus suddenly want to manage their
own idle state we'd need to a way to detect when the upgrade goes away
and thus we should revert to old behaviour. Without reverting to the
previous TTY idle auto-magic we'd otherwise be stuck in an eternally
idle or eternally non-idle state, with really bad effects in case
auto-suspend is used. Thus, let's instead generate a proper error
message, saying clearly we don't support it.

(Also includes some other fixes and clean-ups in related code)

Closes: #14053
(cherry picked from commit be2bb14f00441d9e4a26f94834518db3829e83ed)
(cherry picked from commit 45d52c7615fdc3aefb97a13a8d8f4aa90ad7205e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

4 participants