Merge lp:~a-j-buxton/libindicator/remove-timeout into lp:libindicator/14.04

[Alistair Buxton]

This removes the timeout shutdown code from indicator-service.

This code was intended to allow the services to shut down automatically when using dbus activation and nothing was using the service any more. Indicators are now started by upstart or xdg-autostart, and this code becomes a problem. If the indicator is started by upstart and it exits, upstart will just keep running it again and again until it kills the job "respawning too fast". If the service was started by xdg-autostart and it decides to exit, nothing will start it again. This leads to indicators mysteriously disappearing from the panel, or never loading up in the first place due to race conditions.

If you haven't, please also see:

https://code.launchpad.net/~ted/libindicator/upstart-no-dbus/+merge/166269

This is another MR on libindicator dealing with the upstart changes, but as far as I can tell it does not deal with this specific issue.

Unmerged revisions

519. By Alistair Buxton on 2013-12-06

Remove the shutdown timeout.

Since indicators are now started by upstart, and not on demand, this
code is not needed.

=== modified file 'libindicator/indicator-service.c'
--- libindicator/indicator-service.c	2013-10-24 02:08:35 +0000
+++ libindicator/indicator-service.c	2013-12-06 14:39:26 +0000
@@ -42,7 +42,6 @@
 /**
 	IndicatorSevicePrivate:
 	@name: The DBus well known name for the service.
-	@timeout: The source ID for the timeout event.
 	@watcher: A list of processes on dbus that are watching us.
 	@this_service_version: The version to hand out that we're
 		implementing.  May not be set, so we'll send zero (default).
@@ -54,8 +53,6 @@
 	gchar * name;
 	GDBusConnection * bus;
 	GCancellable * bus_cancel;
-	guint timeout;
-	guint timeout_length;
 	GHashTable * watchers;
 	guint this_service_version;
 	guint dbus_registration;
@@ -188,24 +185,13 @@
 
 	/* Get the private variables in a decent state */
 	priv->name = NULL;
-	priv->timeout = 0;
 	priv->watchers = NULL;
 	priv->bus = NULL;
 	priv->bus_cancel = NULL;
 	priv->this_service_version = 0;
-	priv->timeout_length = 500;
 	priv->dbus_registration = 0;
 	priv->replace_mode = FALSE;
 
-	const gchar * timeoutenv = g_getenv("INDICATOR_SERVICE_SHUTDOWN_TIMEOUT");
-	if (timeoutenv != NULL) {
-		gdouble newtimeout = g_strtod(timeoutenv, NULL);
-		if (newtimeout >= 1.0f) {
-			priv->timeout_length = newtimeout;
-			g_debug("Setting shutdown timeout to: %u", priv->timeout_length);
-		}
-	}
-
 	const gchar * replaceenv = g_getenv("INDICATOR_SERVICE_REPLACE_MODE");
 	if (replaceenv != NULL) {
 		priv->replace_mode = TRUE;
@@ -226,8 +212,7 @@
 	return;
 }
 
-/* Unrefcounting the proxies and making sure that our
-   timeout doesn't come to haunt us. */
+/* Unrefcounting the proxies. */
 static void
 indicator_service_dispose (GObject *object)
 {
@@ -235,11 +220,6 @@
 
 	g_clear_pointer (&priv->watchers, g_hash_table_destroy);
 
-	if (priv->timeout != 0) {
-		g_source_remove(priv->timeout);
-		priv->timeout = 0;
-	}
-
 	if (priv->dbus_registration != 0) {
 		g_dbus_connection_unregister_object(priv->bus, priv->dbus_registration);
 		/* Don't care if it fails, there's nothing we can do */
@@ -415,43 +395,14 @@
 	return;
 }
 
-/* This is the function that gets executed if we timeout
-   because there are no watchers.  We sent the shutdown
-   signal and hope someone does something sane with it. */
-static gboolean
-timeout_no_watchers (gpointer data)
-{
-	g_warning("No watchers, service timing out.");
-	if (g_getenv("INDICATOR_ALLOW_NO_WATCHERS") == NULL) {
-		g_signal_emit(G_OBJECT(data), signals[SHUTDOWN], 0, TRUE);
-	} else {
-		g_warning("\tblocked by environment variable.");
-	}
-	return FALSE;
-}
-
 /* Callback saying that the name we were looking for has been
-   found and we've got it.  Now start the timer to see if anyone
-   cares about us. */
+   found and we've got it. */
 static void
 try_and_get_name_acquired_cb (GDBusConnection * connection, const gchar * name, gpointer user_data)
 {
 	g_return_if_fail(connection != NULL);
 	g_return_if_fail(INDICATOR_IS_SERVICE(user_data));
 
-	IndicatorServicePrivate * priv = INDICATOR_SERVICE_GET_PRIVATE(user_data);
-
-	/* Check to see if we already had a timer, if so we want to
-	   extend it a bit. */
-	if (priv->timeout != 0) {
-		g_source_remove(priv->timeout);
-		priv->timeout = 0;
-	}
-
-	/* Allow some extra time at start up as things can be in high
-	   contention then. */
-	priv->timeout = g_timeout_add(priv->timeout_length * 2, timeout_no_watchers, user_data);
-
 	return;
 }
 
@@ -480,15 +431,6 @@
 
 		g_dbus_connection_send_message(connection, message, G_DBUS_SEND_MESSAGE_FLAGS_NONE, NULL, NULL);
 		g_object_unref(message);
-
-		/* Check to see if we need to clean up a timeout */
-		if (priv->timeout != 0) {
-			g_source_remove(priv->timeout);
-			priv->timeout = 0;
-		}
-
-		/* Set a timeout for no watchers if we can't get the name */
-		priv->timeout = g_timeout_add(priv->timeout_length * 4, timeout_no_watchers, user_data);
 	}
 
 	return;
@@ -534,7 +476,7 @@
 /* Here is the function that gets called by the dbus
    interface "Watch" function.  It is an async function so
    that we can get the sender and store that information.  We
-   put them in a list and reset the timeout. */
+   put them in a list. */
 static GVariant *
 bus_watch (IndicatorService * service, const gchar * sender)
 {
@@ -557,17 +499,11 @@
 		}
 	}
 
-	if (priv->timeout != 0) {
-		g_source_remove(priv->timeout);
-		priv->timeout = 0;
-	}
-
 	return g_variant_new("(uu)", INDICATOR_SERVICE_VERSION, priv->this_service_version);
 }
 
 /* Performs the core of loosing a watcher; it removes them
-   from the list of watchers.  If there are none left, it then
-   starts the timer for the shutdown signal. */
+   from the list of watchers. */
 static void
 unwatch_core (IndicatorService * service, const gchar * name)
 {
@@ -587,19 +523,6 @@
 		g_warning("Unable to find watcher who is unwatching: %s", name);
 	}
 
-	/* If we're out of watchers set the timeout for shutdown */
-	if (g_hash_table_size(priv->watchers) == 0) {
-		if (priv->timeout != 0) {
-			/* This should never really happen, but let's ensure that
-			   bad things don't happen if it does. */
-			g_warning("No watchers timeout set twice.  Resolving, but odd.");
-			g_source_remove(priv->timeout);
-			priv->timeout = 0;
-		}
-		/* If we don't get a new watcher quickly, we'll shutdown. */
-		priv->timeout = g_timeout_add(priv->timeout_length, timeout_no_watchers, service);
-	}
-
 	return;
 }