location-source: Fix subclass resource leaks on start

The start_source() and stop_source() functions in GClueLocationSource
are chained up by all the start() and stop() method implementations in
GClueLocationSource subclasses to determine when to really start and
stop the source and allocate / free the needed resources.

Commit 0661107fe0 made start_source() return TRUE when the source was
already active, which caused the resources already reserved for the
physical source to be reserved again. The source class implementations
are typically designed to hold just one set of resources, so the
references to previously reserved resources are lost, that is they leak.
An example of this is the GSocketClient reference (priv->client) held by
GClueNMEASource.

Another visible (in the debug log) example of the extra resources are
callbacks connected to the "fix-*" signals of the physical sources (see
gclue_modem_gps_start() for example). With more than one client using
the same source, all clients receive the same location update N times,
where N is the number of clients using the source.

Make GClueLocationSource.start() and .stop() methods return enums
GClueLocationSourceStartResult and GClueLocationSourceStopResult which
gives subclasses more information on when to allocate resources on
starting and stopping.

The start_source() function now returns the value
GCLUE_LOCATION_SOURCE_START_RESULT_ALREADY_STARTED when the source is
already active. The subclass start() method implementations are changed
to return immediately when they get this value from the base class.

This effectively reverts the first diff in commit 0661107fe0
"location-source: Return source state from start_source and
stop_source".

The behaviour of stop() method subclass implementations remains the
same. Freeing of resources is always attempted, except when the value
GCLUE_LOCATION_SOURCE_STOP_RESULT_STILL_USED is returned from the base
class.

7 files changed, 96 insertions, 61 deletions

diff --git a/src/gclue-cdma.c b/src/gclue-cdma.c
index 2a26a77..0372788 100644
--- a/src/gclue-cdma.c
+++ b/src/gclue-cdma.c
@@ -47,9 +47,9 @@ G_DEFINE_TYPE_WITH_CODE (GClueCDMA,
                          GCLUE_TYPE_LOCATION_SOURCE,
                          G_ADD_PRIVATE (GClueCDMA))
 
-static gboolean
+static GClueLocationSourceStartResult
 gclue_cdma_start (GClueLocationSource *source);
-static gboolean
+static GClueLocationSourceStopResult
 gclue_cdma_stop (GClueLocationSource *source);
 
 static void
@@ -206,18 +206,20 @@ on_fix_cdma (GClueModem *modem,
         g_object_unref (location);
 }
 
-static gboolean
+static GClueLocationSourceStartResult
 gclue_cdma_start (GClueLocationSource *source)
 {
         GClueLocationSourceClass *base_class;
         GClueCDMAPrivate *priv;
+        GClueLocationSourceStartResult base_result;
 
         g_return_val_if_fail (GCLUE_IS_LOCATION_SOURCE (source), FALSE);
         priv = GCLUE_CDMA (source)->priv;
 
         base_class = GCLUE_LOCATION_SOURCE_CLASS (gclue_cdma_parent_class);
-        if (!base_class->start (source))
-                return FALSE;
+        base_result = base_class->start (source);
+        if (base_result != GCLUE_LOCATION_SOURCE_START_RESULT_OK)
+                return base_result;
 
         g_signal_connect (priv->modem,
                           "fix-cdma",
@@ -230,21 +232,23 @@ gclue_cdma_start (GClueLocationSource *source)
                                          on_cdma_enabled,
                                          source);
 
-        return TRUE;
+        return base_result;
 }
 
-static gboolean
+static GClueLocationSourceStopResult
 gclue_cdma_stop (GClueLocationSource *source)
 {
         GClueCDMAPrivate *priv = GCLUE_CDMA (source)->priv;
         GClueLocationSourceClass *base_class;
         GError *error = NULL;
+        GClueLocationSourceStopResult base_result;
 
         g_return_val_if_fail (GCLUE_IS_LOCATION_SOURCE (source), FALSE);
 
         base_class = GCLUE_LOCATION_SOURCE_CLASS (gclue_cdma_parent_class);
-        if (!base_class->stop (source))
-                return FALSE;
+        base_result = base_class->stop (source);
+        if (base_result == GCLUE_LOCATION_SOURCE_STOP_RESULT_STILL_USED)
+                return base_result;
 
         g_signal_handlers_disconnect_by_func (G_OBJECT (priv->modem),
                                               G_CALLBACK (on_fix_cdma),
@@ -259,5 +263,5 @@ gclue_cdma_stop (GClueLocationSource *source)
                         g_error_free (error);
                 }
 
-        return TRUE;
+        return base_result;
 }
