After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 333525 - Should reset timeouts on hardware events too
Should reset timeouts on hardware events too
Status: RESOLVED FIXED
Product: gnome-power-manager
Classification: Deprecated
Component: gnome-power-manager
unspecified
Other Linux
: Normal major
: ---
Assigned To: GNOME Power Manager Maintainer(s)
GNOME Power Manager Maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2006-03-05 20:20 UTC by David Zeuthen (not reading bugmail)
Modified: 2006-03-06 21:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test patch (791 bytes, patch)
2006-03-06 18:44 UTC, Richard Hughes
none Details | Review

Description David Zeuthen (not reading bugmail) 2006-03-05 20:20:12 UTC
Today I got home to my laptop. The laptop was in my bedroom running on AC and I wanted to move it to the living room. I hit the touchpad to workaround this known bug and then removed the AC power. Then my system went to sleep because of bug 333290. 

I think the solution to the problem is this: When getting hardware events that relates to the user fiddling around with the laptop (unplugging/plugging the AC power, opening/closing the lid, others) gnome-power-manager should reset it's inactivity timers so the system doesn't e.g. goes to sleep. When you think of it, this is as much user input as X events (e.g. moving the mouse and/or using the keyboard) - we should treat it the same way.

I'm marking this major as it's confusing to end users and may lead to data loss (e.g. kicking you off IRC when you don't expect it). I'm also a little surprised we have this bug after this much testing. But, eh, the fix should be simple and should get in soon. 

This is seen on gnome-power-manager-2.13.92-1 on Fedora Core Rawhide.

Thanks.
Comment 1 Richard Hughes 2006-03-05 23:24:27 UTC
>gnome-power-manager should reset it's inactivity timers

It uses events from gnome-screensaver.

Jon, how can I tell g-s that it's got a "user" event and to reset it's timers?

Something like SimulateUserInput would do the trick.

Richard.
Comment 2 David Zeuthen (not reading bugmail) 2006-03-05 23:55:13 UTC
> It uses events from gnome-screensaver.

Ugh, that sounds fragile. You really ought to maintain your own timers in g-p-m so you can reset them as appropriate. Just think of the possible race conditions if you were to use SimulateUserInput

 1. user removes AC
 2. g-p-m sends SimulateUserInput to g-s
 3. g-p-m decides it's inactive and it's time to sleep
 4. g-s responds to SimulateUserInput by telling g-p-m it's no longer inactive
 5. but it's too late now.. g-p-m already invoked Suspend() on HAL

Of course, I haven't read all of the g-s and g-p-m code but this race condition sounds plausible...
Comment 3 Richard Hughes 2006-03-06 09:33:04 UTC
Yes, I guess it't a bit racey. Jon, can we just reset the system timer?
Comment 4 William Jon McCann 2006-03-06 16:13:15 UTC
g-s already has a SimulateUserInput equivalent in the Poke method.

Letting gnome-screensaver do the X input monitoring shouldn't be fragile.  The alternatives are just awful.  Duplicating the functionality in g-p-m would be bad for performance reasons, difficult to keep state synchronized with g-s, code maintenance overheads, etc.

The race David describes couldn't happen because we really have this:
1. Remove AC
2. a) g-p-m calls Poke on g-s
   b) g-s detects an idle session and emits SessionIdleChanged (true)
3. a) g-p-m starts counting down to sleep (1 minute minimum)
   b) g-s resets in reponse to Poke and emits SessionIdleChanged (false)
4. g-p-m resets sleep timers when it sees SessionIdleChanged signal

Or in the case where the screen locks on idle:
1. Remove AC
2. a) g-p-m calls Poke on g-s
   b) g-s detects an idle session and emits SessionIdleChanged (true)
3. a) g-p-m starts counting down to sleep (1 minute minimum)
   b) g-s pops up the unlock prompt
4. User provides authentication
5. g-p-m resets sleep timers when it sees SessionIdleChanged signal


So, the delay between idle and sleep guards us quite a bit.  Even in the cases where the minute delay isn't enough there really isn't any dire outcome.  The computer might suspend if the battery is already critical which is unlikely.  And again, all of this depends on timing the cord yanking exactly with the session idle state change which is quite unlikely.

Perhaps we should handle these events in g-s.  However, g-s is in hard code freeze for 2.14.  For now I think it is fine to use Poke and we'll take another look at this after the thaw.
Comment 5 Richard Hughes 2006-03-06 18:44:35 UTC
Created attachment 60781 [details] [review]
test patch

Ahh, I thought poke was more of a "give me the unlock dialogue" rather than a SimulateUserInput type thing.

David, can you try the attached patch please? Jon, does this look okay (done for button events, and ac-changed)?

Richard.
Comment 6 William Jon McCann 2006-03-06 18:59:07 UTC
Poke should behave the same as moving the mouse or hitting a key that has no effect (of course there is no such key which is one reason why historically inhibiting screensavers has been such a pain).  So, when the screen is blanked it will either unblank or request auth.  If the screen is not blanked then it should reset the idle timers.

I haven't tried the patch yet but it seems sane.  FWIW, Windows seems to interpret AC state changes as input - last I checked it shows the screen when you pull the plug.
Comment 7 Richard Hughes 2006-03-06 19:23:21 UTC
Okay, thanks Jon, I've committed the patch to CVS:

2006-03-06  Richard Hughes  <richard@hughsie.com>
 * src/gpm-manager.c (power_button_pressed_cb, power_on_ac_changed_cb): Poke the screensaver to simulate user input. This is what Windows does, and should solve the problems with the timer event happening shortly after the ac_adapter is removed. See #333525 for details.

David, can you confirm that CVS now works as expected please.

Many thanks to both of you.

Richard.
Comment 8 David Zeuthen (not reading bugmail) 2006-03-06 21:20:39 UTC
Jon, thanks for the eloquent description in comment 4. Richard, I'll try this later today and will report back.
Comment 9 David Zeuthen (not reading bugmail) 2006-03-06 21:31:10 UTC
I've got the password dialog when puling out AC and the screen saver was running. So this looks right, closing. Thanks.