GNOME Bugzilla – Bug 635840
Clock doesn't show correct time immediately after resuming from suspend
Last modified: 2011-03-15 11:23:41 UTC
Looks like it's only updated every minute, and nothing happens when resuming until this timeout is reached.
*** Bug 638835 has been marked as a duplicate of this bug. ***
There was discussion on linux-kernel about adding a generic event for when wall-clock time differs from monotonic time. However, honestly I think the sanest thing is to change UPower or something to emit a DBus signal for now.
i'm pretty sure gnome-panel already gets this right, so we should just see what it does (in addition to watching for resumes, it also watches /etc/localtime so it can notice if you change your timezone)
This is fairly high-visibility if you ever resume, so we should probably try to get a fix for 3.0
David, any update on this ?
This got reported repeatedly during our second F15 GNOME3 test day. It is not only problematic after resume, but also when explicitly changing the time in the date+time panel. I think we should treat this as a blocker.
*** Bug 644623 has been marked as a duplicate of this bug. ***
Created attachment 183344 [details] [review] Proposed patch From: David Zeuthen <davidz@redhat.com> Subject: [PATCH] clock: update every second If we don't update every second, we may show the wrong time for up to a minute on 1. resume; or 2. when changing the time; or 3. when changing the timezone. This is both annoying and and leads to people thinking that the tool for changing the time / timezone is broken. https://bugzilla.gnome.org/show_bug.cgi?id=635840 Signed-off-by: David Zeuthen <davidz@redhat.com>
Review of attachment 183344 [details] [review]: Lame; would much prefer a patch for watching the upower resuming signal, and for an GFileMonitor on /etc/localtime
(In reply to comment #9) > Review of attachment 183344 [details] [review]: > > Lame; would much prefer a patch for watching the upower resuming signal, and > for an GFileMonitor on /etc/localtime Sigh. What you are proposing is an micro-optimization of the worst kind - one that makes the system about an order of magnitude more complex than it needs to be. Leading it wild debugging goose chases ad infinitum. Waking up once a second, on the second, is virtually free. Keep it simple please. (Also, your proposal doesn't cover the case when the time is changed. And the case where you type pm-suspend on the command-line. And so on.)
(In reply to comment #10) > > Sigh. What you are proposing is an micro-optimization of the worst kind - one > that makes the system about an order of magnitude more complex than it needs to > be. Leading it wild debugging goose chases ad infinitum. Waking up once a > second, on the second, is virtually free. Keep it simple please. Maybe it seems like overkill for *this* case; but if we multiplied across all the OS components "waking up once a second is free", if even just say 3 processes did so, multiplied by say 3 logged in users, having to bounce back and forth to X server and kernel etc. to draw or whatever... > (Also, your proposal doesn't cover the case when the time is changed. Yes; we could have gnome-control-center send us a message; but how did GNOME 2 handle this? > And the > case where you type pm-suspend on the command-line. And so on.) Sure it does, because pm-utils runs the pm scripts which poke upower, or should.
Worrying about waking up the compositor once per second seems a bit silly.
(In reply to comment #12) > Worrying about waking up the compositor once per second seems a bit silly. I suppose if we're VT switched away, the X server will block us, and if we're logged in we're probably being used. But all of this doesn't make it less *lame*. I'd be OK with the patch for 3.0 if it added a comment linking back to this bug for someone to fix it better in the future.
Hi, >> Sigh. What you are proposing is an micro-optimization of the worst kind - one >> that makes the system about an order of magnitude more complex than it needs to >> be. Leading it wild debugging goose chases ad infinitum. Waking up once a >> second, on the second, is virtually free. Keep it simple please. > > Maybe it seems like overkill for *this* case; but if we multiplied across all > the OS components "waking up once a second is free", if even just say 3 > processes did so, multiplied by say 3 logged in users, having to bounce back > and forth to X server and kernel etc. to draw or whatever... With the default settings (don't draw seconds), only the JS bits in the shell process would get woken up every second. In particular, the X server etc. wouldn't (necessarily) get woken up because the contents of the St.Label does not change. And with the way g_timeout_add_seconds() work, these JS bits would run at the same time as other code on the whole system would run. And since you are guaranteed to have such code on the system (the kernel and/or udisks polling for media changes), it's virtually free since the JS bits are virtually free.... And if it's only the shell, it's only once a second. (It's true that having a process woken up every second before g_timeout_add_seconds() was problematic... but with g_timeout_add_seconds() it's much less of a problem.) >> (Also, your proposal doesn't cover the case when the time is changed. > > Yes; we could have gnome-control-center send us a message; but how did GNOME 2 > handle this? > >> And the >> case where you type pm-suspend on the command-line. And so on.) > > Sure it does, because pm-utils runs the pm scripts which poke upower, or > should. The problem with this line of thinking is that the shell will only work with these tools that you decide (GNOME system settings, pm-suspend, upower). Since we know that a large part of the user base does use _other_ tools than these (such as date(1), "echo sleep > /sys/power/state", it causes the shell to appear to not work for that population of users. Imagine what happened if the File Manager only showed volumes you mounted by double-clicking an icon? (Btw, this is not far fetched - imagine someone doing automatic regression testing of the shell's clock by using the date(1) command.) That's what I meant with "wild debugging goose chase" - the user population mentioned above (which coincides with the users actually filing bugs on our products) will think that the shell is not working and therefore file bugs because it doesn't work. In this case we're actually deleting code and (bad) assumptions without actually paying anything (ok, very little cf. waking up once a second, on the second) - additionally we gain the ability to properly interoperate with the rest of the system, in particular with date(1).
Just commit it with the comment link please?
Not going to weigh in on this debate, but are we sure that we're going to pick up /etc/localtime if we call: g_date_time_format(g_date_time_new_now_local()) once a second? Which is essentially what we'll be doing if we land my patch in bug 643350. (Right now, we are at the mercies of whatever Spidermonkey does for a non-standard extension.) Looking at the code, no it won't - the timezone hash table will get a permanent entry for the NULL value of the TZ envvar.
(In reply to comment #16) > Not going to weigh in on this debate, but are we sure that we're going to pick > up /etc/localtime if we call: > > g_date_time_format(g_date_time_new_now_local()) > > once a second? Which is essentially what we'll be doing if we land my patch in > bug 643350. > > (Right now, we are at the mercies of whatever Spidermonkey does for a > non-standard extension.) > > Looking at the code, no it won't - the timezone hash table will get a permanent > entry for the NULL value of the TZ envvar. Actually, no, that won't work. Let me do a new patch on top of your patch?
Actually one reason to monitor suspend is that with your approach, it's almost certain the user sees the incorrect time for up to a second after suspend. We should queue a high priority idle right before suspend at least.
(In reply to comment #18) > Actually one reason to monitor suspend is that with your approach, it's almost > certain the user sees the incorrect time for up to a second after suspend. We > should queue a high priority idle right before suspend at least. What you see after unsuspend is the gnome screensaver unlock dialog. (Not inevitably, but by default.)
(In reply to comment #13) > (In reply to comment #12) > > Worrying about waking up the compositor once per second seems a bit silly. > > I suppose if we're VT switched away, the X server will block us, and if we're > logged in we're probably being used. The X server blocking the compositor is a bug. Doesn't seem like something to design for. Ajax had patches for this too at one point.
(In reply to comment #17) > (In reply to comment #16) > > Not going to weigh in on this debate, but are we sure that we're going to pick > > up /etc/localtime if we call: > > > > g_date_time_format(g_date_time_new_now_local()) > > > > once a second? Which is essentially what we'll be doing if we land my patch in > > bug 643350. > > > > (Right now, we are at the mercies of whatever Spidermonkey does for a > > non-standard extension.) > > > > Looking at the code, no it won't - the timezone hash table will get a permanent > > entry for the NULL value of the TZ envvar. > > Actually, no, that won't work. Let me do a new patch on top of your patch? Just tested and works fine with TZ changes with the patch from bug 643350 [1]. So I'm committing my patch as per Colin's comment 15 ("Just commit it with the comment link please?") - Owen's patch applies cleanly on top of this. Thanks. [1] : Clock updates to correct time within a single second of changing the timezone.. that's good enough for me.. if people want more precision than this then either GNOME system settings can poke the shell process or the shell process can monitor something or whatever
Comment on attachment 183344 [details] [review] Proposed patch http://git.gnome.org/browse/gnome-shell/commit/?id=4b2d6f8a992e55027dcffba8f6c7aac2318522a8
Just for good measure I've filed bug 644814 for optimizing this. If anything, it can serve as a place to point people complaining about the shell waking up once a second. Obviously this is post-3.0 stuff.