Bug 25624

Tom: thanks. This patch is smaller than what you had posted on the mailing list. Can you check if you attached the proper thing?

I left out the part to pthread_cond_timedwait() because that change has already been made with commit ae24bb35e2ee3ecde990f55852982b573754ec43.

Understood.

I'd like to take this next stable cycle, two quick questions:

* Is there any precedent for other projects in the OS core using the monotonic clock?  If so code references would be useful.  I think I saw a patch to glib go by, will have to investigate that.
* Have you looked at all the callers of _dbus_get_current_time?  It looks in a quick glance that all of them should be using this but needs solid investigation.

> * Is there any precedent for other projects in the OS core using the monotonic
> clock?  If so code references would be useful.  I think I saw a patch to glib
> go by, will have to investigate that.

I'm not sure offhand; a lot of core components are doing the same thing as dbus.

> * Have you looked at all the callers of _dbus_get_current_time?  It looks in a
> quick glance that all of them should be using this but needs solid
> investigation.

One thing to double-check is whether the functions that use _dbus_get_current_time expect the time to include the time that passed while suspended. With Linux (at least 2.6.24), the time spent suspended is not added to the monotonic clock. I'm not sure what POSIX says about this, so I don't know how other platforms behave.

Qt's timers are all monotonic-based, even when linking to Glib and using its event loop.

There are some places where the usual gettimeofday() timing is used because of convenience (the QTime class), but it's something I want to fix to use the monotonic timer too.

I think that convention is that if you had a timer that hadn't expired before suspending, when you resume, it continues not expired. The reverse of that would be that many applications get hundreds of expired timers when the system resumes from suspend and eventually try to execute too much at that time.

Ok I looked through all the callers of _dbus_get_current_time more carefully, and using the monotonic clock is either clearly right (connection timeouts) or moot (generating a uuid).

For general reference there's a patch to GLib to use a monotonic clock for timeouts here:

https://bugzilla.gnome.org/show_bug.cgi?id=540545

Thiago, completely agree about the suspend behavior.

I'm pushing this patch to dbus-1.2 now.  Thanks a lot for the patch Tom!

Two small changes; first I changed the #ifdef section to just be "#ifdef HAVE_MONOTONIC_CLOCK" since we already define that in configure.ac, and this also matches the GLib patch. 

Second I added a space before the parens.

Now it's too late, but I was going to suggest that this is an invasive change and should go into master only (not 1.2).

Agree it's a big change - however, it is really important for reliability (everyone suspends their linux laptops), it's been running in the field on several linux-based operating systems already, so I decided it's worth taking in 1.2.

The question to ask is if this change will change API/ABI or behavior someone is expecting.  Since clearly we expect timers to not expire when a user changes the system clock or wake up from a suspend it should go into stable.  It is a bug fix, not a whole new feature.  Now if there is some side effect that will break major D-Bus users there might be consideration to keep it out of stable but if anyone is actually relying on timeouts reacting to users changing the time, those apps are clearly broken and should be fixed.