Bug 457859

*** Bug 472541 has been marked as a duplicate of this bug. ***

I can still reproduce this issue on Plasma 6.0.2. I tested with sleeping on laptop lid close:

1. run `systemd-inhibit --what=idle:sleep:shutdown:handle-lid-switch --who=test --why=coz sleep 300` 
2. close the laptop lid

Observed result:
Computer goes to sleep

Expected result:
Computer stays on

I can workaround the issue by stopping PowerDevil using `systemctl --user stop plasma-powerdevil.service`, and then closing the lid without using `systemd-inhibit` puts the computer to sleep, while doing so while systemd-inhibit is running (as per the above command) correctly prevents the computer from sleeping.

An related issue (though unrelated to the use of systemd-inhibit) is that the battery widget's  "Manually block sleep and screen locking"  doesn't prevent sleep by closing the lid either.

Tested on Plasma master (around 6.0.3) with a Wayland session (which shouldn't matter all the inhibition stuff is using D-Bus).

I can reproduce Comment #2 with the lid switch inhibitor getting ignored and the system suspending anyway. A quick look at the code suggests that the processAction() call in HandleButtonEvents::onLidClosedChanged() is getting invoked unconditionally, does not check for current policy status, and makes the call to the "SuspendSession" action with an "Explicit" property that bypasses further policy checks. That's very fixable.

I cannot reproduce Comment #0 though. The created sleep/idle inhibitor works just fine for me to keep my open laptop from sleeping. Can anybody confirm that this is not an issue anymore?

I also cannot reproduce the problem with inhibiting "idle", on Plasma 6.

I just realized that recognizing an inhibition for "handle-lid-switch" would not be possible with the current architecture, because PowerDevil sets the same inhibition by itself already. It's active all the time while the service is running, which is usually the entire time the Plasma session is active. It sets the inhibition to replace systemd's own lid switch configuration with Plasma behaviors, because systemd doesn't allow changing its own behaviors or config at runtime (as far as I'm aware).

I do think it's a bug that "handle-lid-switch" inhibitors don't do anything within a Plasma session. A proper solution to this might involve some investigation into what's possible with or missing in systemd to make it outsource its lid switch handling to Plasma. If it can do that, we'd be able to hook PowerDevil into systemd directly and get rid of PowerDevil's "handle-lid-switch" inhibitor, resolving this issue in the process.

