cache: Clean up leaked resources when loading the cache

When soup_cache_dump() is not called for some reason, like a crash in
the application, the new cached resources are not referenced by the
index and then leaked in the cache directory. soup_cache_load() removes
all the resources found in the cache directory and not referenced by
the cached index.

https://bugzilla.gnome.org/show_bug.cgi?id=667682

2 files changed, 151 insertions, 20 deletions

diff --git a/libsoup/soup-cache.c b/libsoup/soup-cache.c
index 6272dcbe..e9538169 100644
--- a/libsoup/soup-cache.c
+++ b/libsoup/soup-cache.c
@@ -30,6 +30,7 @@
 #endif
 
 #include <string.h>
+#include <glib/gstdio.h>
 
 #include "soup-cache.h"
 #include "soup-body-input-stream.h"
@@ -1274,6 +1275,25 @@ soup_cache_flush (SoupCache *cache)
 		g_warning ("Cache flush finished despite %d pending requests", cache->priv->n_pending);
 }
 
+typedef void (* SoupCacheForeachFileFunc) (SoupCache *cache, const char *name, gpointer user_data);
+
+static void
+soup_cache_foreach_file (SoupCache *cache, SoupCacheForeachFileFunc func, gpointer user_data)
+{
+	GDir *dir;
+	const char *name;
+	SoupCachePrivate *priv = cache->priv;
+
+	dir = g_dir_open (priv->cache_dir, 0, NULL);
+	while ((name = g_dir_read_name (dir))) {
+		if (g_str_has_prefix (name, "soup."))
+		    continue;
+
+		func (cache, name, user_data);
+	}
+	g_dir_close (dir);
+}
+
 static void
 clear_cache_item (gpointer data,
 		  gpointer user_data)
@@ -1282,28 +1302,19 @@ clear_cache_item (gpointer data,
 }
 
 static void
+delete_cache_file (SoupCache *cache, const char *name, gpointer user_data)
+{
+	gchar *path;
+
+	path = g_build_filename (cache->priv->cache_dir, name, NULL);
+	g_unlink (path);
+	g_free (path);
+}
+
+static void
 clear_cache_files (SoupCache *cache)
 {
-	GFileInfo *file_info;
-	GFileEnumerator *file_enumerator;
-	GFile *cache_dir_file = g_file_new_for_path (cache->priv->cache_dir);
-
-	file_enumerator = g_file_enumerate_children (cache_dir_file, G_FILE_ATTRIBUTE_STANDARD_NAME,
-						     G_FILE_QUERY_INFO_NONE, NULL, NULL);
-	if (file_enumerator) {
-		while ((file_info = g_file_enumerator_next_file (file_enumerator, NULL, NULL)) != NULL) {
-			const char *filename = g_file_info_get_name (file_info);
-
-			if (strcmp (filename, SOUP_CACHE_FILE) != 0) {
-				GFile *cache_file = g_file_get_child (cache_dir_file, filename);
-				g_file_delete (cache_file, NULL, NULL);
-				g_object_unref (cache_file);
-			}
-			g_object_unref (file_info);
-		}
-		g_object_unref (file_enumerator);
-	}
-	g_object_unref (cache_dir_file);
+	soup_cache_foreach_file (cache, delete_cache_file, NULL);
 }
 
 /**
@@ -1490,6 +1501,32 @@ soup_cache_dump (SoupCache *cache)
 	g_variant_unref (cache_variant);
 }
 
+static inline guint32
+get_key_from_cache_filename (const char *name)
+{
+	guint64 key;
+
+	key = g_ascii_strtoull (name, NULL, 10);
+	return key ? (guint32)key : 0;
+}
+
+static void
+insert_cache_file (SoupCache *cache, const char *name, GHashTable *leaked_entries)
+{
+	gchar *path;
+
+	path = g_build_filename (cache->priv->cache_dir, name, NULL);
+	if (g_file_test (path, G_FILE_TEST_IS_REGULAR)) {
+		guint32 key = get_key_from_cache_filename (name);
+
+		if (key) {
+			g_hash_table_insert (leaked_entries, GUINT_TO_POINTER (key), path);
+			return;
+		}
+	}
+	g_free (path);
+}
+
 /**
  * soup_cache_load:
  * @cache: a #SoupCache
@@ -1511,6 +1548,9 @@ soup_cache_load (SoupCache *cache)
 	SoupCacheEntry *entry;
 	SoupCachePrivate *priv = cache->priv;
 	guint16 version, status_code;
+	GHashTable *leaked_entries = NULL;
+	GHashTableIter iter;
+	gpointer value;
 
 	filename = g_build_filename (priv->cache_dir, SOUP_CACHE_FILE, NULL);
 	if (!g_file_get_contents (filename, &contents, &length, NULL)) {
@@ -1531,6 +1571,9 @@ soup_cache_load (SoupCache *cache)
 		return;
 	}
 
+	leaked_entries = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_free);
+	soup_cache_foreach_file (cache, (SoupCacheForeachFileFunc)insert_cache_file, leaked_entries);
+
 	while (g_variant_iter_loop (entries_iter, SOUP_CACHE_PHEADERS_FORMAT,
 				    &url, &must_revalidate, &freshness_lifetime, &corrected_initial_age,
 				    &response_time, &hits, &length, &status_code,
@@ -1566,8 +1609,16 @@ soup_cache_load (SoupCache *cache)
 
 		if (!soup_cache_entry_insert (cache, entry, FALSE))
 			soup_cache_entry_free (entry);
+		else
+			g_hash_table_remove (leaked_entries, GUINT_TO_POINTER (entry->key));
 	}
 
+	/* Remove the leaked files */
+	g_hash_table_iter_init (&iter, leaked_entries);
+	while (g_hash_table_iter_next (&iter, NULL, &value))
+		g_unlink ((char *)value);
+	g_hash_table_destroy (leaked_entries);
+
 	cache->priv->lru_start = g_list_reverse (cache->priv->lru_start);
 
 	/* frees */
