From 3137e9db2720dcd4229915eb85c3e9abca8f1ec3 Mon Sep 17 00:00:00 2001 From: Farshid Monhaseri Date: Wed, 22 Jan 2020 19:19:16 +0330 Subject: mediascanner: fix the service memory leakages 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 Change-Id: I943a961b0c13be79c70c80e7a4f558958551f800 --- binding/media-api.c | 2 +- binding/media-manager.c | 68 +++++++++++++++++++++++++++++++++++++------------ binding/media-manager.h | 2 ++ 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); -- cgit 1.2.3-korg