diff --git a/src/gclue-location-source.c b/src/gclue-location-source.c
index 4ae4c57..09028eb 100644
--- a/src/gclue-location-source.c
+++ b/src/gclue-location-source.c
@@ -31,9 +31,9 @@
  * The interface all geolocation sources must implement.
  **/
 
-static gboolean
+static GClueLocationSourceStartResult
 start_source (GClueLocationSource *source);
-static gboolean
+static GClueLocationSourceStopResult
 stop_source (GClueLocationSource *source);
 
 struct _GClueLocationSourcePrivate
@@ -284,14 +284,14 @@ gclue_location_source_init (GClueLocationSource *source)
         source->priv->time_threshold = gclue_min_uint_new ();
 }
 
-static gboolean
+static GClueLocationSourceStartResult
 start_source (GClueLocationSource *source)
 {
         source->priv->active_counter++;
         if (source->priv->active_counter > 1) {
                 g_debug ("%s already active, not starting.",
                          G_OBJECT_TYPE_NAME (source));
-                return TRUE;
+                return GCLUE_LOCATION_SOURCE_START_RESULT_ALREADY_STARTED;
         }
 
         if (source->priv->compute_movement) {
@@ -305,23 +305,23 @@ start_source (GClueLocationSource *source)
 
         g_object_notify (G_OBJECT (source), "active");
         g_debug ("%s now active", G_OBJECT_TYPE_NAME (source));
-        return TRUE;
+        return GCLUE_LOCATION_SOURCE_START_RESULT_OK;
 }
 
-static gboolean
+static GClueLocationSourceStopResult
 stop_source (GClueLocationSource *source)
 {
         if (source->priv->active_counter == 0) {
                 g_debug ("%s already inactive, not stopping.",
                          G_OBJECT_TYPE_NAME (source));
-                return TRUE;
+                return GCLUE_LOCATION_SOURCE_STOP_RESULT_ALREADY_STOPPED;
         }
 
         source->priv->active_counter--;
         if (source->priv->active_counter > 0) {
                 g_debug ("%s still in use, not stopping.",
                          G_OBJECT_TYPE_NAME (source));
-                return FALSE;
+                return GCLUE_LOCATION_SOURCE_STOP_RESULT_STILL_USED;
         }
 
         if (source->priv->compass) {
@@ -333,7 +333,7 @@ stop_source (GClueLocationSource *source)
         g_object_notify (G_OBJECT (source), "active");
         g_debug ("%s now inactive", G_OBJECT_TYPE_NAME (source));
 
-        return TRUE;
+        return GCLUE_LOCATION_SOURCE_STOP_RESULT_OK;
 }
 
 /**
diff --git a/src/gclue-location-source.h b/src/gclue-location-source.h
index 51d2616..620f601 100644
--- a/src/gclue-location-source.h
+++ b/src/gclue-location-source.h
@@ -38,6 +38,19 @@ G_BEGIN_DECLS
 #define GCLUE_IS_LOCATION_SOURCE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass),  GCLUE_TYPE_LOCATION_SOURCE))
 #define GCLUE_LOCATION_SOURCE_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj),  GCLUE_TYPE_LOCATION_SOURCE, GClueLocationSourceClass))
 
+typedef enum {
+        GCLUE_LOCATION_SOURCE_START_RESULT_FAILED = 0,
+        GCLUE_LOCATION_SOURCE_START_RESULT_ALREADY_STARTED,
+        GCLUE_LOCATION_SOURCE_START_RESULT_OK
+} GClueLocationSourceStartResult;
+
+typedef enum {
+        GCLUE_LOCATION_SOURCE_STOP_RESULT_FAILED = 0,
+        GCLUE_LOCATION_SOURCE_STOP_RESULT_ALREADY_STOPPED,
+        GCLUE_LOCATION_SOURCE_STOP_RESULT_STILL_USED,
+        GCLUE_LOCATION_SOURCE_STOP_RESULT_OK
+} GClueLocationSourceStopResult;
+
 typedef struct _GClueLocationSource        GClueLocationSource;
 typedef struct _GClueLocationSourceClass   GClueLocationSourceClass;
 typedef struct _GClueLocationSourcePrivate GClueLocationSourcePrivate;
@@ -54,8 +67,8 @@ struct _GClueLocationSourceClass
 {
         GObjectClass parent_class;
 
-        gboolean (*start) (GClueLocationSource *source);
-        gboolean (*stop)  (GClueLocationSource *source);
+        GClueLocationSourceStartResult (*start) (GClueLocationSource *source);
+        GClueLocationSourceStopResult (*stop)  (GClueLocationSource *source);
 };
 
 GType gclue_location_source_get_type (void) G_GNUC_CONST;
diff --git a/src/gclue-locator.c b/src/gclue-locator.c
index 650be43..0adb3ce 100644
--- a/src/gclue-locator.c
+++ b/src/gclue-locator.c
@@ -49,9 +49,9 @@
  * location sources from rest of the code
  */
 
-static gboolean
+static GClueLocationSourceStartResult
 gclue_locator_start (GClueLocationSource *source);
-static gboolean
+static GClueLocationSourceStopResult 
 gclue_locator_stop (GClueLocationSource *source);
 
 struct _GClueLocatorPrivate
@@ -433,19 +433,21 @@ gclue_locator_init (GClueLocator *locator)
                                             GClueLocatorPrivate);
 }
 
