diff options
| -rw-r--r-- | libpkgconf/client.c | 5 | ||||
| -rw-r--r-- | libpkgconf/pkg.c | 45 |
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; } } |