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
=== modified file 'libindicator/indicator-service-manager.c'
--- libindicator/indicator-service-manager.c 2009-12-07 20:22:35 +0000
+++ libindicator/indicator-service-manager.c 2010-01-16 04:33:13 +0000
@@ -25,6 +25,8 @@
25#include "config.h"25#include "config.h"
26#endif26#endif
2727
28#include <stdlib.h>
29
28#include <dbus/dbus-glib-bindings.h>30#include <dbus/dbus-glib-bindings.h>
29#include <dbus/dbus-glib-lowlevel.h>31#include <dbus/dbus-glib-lowlevel.h>
3032
@@ -41,6 +43,7 @@
41 @connected: Whether we're connected to the service or not.43 @connected: Whether we're connected to the service or not.
42 @this_service_version: The version of the service that we're looking for.44 @this_service_version: The version of the service that we're looking for.
43 @bus: A reference to the bus so we don't have to keep getting it.45 @bus: A reference to the bus so we don't have to keep getting it.
46 @restart_count: The number of times we've restarted this service.
44*/47*/
45typedef struct _IndicatorServiceManagerPrivate IndicatorServiceManagerPrivate;48typedef struct _IndicatorServiceManagerPrivate IndicatorServiceManagerPrivate;
46struct _IndicatorServiceManagerPrivate {49struct _IndicatorServiceManagerPrivate {
@@ -50,6 +53,7 @@
50 gboolean connected;53 gboolean connected;
51 guint this_service_version;54 guint this_service_version;
52 DBusGConnection * bus;55 DBusGConnection * bus;
56 guint restart_count;
53};57};
5458
55/* Signals Stuff */59/* Signals Stuff */
@@ -60,6 +64,9 @@
6064
61static guint signals[LAST_SIGNAL] = { 0 };65static guint signals[LAST_SIGNAL] = { 0 };
6266
67/* If this env variable is set, we don't restart */
68#define TIMEOUT_ENV_NAME "INDICATOR_SERVICE_RESTART_DISABLE"
69#define TIMEOUT_MULTIPLIER 100 /* In ms */
6370
64/* Properties */71/* Properties */
65/* Enum for the properties so that they can be quickly72/* Enum for the properties so that they can be quickly
@@ -86,7 +93,9 @@
86/* Prototypes */93/* Prototypes */
87static void set_property (GObject * object, guint prop_id, const GValue * value, GParamSpec * pspec);94static void set_property (GObject * object, guint prop_id, const GValue * value, GParamSpec * pspec);
88static void get_property (GObject * object, guint prop_id, GValue * value, GParamSpec * pspec);95static void get_property (GObject * object, guint prop_id, GValue * value, GParamSpec * pspec);
96static void service_proxy_destroyed (DBusGProxy * proxy, gpointer user_data);
89static void start_service (IndicatorServiceManager * service);97static void start_service (IndicatorServiceManager * service);
98static void start_service_again (IndicatorServiceManager * manager);
9099
91G_DEFINE_TYPE (IndicatorServiceManager, indicator_service_manager, G_TYPE_OBJECT);100G_DEFINE_TYPE (IndicatorServiceManager, indicator_service_manager, G_TYPE_OBJECT);
92101
@@ -154,6 +163,7 @@
154 priv->connected = FALSE;163 priv->connected = FALSE;
155 priv->this_service_version = 0;164 priv->this_service_version = 0;
156 priv->bus = NULL;165 priv->bus = NULL;
166 priv->restart_count = 0;
157167
158 /* Start talkin' dbus */168 /* Start talkin' dbus */
159 GError * error = NULL;169 GError * error = NULL;
@@ -312,6 +322,12 @@
312 return;322 return;
313 }323 }
314324
325 /* We've done it, now let's stop counting. */
326 /* Note: we're not checking versions. Because, the hope is that
327 the guy holding the name we want with the wrong version will
328 drop and we can start another service quickly. */
329 priv->restart_count = 0;
330
315 if (service_api_version != INDICATOR_SERVICE_VERSION) {331 if (service_api_version != INDICATOR_SERVICE_VERSION) {
316 g_warning("Service is using a different version of the service interface. Expecting %d and got %d.", INDICATOR_SERVICE_VERSION, service_api_version);332 g_warning("Service is using a different version of the service interface. Expecting %d and got %d.", INDICATOR_SERVICE_VERSION, service_api_version);
317 dbus_g_proxy_call_no_reply(priv->service_proxy, "UnWatch", G_TYPE_INVALID);333 dbus_g_proxy_call_no_reply(priv->service_proxy, "UnWatch", G_TYPE_INVALID);
@@ -343,11 +359,13 @@
343359
344 if (error != NULL) {360 if (error != NULL) {
345 g_warning("Unable to start service '%s': %s", priv->name, error->message);361 g_warning("Unable to start service '%s': %s", priv->name, error->message);
362 start_service_again(INDICATOR_SERVICE_MANAGER(user_data));
346 return;363 return;
347 }364 }
348365
349 if (status != DBUS_START_REPLY_SUCCESS && status != DBUS_START_REPLY_ALREADY_RUNNING) {366 if (status != DBUS_START_REPLY_SUCCESS && status != DBUS_START_REPLY_ALREADY_RUNNING) {
350 g_warning("Status of starting the process '%s' was an error: %d", priv->name, status);367 g_warning("Status of starting the process '%s' was an error: %d", priv->name, status);
368 start_service_again(INDICATOR_SERVICE_MANAGER(user_data));
351 return;369 return;
352 }370 }
353371
@@ -358,6 +376,7 @@
358 INDICATOR_SERVICE_INTERFACE,376 INDICATOR_SERVICE_INTERFACE,
359 &error);377 &error);
360 g_object_add_weak_pointer(G_OBJECT(priv->service_proxy), (gpointer *)&(priv->service_proxy));378 g_object_add_weak_pointer(G_OBJECT(priv->service_proxy), (gpointer *)&(priv->service_proxy));
379 g_signal_connect(G_OBJECT(priv->service_proxy), "destroy", G_CALLBACK(service_proxy_destroyed), user_data);
361380
362 org_ayatana_indicator_service_watch_async(priv->service_proxy,381 org_ayatana_indicator_service_watch_async(priv->service_proxy,
363 watch_cb,382 watch_cb,
@@ -396,6 +415,7 @@
396 service);415 service);
397 } else {416 } else {
398 g_object_add_weak_pointer(G_OBJECT(priv->service_proxy), (gpointer *)&(priv->service_proxy));417 g_object_add_weak_pointer(G_OBJECT(priv->service_proxy), (gpointer *)&(priv->service_proxy));
418 g_signal_connect(G_OBJECT(priv->service_proxy), "destroy", G_CALLBACK(service_proxy_destroyed), service);
399419
400 /* If we got a proxy just because we're good people then420 /* If we got a proxy just because we're good people then
401 we need to call watch on it just like 'start_service_cb'421 we need to call watch on it just like 'start_service_cb'
@@ -408,6 +428,58 @@
408 return;428 return;
409}429}
410430
431/* Responds to the destory event of the proxy and starts
432 setting up to restart the service. */
433static void
434service_proxy_destroyed (DBusGProxy * proxy, gpointer user_data)
435{
436 IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(user_data);
437 if (priv->connected) {
438 priv->connected = FALSE;
439 g_signal_emit(G_OBJECT(user_data), signals[CONNECTION_CHANGE], 0, FALSE, TRUE);
440 }
441 return start_service_again(INDICATOR_SERVICE_MANAGER(user_data));
442}
443
444/* The callback that starts the service for real after
445 the timeout as determined in 'start_service_again'.
446 This could be in the idle or a timer. */
447static gboolean
448start_service_again_cb (gpointer data)
449{
450 IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(data);
451 priv->restart_count++;
452 start_service(INDICATOR_SERVICE_MANAGER(data));
453 return FALSE;
454}
455
456/* This function tries to start a new service, perhaps
457 after a timeout that it determines. The real issue
458 here is that it throttles restarting if we're not
459 being successful. */
460static void
461start_service_again (IndicatorServiceManager * manager)
462{
463 IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(manager);
464
465 /* Allow the restarting to be disabled */
466 if (g_getenv(TIMEOUT_ENV_NAME)) {
467 return;
468 }
469
470 if (priv->restart_count == 0) {
471 /* First time, do it in idle */
472 g_idle_add(start_service_again_cb, manager);
473 } else {
474 /* Not our first time 'round the block. Let's slow this down. */
475 if (priv->restart_count > 16)
476 priv->restart_count = 16; /* Not more than 1024x */
477 g_timeout_add((1 << priv->restart_count) * TIMEOUT_MULTIPLIER, start_service_again_cb, manager);
478 }
479
480 return;
481}
482
411/* API */483/* API */
412484
413/**485/**

Subscribers

People subscribed via source and target branches