qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 02/10] rust: qom: add reference counting functionality


From: Paolo Bonzini
Subject: Re: [PATCH 02/10] rust: qom: add reference counting functionality
Date: Wed, 29 Jan 2025 11:03:31 +0100

On Sun, Jan 26, 2025 at 3:56 PM Zhao Liu <zhao1.liu@intel.com> wrote:
>
> Hi Paolo,
>
> On Fri, Jan 17, 2025 at 08:39:55PM +0100, Paolo Bonzini wrote:
> > Date: Fri, 17 Jan 2025 20:39:55 +0100
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > Subject: [PATCH 02/10] rust: qom: add reference counting functionality
> > X-Mailer: git-send-email 2.47.1
> >
> > Add a smart pointer that allows to add and remove references from
> > QOM objects.  It's important to note that while all QOM objects have a
> > reference count, in practice not all of them have their lifetime guarded
> > by it.
>
> About the background, I have a maybe common question...why Rust needs
> extra reference count guarding?

Children properties are removed, and thus their reference is dropped,
before instance_finalize() is called (see object_finalize() in
qom/object.c). This is not valid in Rust, you need to keep the object
alive until the last line of Rust code has run - which is after
Drop::drop() has run.

> Additionally, I felt that the ref count may be a bit confusing. After
> creating Child<> property, the child object's ref count is sometimes 1,
> and other times it's 2:
>
>  * With object_initialize_child(), child's ref count is 1.
>
>  * With object_property_add_child() (usually after a object_new() to
>    create child first):
>
>    - sometimes user will call object_unref(), and then the ref count is 1.
>      E.g., x86_cpu_apic_create() in target/i386/cpu-apic.c.
>
>    - sometimes no object_unref(), then ref count is 2.
>      E.g., exynos4210_realize() in hw/arm/exynos4210.c, creats "cortex-a9".

In C, having a ref count of 2 is usually a latent memory leak (because
most of the time there's not going to be an object_unref() in the
instance_finalize() method). In this case the leak is latent, because
TYPE_EXYNOS4210_SOC is not hot-unpluggable and thus will never really
go away once realized.

In Rust, this class of leaks simply does not exist with the right API.
ObjectMethods::property_add_child() could either:

- take an Owned<T> and consume it, thus always giving a ref count of 1
on exit. If you want to keep the object you would have to clone it.

- take "&'owner self, &'child T where 'owner: 'child", then you can
pass an embedded object like object_initialize_child().

In the latter case however you *still* need to keep the reference
count elevated until Drop runs. That is, unlike C, Rust code will
always have a ref count of 2 for children. For this reason, instead of
having a "T" in the struct you would have another wrapper---something
like Child<'owner, T>. This wrapper cannot be cloned but it does an
unref when dropped.

My expectation is that property_add_child() will be used exclusivel
for the first case, i.e. it will take an Owned<T>. If you want to
create a child property from an embedded object, something like
object_initialize_child() can be used once pinned-init is used to
rewrite how instance_init is used. It will look something like

pin_init! {
  &this in MyClass {
    ...,
    iomem <- MemoryRegion::init_io(
            this,
            &MY_MR_OPS,
            "memory-region-name",
            0x1000,
    ),
    child_obj <- ChildClass::init().to_child(this, "prop-name")
  }
}

where to_child() wraps an "impl PinInit<T>" and turns it into an "impl
PinInit<Child<'a, T>>". Or something like that. :)

> From this description, I understand your goal is:
>
>  * For embedded child object, its lifetimer is managed by its parent
>    object, through Child<> for the most cases.
>
>  * For non-embedded child - a pointer/reference in parent object, its
>    lifetimer is managed by `Owned<>` (and with Child<>).
>
> Am I right?

Yes, you're right.

I am not sure if you meant Child<> as the QOM concept, or as a Rust
struct. If the latter, you're really really right.

Paolo




reply via email to

[Prev in Thread] Current Thread [Next in Thread]