Merge lp:~mhr3/unity/fix-1058619 into lp:unity

Proposed by Michal Hruby
Status: Merged
Approved by: Michal Hruby
Approved revision: no longer in the source branch.
Merged at revision: 2799
Proposed branch: lp:~mhr3/unity/fix-1058619
Merge into: lp:unity
Diff against target: 86 lines (+23/-5)
1 file modified
UnityCore/GLibDBusProxy.cpp (+23/-5)
To merge this branch: bzr merge lp:~mhr3/unity/fix-1058619
Reviewer Review Type Date Requested Status
Gord Allott (community) Approve
Review via email: mp+127955@code.launchpad.net

Commit message

Attempt to reconnect to DBus proxies if the initial connection fails

Description of the change

Currently if connection to DBus proxy fails for whatever reason (most commonly timeout), the proxy object gives up and we can't talk to the service. This patch attempts to reconnect to the proxy up to 5 times and only after that gives up.

As this is very specific edge case it can't really be unit tested (we can't force dbus-daemon to timeout when starting a service), plus even manual test would require editing system files to ensure that a service cannot be started.

FWIW the way I tested this was to edit photo lens's dbus service file and change the Exec line to /bin/cat (cat doesn't exit immediately and therefore dbus-daemon times out the request eventually).

To post a comment you must log in.
Revision history for this message
Gord Allott (gordallott) wrote :

+1 from me

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'UnityCore/GLibDBusProxy.cpp'
2--- UnityCore/GLibDBusProxy.cpp 2012-09-05 15:45:00 +0000
3+++ UnityCore/GLibDBusProxy.cpp 2012-10-04 08:42:24 +0000
4@@ -36,6 +36,7 @@
5
6 namespace
7 {
8+const unsigned MAX_RECONNECTION_ATTEMPTS = 5;
9 nux::logging::Logger logger("unity.glib.dbusproxy");
10 }
11
12@@ -55,7 +56,7 @@
13 GDBusProxyFlags flags);
14 ~Impl();
15
16- void StartReconnectionTimeout();
17+ void StartReconnectionTimeout(unsigned timeout);
18 void Connect();
19
20 void Call(string const& method_name,
21@@ -91,6 +92,7 @@
22 glib::Object<GDBusProxy> proxy_;
23 glib::Object<GCancellable> cancellable_;
24 bool connected_;
25+ unsigned reconnection_attempts_;
26
27 glib::Signal<void, GDBusProxy*, char*, char*, GVariant*> g_signal_connection_;
28 glib::Signal<void, GDBusProxy*, GParamSpec*> name_owner_signal_;
29@@ -113,8 +115,9 @@
30 , flags_(flags)
31 , cancellable_(g_cancellable_new())
32 , connected_(false)
33+ , reconnection_attempts_(0)
34 {
35- StartReconnectionTimeout();
36+ StartReconnectionTimeout(1);
37 }
38
39 DBusProxy::Impl::~Impl()
40@@ -122,7 +125,7 @@
41 g_cancellable_cancel(cancellable_);
42 }
43
44-void DBusProxy::Impl::StartReconnectionTimeout()
45+void DBusProxy::Impl::StartReconnectionTimeout(unsigned timeout)
46 {
47 LOG_DEBUG(logger) << "Starting reconnection timeout for " << name_;
48
49@@ -134,7 +137,7 @@
50 return false;
51 };
52
53- reconnect_timeout_.reset(new glib::TimeoutSeconds(1, callback));
54+ reconnect_timeout_.reset(new glib::TimeoutSeconds(timeout, callback));
55 }
56
57 void DBusProxy::Impl::Connect()
58@@ -169,12 +172,27 @@
59 // therefore we should deal with the error before touching the impl pointer
60 if (!proxy || error)
61 {
62- LOG_WARNING(logger) << "Unable to connect to proxy: " << error;
63+ // if the cancellable was cancelled, "self" points to destroyed object
64+ if (error && !g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
65+ {
66+ if (self->reconnection_attempts_++ < MAX_RECONNECTION_ATTEMPTS)
67+ {
68+ LOG_WARNING(logger) << "Unable to connect to proxy: \"" << error
69+ << "\"... Trying to reconnect (attempt "
70+ << self->reconnection_attempts_ << ")";
71+ self->StartReconnectionTimeout(3);
72+ }
73+ else
74+ {
75+ LOG_ERROR(logger) << "Unable to connect to proxy: " << error;
76+ }
77+ }
78 return;
79 }
80
81 LOG_DEBUG(logger) << "Sucessfully created proxy: " << self->object_path_;
82
83+ self->reconnection_attempts_ = 0;
84 self->proxy_ = proxy;
85 self->g_signal_connection_.Connect(self->proxy_, "g-signal",
86 sigc::mem_fun(self, &Impl::OnProxySignal));