From 7eb69d279d6da9a4fdccdc1505509260672250a4 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 25 Jan 2024 22:03:57 +0000 Subject: [PATCH] gibaseinfo: Break refcount cycle with GIRepository By shifting responsibility for ensuring that the lifetime of a `GIRepository` always exceeds the lifetime of any of its `GIBaseInfo`s to the user. Keeping a weak ref from each `GIBaseInfo` to its `GIRepository` would be too expensive (`GIBaseInfo`s are supposed to be cheap to create and destroy, as they are used within function calls in language bindings). Signed-off-by: Philip Withnall Fixes: #3234 --- girepository/gibaseinfo.c | 12 ++++++++---- girepository/girepository-private.h | 3 ++- girepository/girepository.c | 10 ++++++++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/girepository/gibaseinfo.c b/girepository/gibaseinfo.c index 7a12c811f..926b6bb73 100644 --- a/girepository/gibaseinfo.c +++ b/girepository/gibaseinfo.c @@ -126,9 +126,6 @@ gi_base_info_finalize (GIBaseInfo *self) if (self->ref_count != INVALID_REFCOUNT && self->container && self->container->ref_count != INVALID_REFCOUNT) gi_base_info_unref (self->container); - - if (self->ref_count != INVALID_REFCOUNT) - g_clear_object (&self->repository); } static void @@ -356,7 +353,14 @@ gi_info_new_full (GIInfoType type, if (container && container->ref_count != INVALID_REFCOUNT) gi_base_info_ref (info->container); - info->repository = g_object_ref (repository); + /* Don’t keep a strong ref, since the repository keeps a cache of #GIBaseInfos + * and holds refs on them. If we kept a ref here, there’d be a cycle. + * Don’t keep a weak ref either, as that would make creating/destroying a + * #GIBaseInfo noticeably more expensive, and infos are performance critical + * for bindings. + * As stated in the documentation, the mitigation here is to require the user + * to keep the #GIRepository alive longer than any of its #GIBaseInfos. */ + info->repository = repository; return (GIBaseInfo*)info; } diff --git a/girepository/girepository-private.h b/girepository/girepository-private.h index aaeb97578..19a77262e 100644 --- a/girepository/girepository-private.h +++ b/girepository/girepository-private.h @@ -48,8 +48,9 @@ struct _GIBaseInfo GTypeInstance parent_instance; gatomicrefcount ref_count; - /* these are both reffed if the GIBaseInfo is heap-allocated, but not reffed if it’s stack-allocated */ + /* @repository is never reffed, as that would lead to a refcount cycle with the repository */ GIRepository *repository; + /* @container is reffed if the GIBaseInfo is heap-allocated, but not reffed if it’s stack-allocated */ GIBaseInfo *container; GITypelib *typelib; diff --git a/girepository/girepository.c b/girepository/girepository.c index 5beb830d1..f9619d072 100644 --- a/girepository/girepository.c +++ b/girepository/girepository.c @@ -42,6 +42,16 @@ * `GIRepository` is used to manage repositories of namespaces. Namespaces * are represented on disk by type libraries (`.typelib` files). * + * The individual pieces of API within a type library are represented by + * subclasses of [class@GIRepository.BaseInfo]. These can be found using + * methods like [method@GIRepository.Repository.find_by_name] or + * [method@GIRepository.Repository.get_info]. + * + * You are responsible for ensuring that the lifetime of the + * [class@GIRepository.Repository] exceeds that of the lifetime of any of its + * [class@GIRepository.BaseInfo]s. This cannot be guaranteed by using internal + * references within libgirepository as that would affect performance. + * * ### Discovery of type libraries * * `GIRepository` will typically look for a `girepository-1.0` directory