Redo package cache breakage fix

2 files changed, 38 insertions, 12 deletions

diff --git a/libpkgconf/client.c b/libpkgconf/client.c
index cda1c75..bcb6ae8 100644
--- a/libpkgconf/client.c
+++ b/libpkgconf/client.c
@@ -141,8 +141,9 @@ pkgconf_client_deinit(pkgconf_client_t *client)
 	pkgconf_path_free(&client->dir_list);
 	pkgconf_cache_free(client);
 
-        // Added to the original leaking code (reported issue #130).
-        //
+        /*
+         * Added to the original leaking code (reported issue #130).
+         */
         pkgconf_path_free(&client->filter_libdirs);
         pkgconf_path_free(&client->filter_includedirs);
 
diff --git a/libpkgconf/pkg.c b/libpkgconf/pkg.c
index 59957ff..46d562b 100644
--- a/libpkgconf/pkg.c
+++ b/libpkgconf/pkg.c
@@ -409,17 +409,46 @@ pkgconf_pkg_new_from_file(pkgconf_client_t *client, const char *filename, FILE *
 void
 pkgconf_pkg_free(pkgconf_client_t *client, pkgconf_pkg_t *pkg)
 {
-  // The function leaks (issue #132 is reported). The whole concept of "static"
-  // packages is quite murky, so it's better just not to use it, at least
-  // until fixed by the library owners. In particular don't use
-  // pkgconf_queue_* functions.
-  //
+  /*
+   * The function leaks (issue #132 is reported). The whole concept of "static"
+   * packages is quite murky, so it's better just not to use it, at least
+   * until fixed by the library owners. In particular don't use
+   * pkgconf_queue_* functions.
+   */
   assert (pkg == NULL || (pkg->flags & PKGCONF_PKG_PROPF_STATIC) == 0);
 
 	if (pkg == NULL || pkg->flags & PKGCONF_PKG_PROPF_STATIC)
 		return;
 
-	pkgconf_cache_remove(client, pkg);
+        /*
+         * Note that if a package is loaded by the file path it is not cached
+         * (see pkgconf_pkg_find() for details). Trying to remove such a
+         * package from the cache just breaks the cache (issue #133 is
+         * reported). So let's first check if the package in the cache.
+         *
+         * Generally it's quite murky that the function that frees the
+         * reference-countable object also removes it's pointer from cache that
+         * must own a reference to it (see how pkgconf_cache_add() increments
+         * the reference count). It sounds that by the time the object is
+         * freed the reference count should be zero and so nobody, including
+         * cache, can contain a reference to the object.
+         *
+         * While probably nothing should surprise in the world, where
+         * pkgconf_pkg_free() and pkgconf_pkg_unref() are both available to the
+         * client (quite widelly used both in the library's code) and where
+         * pkgconf_pkg_unref() assumes the reference count can be negative.
+         */
+	pkgconf_node_t *node;
+	PKGCONF_FOREACH_LIST_ENTRY(client->pkg_cache.head, node)
+	{
+          pkgconf_pkg_t *p = node->data;
+
+          if (p == pkg)
+          {
+            pkgconf_cache_remove(client, pkg);
+            break;
+          }
+        }
 
 	pkgconf_dependency_free(&pkg->requires);
 	pkgconf_dependency_free(&pkg->requires_private);
@@ -681,10 +710,6 @@ pkgconf_pkg_find(pkgconf_client_t *client, const char *name)
 			if (pkg != NULL)
 			{
 				pkgconf_path_add(pkg_get_parent_dir(pkg), &client->dir_list, true);
-
-                                // Add the package to the cache (issue #133 is reported).
-                                //
-                                pkgconf_cache_add(client, pkg);
 				return pkg;
 			}
 		}