-static gboolean
+static GClueLocationSourceStartResult
 gclue_locator_start (GClueLocationSource *source)
 {
         GClueLocationSourceClass *base_class;
         GClueLocator *locator;
         GList *node;
+        GClueLocationSourceStopResult base_result;
 
         g_return_val_if_fail (GCLUE_IS_LOCATOR (source), FALSE);
         locator = GCLUE_LOCATOR (source);
 
         base_class = GCLUE_LOCATION_SOURCE_CLASS (gclue_locator_parent_class);
-        if (!base_class->start (source))
-                return FALSE;
+        base_result = base_class->start (source);
+        if (base_result != GCLUE_LOCATION_SOURCE_START_RESULT_OK)
+                return base_result;
 
         for (node = locator->priv->sources; node != NULL; node = node->next) {
                 GClueLocationSource *src = GCLUE_LOCATION_SOURCE (node->data);
@@ -468,22 +470,24 @@ gclue_locator_start (GClueLocationSource *source)
                 start_source (locator, src);
         }
 
-        return TRUE;
+        return base_result;
 }
 
-static gboolean
+static GClueLocationSourceStopResult
 gclue_locator_stop (GClueLocationSource *source)
 {
         GClueLocationSourceClass *base_class;
         GClueLocator *locator;
         GList *node;
+        GClueLocationSourceStopResult base_result;
 
         g_return_val_if_fail (GCLUE_IS_LOCATOR (source), FALSE);
         locator = GCLUE_LOCATOR (source);
 
         base_class = GCLUE_LOCATION_SOURCE_CLASS (gclue_locator_parent_class);
-        if (!base_class->stop (source))
-                return FALSE;
+        base_result = base_class->stop (source);
+        if (base_result == GCLUE_LOCATION_SOURCE_STOP_RESULT_STILL_USED)
+                return base_result;
 
         for (node = locator->priv->active_sources; node != NULL; node = node->next) {
                 GClueLocationSource *src = GCLUE_LOCATION_SOURCE (node->data);
@@ -497,7 +501,7 @@ gclue_locator_stop (GClueLocationSource *source)
 
         g_list_free (locator->priv->active_sources);
         locator->priv->active_sources = NULL;
-        return TRUE;
+        return base_result;
 }
 
 GClueLocator *
diff --git a/src/gclue-modem-gps.c b/src/gclue-modem-gps.c
index 501c56e..53ea1b4 100644
--- a/src/gclue-modem-gps.c
+++ b/src/gclue-modem-gps.c
@@ -48,9 +48,9 @@ G_DEFINE_TYPE_WITH_CODE (GClueModemGPS,
                          GCLUE_TYPE_LOCATION_SOURCE,
                          G_ADD_PRIVATE (GClueModemGPS))
 
-static gboolean
+static GClueLocationSourceStartResult
 gclue_modem_gps_start (GClueLocationSource *source);
-static gboolean
+static GClueLocationSourceStopResult
 gclue_modem_gps_stop (GClueLocationSource *source);
 
 static void
@@ -233,18 +233,20 @@ on_fix_gps (GClueModem *modem,
                                             location);
 }
 
