From 41559b3cc70cffacc023a6a21cca41b5b1070447 Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Fri, 15 Jan 2010 19:54:42 -0600 Subject: Getting a multiplier, and providing a way to override it using an environment variable. --- libindicator/indicator-service-manager.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/libindicator/indicator-service-manager.c b/libindicator/indicator-service-manager.c index 69b1343..ee29767 100644 --- a/libindicator/indicator-service-manager.c +++ b/libindicator/indicator-service-manager.c @@ -25,6 +25,8 @@ License along with this library. If not, see #include "config.h" #endif +#include + #include #include @@ -60,6 +62,10 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; +/* We multiply our timeouts by in ms. This can be overriden by + the environment variable INDICATOR_SERVICE_RESTART_TIMEOUT. */ +static guint timeout_multiplier = 100; +#define TIMEOUT_ENV_NAME "INDICATOR_SERVICE_RESTART_TIMEOUT" /* Properties */ /* Enum for the properties so that they can be quickly @@ -136,6 +142,15 @@ indicator_service_manager_class_init (IndicatorServiceManagerClass *klass) 0, G_MAXUINT, 0, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); + /* Look to see if there is an environment variable effecting + the restart interval. This is in the class init as it should + only happen once. */ + const gchar * restart_env = g_getenv(TIMEOUT_ENV_NAME); + if (restart_env != NULL) { + timeout_multiplier = atoi(restart_env); + g_log(G_LOG_DOMAIN, G_LOG_LEVEL_INFO, "Time out multipler set to: %d", timeout_multiplier); + } + return; } -- cgit v1.2.3 From b9344c35f45a52ab15462746998d1b11b63e5928 Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Fri, 15 Jan 2010 20:38:04 -0600 Subject: Reset the restart_count when we start, and start to bring in 'start_service_again' to begin to throttle the restarts. --- libindicator/indicator-service-manager.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/libindicator/indicator-service-manager.c b/libindicator/indicator-service-manager.c index ee29767..6c6dca9 100644 --- a/libindicator/indicator-service-manager.c +++ b/libindicator/indicator-service-manager.c @@ -43,6 +43,7 @@ License along with this library. If not, see @connected: Whether we're connected to the service or not. @this_service_version: The version of the service that we're looking for. @bus: A reference to the bus so we don't have to keep getting it. + @restart_count: The number of times we've restarted this service. */ typedef struct _IndicatorServiceManagerPrivate IndicatorServiceManagerPrivate; struct _IndicatorServiceManagerPrivate { @@ -52,6 +53,7 @@ struct _IndicatorServiceManagerPrivate { gboolean connected; guint this_service_version; DBusGConnection * bus; + guint restart_count; }; /* Signals Stuff */ @@ -93,6 +95,7 @@ static void indicator_service_manager_finalize (GObject *object); static void set_property (GObject * object, guint prop_id, const GValue * value, GParamSpec * pspec); static void get_property (GObject * object, guint prop_id, GValue * value, GParamSpec * pspec); static void start_service (IndicatorServiceManager * service); +static void start_service_again (IndicatorServiceManager * manager); G_DEFINE_TYPE (IndicatorServiceManager, indicator_service_manager, G_TYPE_OBJECT); @@ -169,6 +172,7 @@ indicator_service_manager_init (IndicatorServiceManager *self) priv->connected = FALSE; priv->this_service_version = 0; priv->bus = NULL; + priv->restart_count = 0; /* Start talkin' dbus */ GError * error = NULL; @@ -327,6 +331,12 @@ watch_cb (DBusGProxy * proxy, guint service_api_version, guint this_service_vers return; } + /* We've done it, now let's stop counting. */ + /* Note: we're not checking versions. Because, the hope is that + the guy holding the name we want with the wrong version will + drop and we can start another service quickly. */ + priv->restart_count = 0; + if (service_api_version != INDICATOR_SERVICE_VERSION) { g_warning("Service is using a different version of the service interface. Expecting %d and got %d.", INDICATOR_SERVICE_VERSION, service_api_version); dbus_g_proxy_call_no_reply(priv->service_proxy, "UnWatch", G_TYPE_INVALID); @@ -358,11 +368,13 @@ start_service_cb (DBusGProxy * proxy, guint status, GError * error, gpointer use if (error != NULL) { g_warning("Unable to start service '%s': %s", priv->name, error->message); + start_service_again(INDICATOR_SERVICE_MANAGER(user_data)); return; } if (status != DBUS_START_REPLY_SUCCESS && status != DBUS_START_REPLY_ALREADY_RUNNING) { g_warning("Status of starting the process '%s' was an error: %d", priv->name, status); + start_service_again(INDICATOR_SERVICE_MANAGER(user_data)); return; } @@ -423,6 +435,18 @@ start_service (IndicatorServiceManager * service) return; } +/* This function tries to start a new service, perhaps + after a timeout that it determines. The real issue + here is that it throttles restarting if we're not + being successful. */ +static void +start_service_again (IndicatorServiceManager * manager) +{ + + + return; +} + /* API */ /** -- cgit v1.2.3 From 2ce95b0279e75123beef7cdc1b440b03ffb56c55 Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Fri, 15 Jan 2010 21:15:30 -0600 Subject: Filling out the function to start it again, and adding in the function to respond to the timeout. --- libindicator/indicator-service-manager.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/libindicator/indicator-service-manager.c b/libindicator/indicator-service-manager.c index 6c6dca9..0c552d4 100644 --- a/libindicator/indicator-service-manager.c +++ b/libindicator/indicator-service-manager.c @@ -435,6 +435,18 @@ start_service (IndicatorServiceManager * service) return; } +/* The callback that starts the service for real after + the timeout as determined in 'start_service_again'. + This could be in the idle or a timer. */ +static gboolean +start_service_again_cb (gpointer data) +{ + IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(data); + priv->restart_count++; + start_service(INDICATOR_SERVICE_MANAGER(data)); + return FALSE; +} + /* This function tries to start a new service, perhaps after a timeout that it determines. The real issue here is that it throttles restarting if we're not @@ -442,7 +454,17 @@ start_service (IndicatorServiceManager * service) static void start_service_again (IndicatorServiceManager * manager) { + IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(manager); + if (priv->restart_count == 0) { + /* First time, do it in idle */ + g_idle_add(start_service_again_cb, manager); + } else { + /* Not our first time 'round the block. Let's slow this down. */ + if (priv->restart_count > 16) + priv->restart_count = 16; /* Not more than 1024x */ + g_timeout_add((1 << priv->restart_count) * timeout_multiplier, start_service_again_cb, manager); + } return; } -- cgit v1.2.3 From 9bccc146b8f1ee955d713fec30d7f5faf2ce88c1 Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Fri, 15 Jan 2010 21:25:48 -0600 Subject: Setting up the signal for when the proxies falls down, we restart. --- libindicator/indicator-service-manager.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/libindicator/indicator-service-manager.c b/libindicator/indicator-service-manager.c index 0c552d4..1a8db07 100644 --- a/libindicator/indicator-service-manager.c +++ b/libindicator/indicator-service-manager.c @@ -94,6 +94,7 @@ static void indicator_service_manager_finalize (GObject *object); /* Prototypes */ static void set_property (GObject * object, guint prop_id, const GValue * value, GParamSpec * pspec); static void get_property (GObject * object, guint prop_id, GValue * value, GParamSpec * pspec); +static void service_proxy_destroyed (DBusGProxy * proxy, gpointer user_data); static void start_service (IndicatorServiceManager * service); static void start_service_again (IndicatorServiceManager * manager); @@ -385,6 +386,7 @@ start_service_cb (DBusGProxy * proxy, guint status, GError * error, gpointer use INDICATOR_SERVICE_INTERFACE, &error); g_object_add_weak_pointer(G_OBJECT(priv->service_proxy), (gpointer *)&(priv->service_proxy)); + g_signal_connect(G_OBJECT(priv->service_proxy), "destroy", G_CALLBACK(service_proxy_destroyed), user_data); org_ayatana_indicator_service_watch_async(priv->service_proxy, watch_cb, @@ -423,6 +425,7 @@ start_service (IndicatorServiceManager * service) service); } else { g_object_add_weak_pointer(G_OBJECT(priv->service_proxy), (gpointer *)&(priv->service_proxy)); + g_signal_connect(G_OBJECT(priv->service_proxy), "destroy", G_CALLBACK(service_proxy_destroyed), service); /* If we got a proxy just because we're good people then we need to call watch on it just like 'start_service_cb' @@ -435,6 +438,14 @@ start_service (IndicatorServiceManager * service) return; } +/* Responds to the destory event of the proxy and starts + setting up to restart the service. */ +static void +service_proxy_destroyed (DBusGProxy * proxy, gpointer user_data) +{ + return start_service_again(INDICATOR_SERVICE_MANAGER(user_data)); +} + /* The callback that starts the service for real after the timeout as determined in 'start_service_again'. This could be in the idle or a timer. */ -- cgit v1.2.3 From deabfe64bf3b306c1c068665c4d8324dee7f5435 Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Fri, 15 Jan 2010 21:40:03 -0600 Subject: Switching what our enviroment variable is for, let's just stop the whole thing instead of playing with it. --- libindicator/indicator-service-manager.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/libindicator/indicator-service-manager.c b/libindicator/indicator-service-manager.c index 1a8db07..42160ee 100644 --- a/libindicator/indicator-service-manager.c +++ b/libindicator/indicator-service-manager.c @@ -64,10 +64,9 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; -/* We multiply our timeouts by in ms. This can be overriden by - the environment variable INDICATOR_SERVICE_RESTART_TIMEOUT. */ -static guint timeout_multiplier = 100; -#define TIMEOUT_ENV_NAME "INDICATOR_SERVICE_RESTART_TIMEOUT" +/* If this env variable is set, we don't restart */ +#define TIMEOUT_ENV_NAME "INDICATOR_SERVICE_RESTART_DISABLE" +#define TIMEOUT_MULTIPLIER 100 /* In ms */ /* Properties */ /* Enum for the properties so that they can be quickly @@ -146,15 +145,6 @@ indicator_service_manager_class_init (IndicatorServiceManagerClass *klass) 0, G_MAXUINT, 0, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); - /* Look to see if there is an environment variable effecting - the restart interval. This is in the class init as it should - only happen once. */ - const gchar * restart_env = g_getenv(TIMEOUT_ENV_NAME); - if (restart_env != NULL) { - timeout_multiplier = atoi(restart_env); - g_log(G_LOG_DOMAIN, G_LOG_LEVEL_INFO, "Time out multipler set to: %d", timeout_multiplier); - } - return; } @@ -465,6 +455,11 @@ start_service_again_cb (gpointer data) static void start_service_again (IndicatorServiceManager * manager) { + /* Allow the restarting to be disabled */ + if (g_getenv(TIMEOUT_ENV_NAME)) { + return; + } + IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(manager); if (priv->restart_count == 0) { @@ -474,7 +469,7 @@ start_service_again (IndicatorServiceManager * manager) /* Not our first time 'round the block. Let's slow this down. */ if (priv->restart_count > 16) priv->restart_count = 16; /* Not more than 1024x */ - g_timeout_add((1 << priv->restart_count) * timeout_multiplier, start_service_again_cb, manager); + g_timeout_add((1 << priv->restart_count) * TIMEOUT_MULTIPLIER, start_service_again_cb, manager); } return; -- cgit v1.2.3 From 62d8c1c86b21ab250342900324565f1b95ac2da8 Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Fri, 15 Jan 2010 22:16:45 -0600 Subject: Handling the connected signal as well, making sure we emit it. --- libindicator/indicator-service-manager.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libindicator/indicator-service-manager.c b/libindicator/indicator-service-manager.c index 42160ee..9d56a9f 100644 --- a/libindicator/indicator-service-manager.c +++ b/libindicator/indicator-service-manager.c @@ -455,18 +455,24 @@ start_service_again_cb (gpointer data) static void start_service_again (IndicatorServiceManager * manager) { + IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(manager); + /* Allow the restarting to be disabled */ if (g_getenv(TIMEOUT_ENV_NAME)) { + priv->connected = FALSE; + g_signal_emit(manager, signals[CONNECTION_CHANGE], 0, FALSE, TRUE); return; } - IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(manager); - if (priv->restart_count == 0) { /* First time, do it in idle */ g_idle_add(start_service_again_cb, manager); } else { /* Not our first time 'round the block. Let's slow this down. */ + if (priv->connected) { + priv->connected = FALSE; + g_signal_emit(manager, signals[CONNECTION_CHANGE], 0, FALSE, TRUE); + } if (priv->restart_count > 16) priv->restart_count = 16; /* Not more than 1024x */ g_timeout_add((1 << priv->restart_count) * TIMEOUT_MULTIPLIER, start_service_again_cb, manager); -- cgit v1.2.3 From 079c2b517deba95385ac47eea1fd6560aef3aacb Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Fri, 15 Jan 2010 22:28:13 -0600 Subject: 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. --- libindicator/indicator-service-manager.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/libindicator/indicator-service-manager.c b/libindicator/indicator-service-manager.c index 9d56a9f..891f49e 100644 --- a/libindicator/indicator-service-manager.c +++ b/libindicator/indicator-service-manager.c @@ -433,6 +433,11 @@ start_service (IndicatorServiceManager * service) static void service_proxy_destroyed (DBusGProxy * proxy, gpointer user_data) { + IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(user_data); + if (priv->connected) { + priv->connected = FALSE; + g_signal_emit(G_OBJECT(user_data), signals[CONNECTION_CHANGE], 0, FALSE, TRUE); + } return start_service_again(INDICATOR_SERVICE_MANAGER(user_data)); } @@ -459,8 +464,6 @@ start_service_again (IndicatorServiceManager * manager) /* Allow the restarting to be disabled */ if (g_getenv(TIMEOUT_ENV_NAME)) { - priv->connected = FALSE; - g_signal_emit(manager, signals[CONNECTION_CHANGE], 0, FALSE, TRUE); return; } @@ -469,10 +472,6 @@ start_service_again (IndicatorServiceManager * manager) g_idle_add(start_service_again_cb, manager); } else { /* Not our first time 'round the block. Let's slow this down. */ - if (priv->connected) { - priv->connected = FALSE; - g_signal_emit(manager, signals[CONNECTION_CHANGE], 0, FALSE, TRUE); - } if (priv->restart_count > 16) priv->restart_count = 16; /* Not more than 1024x */ g_timeout_add((1 << priv->restart_count) * TIMEOUT_MULTIPLIER, start_service_again_cb, manager); -- cgit v1.2.3