aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKaren Arutyunov <karen@codesynthesis.com>2017-09-12 17:43:16 +0300
committerKaren Arutyunov <karen@codesynthesis.com>2017-09-12 17:43:16 +0300
commitde58cf32b39f0a996895da62dc82defaaf8fdc3c (patch)
treeb8ca9ed3120ba782319a29c2570d9c727775f5d1
parent53a76972f56688f4e0c7889e295d87b91ae85bc5 (diff)
Redo package cache breakage fix
-rw-r--r--libpkgconf/client.c5
-rw-r--r--libpkgconf/pkg.c45
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;
}
}