diff options
| author | Patrick Ohly <patrick.ohly@intel.com> | 2010-03-23 15:38:32 +0100 |
|---|---|---|
| committer | Patrick Ohly <patrick.ohly@intel.com> | 2010-03-24 08:25:08 +0100 |
| commit | 17cf1f410a60a944b56378a190baa564b533e9c2 (patch) | |
| tree | 9eeeb60258fd0dc425732a9fff1643cfe11161e0 | |
| parent | 73479a427ece68d5d921edfac7fc1a8263a767fa (diff) | |
syncevo-dbus-server: avoid regular wakeups (MB #10350)mb10350
The AutoTerm class, responsible for implementing the "terminate server
when idle for x seconds", woke up the process every five seconds to
check whether x seconds had passed since the last D-Bus call. The
regular wakeups increase power consumption and should be avoided.
This patch does that as follows:
- if the server has clients, there is no need for wakeups
because the server is obviously in use
- if it has no clients, sleep for x seconds since the last
D-Bus call (note that ref()/unref() imply D-Bus activity)
- instead of adding/removing the timeout on each D-Bus call,
let the pending one expire and set another one if there has
been activity since the current timeout was created
"No idle timeout" is treated like "server always has one client".
In addition, this patch cleans up a few things:
- remove the timeout when destructing the class; otherwise
it might still fire when the main loop is invoked again
for whatever reason and then segfault when using a stale
pointer
- don't typecast checkCallback() - that hides problems should
that function not match the expected prototype
glib's g_source_attach() does not check for guint ID
overflows. Because we add a timeout at most once per second (in the
unlikely case of 1 second idle timeout and permanent D-Bus activity),
we don't have to worry about that.
This patch passed both manual tests and test-dbus.py
TestDBusServerTerm. There was a missing "m_checkSource = 0" initially,
which was found by failures in TestDBusServerTerm - good to have these!
| -rw-r--r-- | src/syncevo-dbus-server.cpp | 95 |
1 files changed, 62 insertions, 33 deletions
diff --git a/src/syncevo-dbus-server.cpp b/src/syncevo-dbus-server.cpp index b55d6bc7..df43ca1e 100644 --- a/src/syncevo-dbus-server.cpp +++ b/src/syncevo-dbus-server.cpp @@ -253,31 +253,33 @@ template<> struct dbus_traits<ReadOperations::SourceDatabase> : */ class AutoTerm { int m_refs; - guint m_elapsed; guint m_interval; guint m_checkSource; + time_t m_lastUsed; - static const guint TERM_INTERVAL = 5; - - /* A callback is called in each TERM_INTERVAL seconds, registered to check whether the - * dbus server is timeout regurally. - * In reality it will be called after TERM_INTERVAL + delta, with delta being large - * if the D-Bus server is busy. Therefore m_elapsed will underestimate the real - * elapsed time. But because this only happens in a busy server and a busy server - * doesn't have to auto-terminate, this assumption could work. + /** + * This callback is called as soon as we might have to terminate. + * If it finds that the server has been used in the meantime, it + * will simply set another timeout and check again later. */ static gboolean checkCallback(gpointer data) { AutoTerm *at = static_cast<AutoTerm*>(data); - // if no conncetions or attached clients - if(at->m_refs <= 0) { - at->m_elapsed += TERM_INTERVAL; - if(at->m_elapsed >= at->m_interval) { + if (!at->m_refs) { + // currently idle, but also long enough? + time_t now = time(NULL); + if (at->m_lastUsed + at->m_interval <= now) { + // yes, shut down event loop and daemon shutdownRequested = true; g_main_loop_quit(loop); - return FALSE; + } else { + // check again later + at->m_checkSource = g_timeout_add_seconds(at->m_lastUsed + at->m_interval - now, + checkCallback, + data); } } - return TRUE; + // always remove the current timeout, its job is done + return FALSE; } public: @@ -285,39 +287,66 @@ class AutoTerm { * constructor * If interval is less than 0, it means 'unlimited' and never terminate */ - AutoTerm(int interval) : m_refs(0), m_elapsed(0) { - if(interval <= 0) { - ref(); - m_interval = 0; - m_checkSource = 0; - } else { - m_interval = interval; - // call checking every 5 seconds - // here we don't use add/remove new sources for we might - // make glib source id integer overflow since its id calculation - // only plus one each time - m_checkSource = g_timeout_add_seconds(TERM_INTERVAL, - (GSourceFunc) checkCallback, - static_cast<gpointer>(this)); - } + AutoTerm(int interval) : + m_refs(0), + m_interval(interval), + m_checkSource(0), + m_lastUsed(0) + { + if (interval <= 0) { + // increasing reference counts prevents shutdown forever + ref(); + } + reset(); + } + + ~AutoTerm() + { + if (m_checkSource) { + g_source_remove(m_checkSource); } + } //increase the actives objects void ref(int refs = 1) { m_refs += refs; + reset(); } //decrease the actives objects void unref(int refs = 1) { m_refs -= refs; if(m_refs <= 0) { - reset(); m_refs = 0; } + reset(); } - void reset() { - m_elapsed = 0; + /** + * To be called each time the server interacts with a client, + * which includes adding or removing a client. If necessary, + * this installs a timeout to stop the daemon when it has been + * idle long enough. + */ + void reset() + { + if (m_refs > 0) { + // in use, don't need timeout + if (m_checkSource) { + g_source_remove(m_checkSource); + m_checkSource = 0; + } + } else { + // An already active timeout will trigger at the chosen time, + // then notice that the server has been used in the meantime and + // reset the timer. Therefore we don't have to remove it. + m_lastUsed = time(NULL); + if (!m_checkSource) { + m_checkSource = g_timeout_add_seconds(m_interval, + checkCallback, + static_cast<gpointer>(this)); + } + } } }; |