-static gboolean
+static GClueLocationSourceStartResult
 gclue_modem_gps_start (GClueLocationSource *source)
 {
         GClueLocationSourceClass *base_class;
         GClueModemGPSPrivate *priv;
+        GClueLocationSourceStartResult base_result;
 
         g_return_val_if_fail (GCLUE_IS_LOCATION_SOURCE (source), FALSE);
         priv = GCLUE_MODEM_GPS (source)->priv;
 
         base_class = GCLUE_LOCATION_SOURCE_CLASS (gclue_modem_gps_parent_class);
-        if (!base_class->start (source))
-                return FALSE;
+        base_result = base_class->start (source);
+        if (base_result != GCLUE_LOCATION_SOURCE_START_RESULT_OK)
+                return base_result;
 
         g_signal_connect (priv->modem,
                           "fix-gps",
@@ -257,21 +259,23 @@ gclue_modem_gps_start (GClueLocationSource *source)
                                         on_gps_enabled,
                                         source);
 
-        return TRUE;
+        return base_result;
 }
 
-static gboolean
+static GClueLocationSourceStopResult
 gclue_modem_gps_stop (GClueLocationSource *source)
 {
         GClueModemGPSPrivate *priv = GCLUE_MODEM_GPS (source)->priv;
         GClueLocationSourceClass *base_class;
         GError *error = NULL;
+        GClueLocationSourceStopResult base_result;
 
         g_return_val_if_fail (GCLUE_IS_LOCATION_SOURCE (source), FALSE);
 
         base_class = GCLUE_LOCATION_SOURCE_CLASS (gclue_modem_gps_parent_class);
-        if (!base_class->stop (source))
-                return FALSE;
+        base_result = base_class->stop (source);
+        if (base_result == GCLUE_LOCATION_SOURCE_STOP_RESULT_STILL_USED)
+                return base_result;
 
         g_signal_handlers_disconnect_by_func (G_OBJECT (priv->modem),
                                               G_CALLBACK (on_fix_gps),
@@ -286,5 +290,5 @@ gclue_modem_gps_stop (GClueLocationSource *source)
                         g_error_free (error);
                 }
 
-        return TRUE;
+        return base_result;
 }
diff --git a/src/gclue-nmea-source.c b/src/gclue-nmea-source.c
index 7461b79..c1be172 100644
--- a/src/gclue-nmea-source.c
+++ b/src/gclue-nmea-source.c
@@ -58,9 +58,9 @@ G_DEFINE_TYPE_WITH_CODE (GClueNMEASource,
                          GCLUE_TYPE_LOCATION_SOURCE,
                          G_ADD_PRIVATE (GClueNMEASource))
 
-static gboolean
+static GClueLocationSourceStartResult
 gclue_nmea_source_start (GClueLocationSource *source);
-static gboolean
+static GClueLocationSourceStopResult
 gclue_nmea_source_stop (GClueLocationSource *source);
 
 static void
@@ -775,34 +775,38 @@ gclue_nmea_source_get_singleton (void)
         return source;
 }
 
