mediascanner: fix the service memory leakagesicefish_8.99.5icefish/8.99.58.99.5

Bug-AGL: SPEC-3103

- fix memory leakage in 'media_results_get' callback function which
  is called by 'media_result' verb. Set free-full the whole list with
  'free_media_item' callback function(major leak).
- fix memory leakage in 'MediaPlayerManagerInit' function, file object
  is not released(minor leak).
- fix memory leakage in 'media_lightmediascanner_scan' function and
  avoid open sqlite3 connection and allocate whole sqlite
  resources again and again on every function execution (major leak).
- fix memory leakage in 'on_interface_proxy_properties_changed' when
  iterating over 'changed_properties' and not release key & subValues.

Signed-off-by: Farshid Monhaseri <monhaseri.f@gmail.com>
Change-Id: I943a961b0c13be79c70c80e7a4f558958551f800

3 files changed, 55 insertions, 17 deletions

diff --git a/binding/media-api.c b/binding/media-api.c
index 05fbba2..0365b49 100644
--- a/binding/media-api.c
+++ b/binding/media-api.c
@@ -245,7 +245,7 @@ static void media_results_get (afb_req_t request)
     }
 
     jresp = new_json_object_from_device(list);
-    g_list_free(list);
+    g_list_free_full(list,free_media_item);
     ListUnlock();
 
     if (jresp == NULL) {
diff --git a/binding/media-manager.c b/binding/media-manager.c
index fbe4bb7..283692c 100644
--- a/binding/media-manager.c
+++ b/binding/media-manager.c
@@ -38,9 +38,14 @@ const gchar *lms_scan_types[] = {
     MEDIA_VIDEO,
     MEDIA_IMAGE
 };
+typedef struct {
+    sqlite3 *db;
+    gboolean is_open;
+}scannerDB;
 
 static Binding_RegisterCallback_t g_RegisterCallback = { 0 };
 static stMediaPlayerManage MediaPlayerManage = { 0 };
+static scannerDB scanDB = { 0 };
 
 /* ------ LOCAL  FUNCTIONS --------- */
 void ListLock() {
@@ -100,12 +105,17 @@ GList* media_lightmediascanner_scan(GList *list, gchar *uri, int scan_type)
     gchar *media_type;
     int ret = 0;
 
-    db_path = scanner1_get_data_base_path(MediaPlayerManage.lms_proxy);
+    if(!scanDB.is_open) {
+        db_path = scanner1_get_data_base_path(MediaPlayerManage.lms_proxy);
 
-    ret = sqlite3_open(db_path, &conn);
-    if (ret) {
-        LOGE("Cannot open SQLITE database: '%s'\n", db_path);
-        return NULL;
+        ret = sqlite3_open(db_path, &scanDB.db);
+        if (ret != SQLITE_OK) {
+            LOGE("Cannot open SQLITE database: '%s'\n", db_path);
+            scanDB.is_open = FALSE;
+            g_list_free_full(list,free_media_item);
+            return NULL;
+        }
+        scanDB.is_open = TRUE;
     }
 
     switch (scan_type) {
@@ -128,7 +138,7 @@ GList* media_lightmediascanner_scan(GList *list, gchar *uri, int scan_type)
         return NULL;
     }
 
-    ret = sqlite3_prepare_v2(conn, query, (int) strlen(query), &res, &tail);
+    ret = sqlite3_prepare_v2(scanDB.db, query, (int) strlen(query), &res, &tail);
     if (ret) {
         LOGE("Cannot execute query '%s'\n", query);
         g_free(query);
@@ -145,6 +155,7 @@ GList* media_lightmediascanner_scan(GList *list, gchar *uri, int scan_type)
         if (ret)
             continue;
 
+        //We may check the allocation result ... but It may be a bit expensive in such a loop
         item = g_malloc0(sizeof(*item));
         tmp = g_uri_escape_string(path, "/", TRUE);
         item->path = g_strdup_printf("file://%s", tmp);
@@ -163,7 +174,7 @@ GList* media_lightmediascanner_scan(GList *list, gchar *uri, int scan_type)
     return list;
 }
 
-static void free_media_item(void *data)
+void free_media_item(void *data)
 {
     struct Media_Item *item = data;
 
@@ -184,31 +195,34 @@ on_interface_proxy_properties_changed (GDBusProxy *proxy,
         return;
 
     GVariantIter iter;
-    const gchar *key;
-    GVariant *subValue;
-    const gchar *pInterface;
+    gchar *key = NULL;
+    GVariant *subValue = NULL;
+    gchar *pInterface = NULL;
     GList *list = NULL;
-
+    gboolean br = FALSE;
     pInterface = g_dbus_proxy_get_interface_name (proxy);
 
     if (0 != g_strcmp0(pInterface, LIGHTMEDIASCANNER_INTERFACE))
         return;
 
     g_variant_iter_init (&iter, changed_properties);
-    while (g_variant_iter_next (&iter, "{&sv}", &key, &subValue))
+    while (g_variant_iter_loop(&iter, "{&sv}", &key, &subValue))
     {
         gboolean val;
         if (0 == g_strcmp0(key,"IsScanning")) {
             g_variant_get(subValue, "b", &val);
             if (val == TRUE)
-                return;
+                br = TRUE;
         } else if (0 == g_strcmp0(key, "WriteLocked")) {
             g_variant_get(subValue, "b", &val);
             if (val == TRUE)
-                return;
+                br = TRUE;
         }
     }
 
+    if(br)
+        return;
+
     ListLock();
     if(MediaPlayerManage.type_filter & LMS_AUDIO_SCAN)
         list = media_lightmediascanner_scan(list, MediaPlayerManage.uri_filter, LMS_AUDIO_SCAN);
@@ -271,12 +285,28 @@ unmount_cb (GFileMonitor      *mon,
 {
     gchar *path = g_file_get_path(file);
     gchar *uri = g_strconcat("file://", path, NULL);
+    gint ret = 0;
 
     ListLock();
 
     if (g_RegisterCallback.binding_device_removed &&
         event == G_FILE_MONITOR_EVENT_DELETED) {
+
         g_RegisterCallback.binding_device_removed(uri);
+        ret = sqlite3_close(scanDB.db);
+        /* TODO: Release SQLite connection handle resources on the end of each session
+        *
+        * we should be able to handle the SQLITE_BUSY return value safely
+        * and be sure the connection handle will be released finally at the end of session
+        * There are a few synchronous & asynchronous libsqlite methods
+        * to handle this situation properly.
+        */
+        if(ret == SQLITE_OK) {
+            scanDB.db = NULL;
+            scanDB.is_open = FALSE;
+        } else {
+            LOGE("Failed to release SQLite connection handle.\n");
+        }
         g_free(path);
     } else if (event == G_FILE_MONITOR_EVENT_CREATED) {
         MediaPlayerManage.uri_filter = path;
@@ -296,16 +326,22 @@ unmount_cb (GFileMonitor      *mon,
  */
 int MediaPlayerManagerInit() {
     pthread_t thread_id;
-    GFile *file;
-    GFileMonitor *mon;
+    GFile *file = NULL;
+    GFileMonitor *mon = MediaPlayerManage.mon;
     int ret;
 
     g_mutex_init(&(MediaPlayerManage.m));
+    scanDB.is_open = FALSE;
+    if(mon != NULL) {
+        g_object_unref(mon);
+        mon = NULL;
+    }
 
     file = g_file_new_for_path("/media");
     g_assert(file != NULL);
 
     mon = g_file_monitor (file, G_FILE_MONITOR_NONE, NULL, NULL);
+    g_object_unref(file);
     g_assert(mon != NULL);
     g_signal_connect (mon, "changed", G_CALLBACK(unmount_cb), NULL);
 
diff --git a/binding/media-manager.h b/binding/media-manager.h
index c5b6778..65ee831 100644
--- a/binding/media-manager.h
+++ b/binding/media-manager.h
@@ -111,6 +111,7 @@ typedef struct {
     gint  type_filter;
     GMutex m;
     Scanner1 *lms_proxy;
+    GFileMonitor *mon;
 } stMediaPlayerManage;
 
 typedef struct tagBinding_RegisterCallback
@@ -120,6 +121,7 @@ typedef struct tagBinding_RegisterCallback
 } Binding_RegisterCallback_t;
 
 /* ------ PUBLIC PLUGIN FUNCTIONS --------- */
+void free_media_item(void *data);
 void BindingAPIRegister(const Binding_RegisterCallback_t* pstRegisterCallback);
 int MediaPlayerManagerInit(void);