diff --git a/tests/cache-test.c b/tests/cache-test.c
index ce811dff..543ef022 100644
--- a/tests/cache-test.c
+++ b/tests/cache-test.c
@@ -630,6 +630,85 @@ do_headers_test (gconstpointer data)
 	g_free (body1);
 }
 
+static guint
+count_cached_resources_in_dir (const char *cache_dir)
+{
+	GDir *dir;
+	const char *name;
+	guint retval = 0;
+
+	dir = g_dir_open (cache_dir, 0, NULL);
+	while ((name = g_dir_read_name (dir))) {
+		if (g_str_has_prefix (name, "soup."))
+			continue;
+
+		retval++;
+	}
+	g_dir_close (dir);
+
+	return retval;
+}
+
+static void
+do_leaks_test (gconstpointer data)
+{
+	SoupURI *base_uri = (SoupURI *)data;
+	SoupSession *session;
+	SoupCache *cache;
+	char *cache_dir;
+	char *body;
+
+	cache_dir = g_dir_make_tmp ("cache-test-XXXXXX", NULL);
+	debug_printf (2, "  Caching to %s\n", cache_dir);
+	cache = soup_cache_new (cache_dir, SOUP_CACHE_SINGLE_USER);
+	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC,
+					 SOUP_SESSION_USE_THREAD_CONTEXT, TRUE,
+					 SOUP_SESSION_ADD_FEATURE, cache,
+					 NULL);
+
+	debug_printf (2, "  Initial requests\n");
+	body = do_request (session, base_uri, "GET", "/1", NULL,
+			   "Test-Set-Expires", "Fri, 01 Jan 2100 00:00:00 GMT",
+			   NULL);
+	g_free (body);
+	body = do_request (session, base_uri, "GET", "/2", NULL,
+			   "Test-Set-Expires", "Fri, 01 Jan 2100 00:00:00 GMT",
+			   NULL);
+	g_free (body);
+	body = do_request (session, base_uri, "GET", "/3", NULL,
+			   "Test-Set-Expires", "Fri, 01 Jan 2100 00:00:00 GMT",
+			   NULL);
+	g_free (body);
+
+	debug_printf (2, "  Dumping the cache\n");
+	soup_cache_dump (cache);
+
+	g_assert_cmpuint (count_cached_resources_in_dir (cache_dir), ==, 3);
+
+	body = do_request (session, base_uri, "GET", "/4", NULL,
+			   "Test-Set-Expires", "Fri, 01 Jan 2100 00:00:00 GMT",
+			   NULL);
+	g_free (body);
+	body = do_request (session, base_uri, "GET", "/5", NULL,
+			   "Test-Set-Expires", "Fri, 01 Jan 2100 00:00:00 GMT",
+			   NULL);
+	g_free (body);
+
+	/* Destroy the cache without dumping the last two resources */
+	soup_test_session_abort_unref (session);
+	g_object_unref (cache);
+
+	cache = soup_cache_new (cache_dir, SOUP_CACHE_SINGLE_USER);
+
+	debug_printf (2, "  Loading the cache\n");
+	g_assert_cmpuint (count_cached_resources_in_dir (cache_dir), ==, 5);
+	soup_cache_load (cache);
+	g_assert_cmpuint (count_cached_resources_in_dir (cache_dir), ==, 3);
+
+	g_object_unref (cache);
+	g_free (cache_dir);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -647,6 +726,7 @@ main (int argc, char **argv)
 	g_test_add_data_func ("/cache/cancellation", base_uri, do_cancel_test);
 	g_test_add_data_func ("/cache/refcounting", base_uri, do_refcounting_test);
 	g_test_add_data_func ("/cache/headers", base_uri, do_headers_test);
+	g_test_add_data_func ("/cache/leaks", base_uri, do_leaks_test);
 
 	ret = g_test_run ();