-static gboolean
+static GClueLocationSourceStartResult
 gclue_nmea_source_start (GClueLocationSource *source)
 {
         GClueLocationSourceClass *base_class;
+        GClueLocationSourceStartResult base_result;
 
         g_return_val_if_fail (GCLUE_IS_NMEA_SOURCE (source), FALSE);
 
         base_class = GCLUE_LOCATION_SOURCE_CLASS (gclue_nmea_source_parent_class);
-        if (!base_class->start (source))
-                return FALSE;
+        base_result = base_class->start (source);
+        if (base_result != GCLUE_LOCATION_SOURCE_START_RESULT_OK)
+                return base_result;
 
         connect_to_service (GCLUE_NMEA_SOURCE (source));
 
-        return TRUE;
+        return base_result;
 }
 
-static gboolean
+static GClueLocationSourceStopResult
 gclue_nmea_source_stop (GClueLocationSource *source)
 {
         GClueLocationSourceClass *base_class;
+        GClueLocationSourceStopResult base_result;
 
         g_return_val_if_fail (GCLUE_IS_NMEA_SOURCE (source), FALSE);
 
         base_class = GCLUE_LOCATION_SOURCE_CLASS (gclue_nmea_source_parent_class);
-        if (!base_class->stop (source))
-                return FALSE;
+        base_result = base_class->stop (source);
+        if (base_result == GCLUE_LOCATION_SOURCE_STOP_RESULT_STILL_USED)
+                return base_result;
 
         disconnect_from_service (GCLUE_NMEA_SOURCE (source));
 
-        return TRUE;
+        return base_result;
 }
diff --git a/src/gclue-wifi.c b/src/gclue-wifi.c
index 3fc78d0..2c1e34b 100644
--- a/src/gclue-wifi.c
+++ b/src/gclue-wifi.c
@@ -52,9 +52,9 @@
  * Contains functions to get the geolocation based on nearby WiFi networks.
  **/
 
-static gboolean
+static GClueLocationSourceStartResult
 gclue_wifi_start (GClueLocationSource *source);
-static gboolean
+static GClueLocationSourceStopResult
 gclue_wifi_stop (GClueLocationSource *source);
 
 static guint
@@ -730,36 +730,42 @@ disconnect_cache_prune_timeout (GClueWifi *wifi)
         priv->cache_prune_timeout_id = 0;
 }
 
-static gboolean
+static GClueLocationSourceStartResult
 gclue_wifi_start (GClueLocationSource *source)
 {
         GClueLocationSourceClass *base_class;
+        GClueLocationSourceStartResult base_result;
 
         g_return_val_if_fail (GCLUE_IS_WIFI (source), FALSE);
 
         base_class = GCLUE_LOCATION_SOURCE_CLASS (gclue_wifi_parent_class);
-        if (!base_class->start (source))
-                return FALSE;
+        base_result = base_class->start (source);
+        if (base_result != GCLUE_LOCATION_SOURCE_START_RESULT_OK)
+                return base_result;
 
         connect_cache_prune_timeout (GCLUE_WIFI (source));
         connect_bss_signals (GCLUE_WIFI (source));
-        return TRUE;
+
+        return base_result;
 }
 
-static gboolean
+static GClueLocationSourceStopResult
 gclue_wifi_stop (GClueLocationSource *source)
 {
         GClueLocationSourceClass *base_class;
+        GClueLocationSourceStopResult base_result;
 
         g_return_val_if_fail (GCLUE_IS_WIFI (source), FALSE);
 
         base_class = GCLUE_LOCATION_SOURCE_CLASS (gclue_wifi_parent_class);
-        if (!base_class->stop (source))
-                return FALSE;
+        base_result = base_class->stop (source);
+        if (base_result == GCLUE_LOCATION_SOURCE_STOP_RESULT_STILL_USED)
+                return base_result;
 
         disconnect_bss_signals (GCLUE_WIFI (source));
         disconnect_cache_prune_timeout (GCLUE_WIFI (source));
-        return TRUE;
+
+        return base_result;
 }
 
 static GClueAccuracyLevel