Merge lp://qastaging/~ted/libindicator/service-restart into lp://qastaging/libindicator/0.4

Proposed by Ted Gould
Status: Merged
Merged at revision: not available
Proposed branch: lp://qastaging/~ted/libindicator/service-restart
Merge into: lp://qastaging/libindicator/0.4
Diff against target: 158 lines (+72/-0)
1 file modified
libindicator/indicator-service-manager.c (+72/-0)
To merge this branch: bzr merge lp://qastaging/~ted/libindicator/service-restart
Reviewer Review Type Date Requested Status
Cody Russell (community) Approve
Review via email: mp+17502@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
Ted Gould (ted) wrote :

This code makes it so that services will restart under a variety of
conditions. Mostly commonly, when they crash. It does a very simple
exponential backoff with how fast it restarts them.

346. By Ted Gould

Handling the connected signal as well, making sure we emit it.

347. By Ted Gould

It's a fundamental mistake to believe that we can protect people using
this interface from the disconnection. We have no information to say
that the new service starting will come up in the same state as the one
before it. We need the individual implementers to verify that. Now we
need to fix that. This commit does so.

Revision history for this message
Cody Russell (bratsche) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libindicator/indicator-service-manager.c'
2--- libindicator/indicator-service-manager.c 2009-12-07 20:22:35 +0000
3+++ libindicator/indicator-service-manager.c 2010-01-16 04:33:13 +0000
4@@ -25,6 +25,8 @@
5 #include "config.h"
6 #endif
7
8+#include <stdlib.h>
9+
10 #include <dbus/dbus-glib-bindings.h>
11 #include <dbus/dbus-glib-lowlevel.h>
12
13@@ -41,6 +43,7 @@
14 @connected: Whether we're connected to the service or not.
15 @this_service_version: The version of the service that we're looking for.
16 @bus: A reference to the bus so we don't have to keep getting it.
17+ @restart_count: The number of times we've restarted this service.
18 */
19 typedef struct _IndicatorServiceManagerPrivate IndicatorServiceManagerPrivate;
20 struct _IndicatorServiceManagerPrivate {
21@@ -50,6 +53,7 @@
22 gboolean connected;
23 guint this_service_version;
24 DBusGConnection * bus;
25+ guint restart_count;
26 };
27
28 /* Signals Stuff */
29@@ -60,6 +64,9 @@
30
31 static guint signals[LAST_SIGNAL] = { 0 };
32
33+/* If this env variable is set, we don't restart */
34+#define TIMEOUT_ENV_NAME "INDICATOR_SERVICE_RESTART_DISABLE"
35+#define TIMEOUT_MULTIPLIER 100 /* In ms */
36
37 /* Properties */
38 /* Enum for the properties so that they can be quickly
39@@ -86,7 +93,9 @@
40 /* Prototypes */
41 static void set_property (GObject * object, guint prop_id, const GValue * value, GParamSpec * pspec);
42 static void get_property (GObject * object, guint prop_id, GValue * value, GParamSpec * pspec);
43+static void service_proxy_destroyed (DBusGProxy * proxy, gpointer user_data);
44 static void start_service (IndicatorServiceManager * service);
45+static void start_service_again (IndicatorServiceManager * manager);
46
47 G_DEFINE_TYPE (IndicatorServiceManager, indicator_service_manager, G_TYPE_OBJECT);
48
49@@ -154,6 +163,7 @@
50 priv->connected = FALSE;
51 priv->this_service_version = 0;
52 priv->bus = NULL;
53+ priv->restart_count = 0;
54
55 /* Start talkin' dbus */
56 GError * error = NULL;
57@@ -312,6 +322,12 @@
58 return;
59 }
60
61+ /* We've done it, now let's stop counting. */
62+ /* Note: we're not checking versions. Because, the hope is that
63+ the guy holding the name we want with the wrong version will
64+ drop and we can start another service quickly. */
65+ priv->restart_count = 0;
66+
67 if (service_api_version != INDICATOR_SERVICE_VERSION) {
68 g_warning("Service is using a different version of the service interface. Expecting %d and got %d.", INDICATOR_SERVICE_VERSION, service_api_version);
69 dbus_g_proxy_call_no_reply(priv->service_proxy, "UnWatch", G_TYPE_INVALID);
70@@ -343,11 +359,13 @@
71
72 if (error != NULL) {
73 g_warning("Unable to start service '%s': %s", priv->name, error->message);
74+ start_service_again(INDICATOR_SERVICE_MANAGER(user_data));
75 return;
76 }
77
78 if (status != DBUS_START_REPLY_SUCCESS && status != DBUS_START_REPLY_ALREADY_RUNNING) {
79 g_warning("Status of starting the process '%s' was an error: %d", priv->name, status);
80+ start_service_again(INDICATOR_SERVICE_MANAGER(user_data));
81 return;
82 }
83
84@@ -358,6 +376,7 @@
85 INDICATOR_SERVICE_INTERFACE,
86 &error);
87 g_object_add_weak_pointer(G_OBJECT(priv->service_proxy), (gpointer *)&(priv->service_proxy));
88+ g_signal_connect(G_OBJECT(priv->service_proxy), "destroy", G_CALLBACK(service_proxy_destroyed), user_data);
89
90 org_ayatana_indicator_service_watch_async(priv->service_proxy,
91 watch_cb,
92@@ -396,6 +415,7 @@
93 service);
94 } else {
95 g_object_add_weak_pointer(G_OBJECT(priv->service_proxy), (gpointer *)&(priv->service_proxy));
96+ g_signal_connect(G_OBJECT(priv->service_proxy), "destroy", G_CALLBACK(service_proxy_destroyed), service);
97
98 /* If we got a proxy just because we're good people then
99 we need to call watch on it just like 'start_service_cb'
100@@ -408,6 +428,58 @@
101 return;
102 }
103
104+/* Responds to the destory event of the proxy and starts
105+ setting up to restart the service. */
106+static void
107+service_proxy_destroyed (DBusGProxy * proxy, gpointer user_data)
108+{
109+ IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(user_data);
110+ if (priv->connected) {
111+ priv->connected = FALSE;
112+ g_signal_emit(G_OBJECT(user_data), signals[CONNECTION_CHANGE], 0, FALSE, TRUE);
113+ }
114+ return start_service_again(INDICATOR_SERVICE_MANAGER(user_data));
115+}
116+
117+/* The callback that starts the service for real after
118+ the timeout as determined in 'start_service_again'.
119+ This could be in the idle or a timer. */
120+static gboolean
121+start_service_again_cb (gpointer data)
122+{
123+ IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(data);
124+ priv->restart_count++;
125+ start_service(INDICATOR_SERVICE_MANAGER(data));
126+ return FALSE;
127+}
128+
129+/* This function tries to start a new service, perhaps
130+ after a timeout that it determines. The real issue
131+ here is that it throttles restarting if we're not
132+ being successful. */
133+static void
134+start_service_again (IndicatorServiceManager * manager)
135+{
136+ IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(manager);
137+
138+ /* Allow the restarting to be disabled */
139+ if (g_getenv(TIMEOUT_ENV_NAME)) {
140+ return;
141+ }
142+
143+ if (priv->restart_count == 0) {
144+ /* First time, do it in idle */
145+ g_idle_add(start_service_again_cb, manager);
146+ } else {
147+ /* Not our first time 'round the block. Let's slow this down. */
148+ if (priv->restart_count > 16)
149+ priv->restart_count = 16; /* Not more than 1024x */
150+ g_timeout_add((1 << priv->restart_count) * TIMEOUT_MULTIPLIER, start_service_again_cb, manager);
151+ }
152+
153+ return;
154+}
155+
156 /* API */
157
158 /**

Subscribers

People subscribed via source and target branches