(In reply to Jakob Petsovits from comment #5)
> I do think it's a bug that "handle-lid-switch" inhibitors don't do anything
> within a Plasma session. A proper solution to this might involve some
> investigation into what's possible with or missing in systemd to make it
> outsource its lid switch handling to Plasma.

Well - if we close the lid on an active Plasma session, it will put the system to sleep, so PowerDevil definitely knows when the lid has been switched. And if you ask systemd for the inhibitors list (for example - using the command `systemd-inhibit --list`) you can see that other inhibitors are holding the lid-switch inhibition - so PowerDevil should be able to note that and not suspend the device.

(In reply to Oded Arbel from comment #6)
> (In reply to Jakob Petsovits from comment #5)
> > I do think it's a bug that "handle-lid-switch" inhibitors don't do anything
> > within a Plasma session. A proper solution to this might involve some
> > investigation into what's possible with or missing in systemd to make it
> > outsource its lid switch handling to Plasma.
> 
> Well - if we close the lid on an active Plasma session, it will put the
> system to sleep, so PowerDevil definitely knows when the lid has been
> switched. And if you ask systemd for the inhibitors list (for example -
> using the command `systemd-inhibit --list`) you can see that other
> inhibitors are holding the lid-switch inhibition - so PowerDevil should be
> able to note that and not suspend the device.

Ah yes, you're right. Filtering out PowerDevil by "who" should do the trick. Thanks for the good thought, it looks like my brain isn't in top shape today :P

(In reply to Jakob Petsovits from comment #7)
> Ah yes, you're right. Filtering out PowerDevil by "who" should do the trick.
> Thanks for the good thought, it looks like my brain isn't in top shape today
> :P

Although there is still the issue of when to check for updates. Right now PowerDevil monitors the "BlockInhibited" property of the logind D-Bus interface and re-reads the current list of inhibitors when that property changes. If an inhibitor for "handle-lid-switch" is already set, adding a second one won't cause "BlockInhibited" to change. There is an "NCurrentInhibitors" property which could help with this, but no D-Bus signal is emitted when it changes.

One could resort to polling, but that's a terrible hack and we should first look into a proper long-term solution before considering to add this temporarily.

(In reply to Jakob Petsovits from comment #8)
> (In reply to Jakob Petsovits from comment #7)
> > Ah yes, you're right. Filtering out PowerDevil by "who" should do the trick.
> > Thanks for the good thought, it looks like my brain isn't in top shape today
> > :P
> 
> Although there is still the issue of when to check for updates. Right now
> PowerDevil monitors the "BlockInhibited" property of the logind D-Bus
> interface and re-reads the current list of inhibitors when that property
> changes. If an inhibitor for "handle-lid-switch" is already set, adding a
> second one won't cause "BlockInhibited" to change. There is an
> "NCurrentInhibitors" property which could help with this, but no D-Bus
> signal is emitted when it changes.
> 
> One could resort to polling, but that's a terrible hack and we should first
> look into a proper long-term solution before considering to add this
> temporarily.

Again, not my brightest day. The next best thing, which should rather work out, would be not to rely on monitoring properties in the first place. We can make an extra D-Bus call right when the lid-closed event comes in, to see if any "handle-lid-switch" inhibitors other than PowerDevil's own are active at the time of lid closure.

We'd still miss the removal of such inhibitors without polling, though. Perhaps if only polling while the lid is already closed and an inhibitor was previously found, the hack isn't too severe. Still, it would be nice if systemd made this easier for us.

Submitted https://github.com/systemd/systemd/issues/32196 for expanding inhibitor monitoring capabilities of the logind interface, hopefully this makes even remote sense to the systemd team.

In the meantime, it would be worth exploring whether setting a systemd "sleep" and/or "idle" inhibitor should always keep a laptop from going to sleep, or locking the screen, respectively. Does a user expect that the laptop will keep running with a sleep inhibitor being set, or would a user expect that their configured lid action overrides any apps setting inhibitors? Will we need to distinguish?

(In reply to Jakob Petsovits from comment #11)
> In the meantime, it would be worth exploring whether setting a systemd
> "sleep" and/or "idle" inhibitor should always keep a laptop from going to
> sleep, or locking the screen, respectively.

I think this is exactly what it means to "inhibit sleep" - that the system will not go to sleep.

> Does a user expect that the
> laptop will keep running with a sleep inhibitor being set, or would a user
> expect that their configured lid action overrides any apps setting
> inhibitors?

If the user wants to just inhibit the lid close action, then they should inhibit "handle-lid-switch". I would like to use that so I can take my laptop from room to room, while it is still running, without having to carry it with the screen open - it is mighty uncomfortable and as far as I know this was the original impetus for inhibiting. 

> Will we need to distinguish?

No, the inhibit actions are clearly labeled.

I think the only confusing may arise from the inhibit type "sleep" vs. "idle", "handle-suspend-key" or "handle-lid-switch" - all the later ones describe the user behavior that will trigger sleep while the first one I believe would mean "any sleep". So closing the lid will cause the computer to sleep unless either "handle-lid-switch" *or* "sleep" is inhibited.

Specifically, when I toggle the "Manually block sleep and screen locking" button in the PowerDevil Plasma widget, I expect my computer to not sleep for any reason - including closing the laptop lid or pressing the suspend key.

I can't reproduce the comment described in the original post either.

As for https://bugs.kde.org/show_bug.cgi?id=457859#c2; sleep inhibitions not being respected on lid close would be a different issue and best discussed in a separate bug report. However I would consider this behavior intentional. I think that inhibition makes sense for preventing the computer from *automatically* going to sleep when the system is considered idle because no user activity is detected while there actually is activity, e.g. watching a video. When the user closes the lid or presses the power button, this is an intentional action so I would expect the system to go to sleep anyway. 

> If the user wants to just inhibit the lid close action, then they should inhibit "handle-lid-switch". I would like to use that so I can take my laptop from room to room, while it is still running, without having to carry it with the screen open - it is mighty uncomfortable and as far as I know this was the original impetus for inhibiting. 

There is no user-facing setting for this though.

> If the user wants to just inhibit the lid close action, then they should inhibit "handle-lid-switch". I would like to use that so I can take my laptop from room to room, while it is still running, without having to carry it with the screen open - it is mighty uncomfortable and as far as I know this was the original impetus for inhibiting. 

This seems like a bit of a corner case to me: Having the system configured to sleep on lid close and having an active inhibition ongoing and carrying the laptop to a different room and not wanting to keep it open by doing so. Imo, the slight inconvenience of having to keep the laptop open in this scenario is outweighed by the effects that blocking sleep unconditionally would have. A user might close their laptop to carry it in their bag, expecting it to go to sleep, and instead the machine would keep running, potentially causing it to overheat.

> Specifically, when I toggle the "Manually block sleep and screen locking" button in the PowerDevil Plasma widget, I expect my computer to not sleep for any reason - including closing the laptop lid or pressing the suspend key.

Fwiw, the wording for this in the applet has just been clarified; the inhibition section now has a caption which reads "Automatic Sleep and and Screen Locking after Inactivity".

(In reply to Natalie Clarius from comment #13)
> > If the user wants to just inhibit the lid close action, then they should inhibit "handle-lid-switch". ...
>
> There is no user-facing setting for this though.

There is - it is `systemd-inhibit --what:handle-lid-switch` or possibly I didn't understand what specifically you have talked about.

> This seems like a bit of a corner case to me: [...]
> Imo, the slight inconvenience of having to keep the laptop open in this
> scenario is outweighed by the effects that blocking sleep unconditionally
> would have.  [...]

I think this is the entire point of the "handle-lid-switch" inhibit action is to inhibit handling of the lid switch - it doesn't matter if you think it is a bad idea, if the user has specifically told you that this is what they want to do, using the API they have for doing that, and all other systems respect that (e.g. systemd, or - I'm going to say without actually testing - GNOME) - then PowerDevil should also respect that.

> As for https://bugs.kde.org/show_bug.cgi?id=457859#c2; sleep inhibitions not
> being respected on lid close would be a different issue and best discussed
> in a separate bug report. 

Granted - I'll go open a different ticket for the lid switch inhibition support, and I will consider this ticket as "RESOLVED WORKSFORME" - though its not my ticket to close and there are maintainers around now, so I'll keep my greedy little paws off of the status selector ;-)

Oded, your Comment #2 included the command `systemd-inhibit --what=idle:sleep:shutdown:handle-lid-switch --who=test --why=coz sleep 300` which sets all the inhibitors at once. To me it's clear that the "handle-lid-switch" inhibitor is insufficiently handled right now, and Bug 485623 (which you've now created to address that particular issue) is arguably valid.

Natalie's comment was about the "sleep" and "idle" inhibitors, replying to the question I raised independently of "handle-lid-switch" which I tried to analyze first (up to Comment #10). There was some discussion on Matrix involving a number of KDE developers, I think the outcome is that we're too concerned to make "sleep" and "idle" inhibit lid switch behavior. Despite the difference in behavior compared to systemd's own handling of the same inhibitors.

But we'll still want to fix "handle-lid-switch". Let me follow up in Bug 485623 as well.

I'd say we can regard the original issue as fixed, "sleep" and "idle" work as they should to inhibit after inactivity. The existing lid switch behavior for "sleep" and "idle" (i.e. ignoring these inhibitors to prevent overheating laptops in bags) has been clarified as intentional.