libindicator

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

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

This makes the merge_cb function handle the error conditions by trying
again. I think this will fix some issues that are being seen with lost
menus in that it's a race condition that "Watch" is being requested
before the interface is setup.

lp://qastaging/~ted/libindicator/watch-fail-restart updated
344. By Ted Gould

In case we're restarting because of the 'Watch' returning failure we'd have a valid 'service_proxy' object to kill

345. By Ted Gould

Adding in tracking of the restart idle function and making sure we don't do it twice.

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 2010-01-16 04:28:13 +0000
3+++ libindicator/indicator-service-manager.c 2010-01-21 16:32:12 +0000
4@@ -54,6 +54,7 @@
5 guint this_service_version;
6 DBusGConnection * bus;
7 guint restart_count;
8+ gint restart_source;
9 };
10
11 /* Signals Stuff */
12@@ -67,6 +68,10 @@
13 /* If this env variable is set, we don't restart */
14 #define TIMEOUT_ENV_NAME "INDICATOR_SERVICE_RESTART_DISABLE"
15 #define TIMEOUT_MULTIPLIER 100 /* In ms */
16+/* What to reset the restart_count to if we know that we're
17+ in a recoverable error condition, but waiting a little bit
18+ will probably make things better. 5 ~= 3 sec. */
19+#define TIMEOUT_A_LITTLE_WHILE 5
20
21 /* Properties */
22 /* Enum for the properties so that they can be quickly
23@@ -164,6 +169,7 @@
24 priv->this_service_version = 0;
25 priv->bus = NULL;
26 priv->restart_count = 0;
27+ priv->restart_source = 0;
28
29 /* Start talkin' dbus */
30 GError * error = NULL;
31@@ -197,6 +203,13 @@
32 {
33 IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(object);
34
35+ /* Removing the idle task to restart if it exists. */
36+ if (priv->restart_source != 0) {
37+ g_source_remove(priv->restart_source);
38+ }
39+ /* Block any restart calls */
40+ priv->restart_source = -1;
41+
42 /* If we were connected we need to make sure to
43 tell people that it's no longer the case. */
44 if (priv->connected) {
45@@ -319,6 +332,7 @@
46 if (error != NULL) {
47 g_warning("Unable to set watch on '%s': '%s'", priv->name, error->message);
48 g_error_free(error);
49+ start_service_again(INDICATOR_SERVICE_MANAGER(user_data));
50 return;
51 }
52
53@@ -331,12 +345,20 @@
54 if (service_api_version != INDICATOR_SERVICE_VERSION) {
55 g_warning("Service is using a different version of the service interface. Expecting %d and got %d.", INDICATOR_SERVICE_VERSION, service_api_version);
56 dbus_g_proxy_call_no_reply(priv->service_proxy, "UnWatch", G_TYPE_INVALID);
57+
58+ /* Let's make us wait a little while, then try again */
59+ priv->restart_count = TIMEOUT_A_LITTLE_WHILE;
60+ start_service_again(INDICATOR_SERVICE_MANAGER(user_data));
61 return;
62 }
63
64 if (this_service_version != priv->this_service_version) {
65 g_warning("Service is using a different API version than the manager. Expecting %d and got %d.", priv->this_service_version, this_service_version);
66 dbus_g_proxy_call_no_reply(priv->service_proxy, "UnWatch", G_TYPE_INVALID);
67+
68+ /* Let's make us wait a little while, then try again */
69+ priv->restart_count = TIMEOUT_A_LITTLE_WHILE;
70+ start_service_again(INDICATOR_SERVICE_MANAGER(user_data));
71 return;
72 }
73
74@@ -398,6 +420,11 @@
75 g_return_if_fail(priv->dbus_proxy != NULL);
76 g_return_if_fail(priv->name != NULL);
77
78+ if (priv->service_proxy != NULL) {
79+ g_object_unref(priv->service_proxy);
80+ priv->service_proxy = NULL;
81+ }
82+
83 /* Check to see if we can get a proxy to it first. */
84 priv->service_proxy = dbus_g_proxy_new_for_name_owner(priv->bus,
85 priv->name,
86@@ -450,6 +477,7 @@
87 IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(data);
88 priv->restart_count++;
89 start_service(INDICATOR_SERVICE_MANAGER(data));
90+ priv->restart_source = 0;
91 return FALSE;
92 }
93
94@@ -462,6 +490,12 @@
95 {
96 IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(manager);
97
98+ /* If we've already got a restart source running then
99+ let's not do this again. */
100+ if (priv->restart_source != 0) {
101+ return;
102+ }
103+
104 /* Allow the restarting to be disabled */
105 if (g_getenv(TIMEOUT_ENV_NAME)) {
106 return;
107@@ -474,7 +508,7 @@
108 /* Not our first time 'round the block. Let's slow this down. */
109 if (priv->restart_count > 16)
110 priv->restart_count = 16; /* Not more than 1024x */
111- g_timeout_add((1 << priv->restart_count) * TIMEOUT_MULTIPLIER, start_service_again_cb, manager);
112+ priv->restart_source = g_timeout_add((1 << priv->restart_count) * TIMEOUT_MULTIPLIER, start_service_again_cb, manager);
113 }
114
115 return;

Subscribers

People subscribed via source and target branches