Re: [PATCH 13/41] RFC system/ioport: move embedded memory region cleanup to portio_list_destroy

[Peter Xu <peterx@redhat.com>]

On Mon, Apr 27, 2026 at 11:42:12PM +0400, Marc-André Lureau wrote:
> memory_region_portio_list_finalize() unconditionally calls
> object_unref() on the embedded MemoryRegion, but when the object was
> created via bare object_new() the MemoryRegion was never initialized
> and has a refcount of 0, triggering an assertion failure.
> 
> The unref in finalize drops the keep-alive reference added during the
> reparenting in portio_list_add_1(). Move it to portio_list_destroy()
> where the cleanup belongs, and assert the refcount is zero in
> finalize.
> 
> This is an alternative to "system/ioport: Fix qom-list-properties crash
> on portio list obj" patch, hence the RFC.
> 
> Fixes: 690705ca0b0f ("softmmu/ioport.c: make MemoryRegionPortioList owner of portio_list MemoryRegions")
> Cc: peterx@redhat.com
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  system/ioport.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/system/ioport.c b/system/ioport.c
> index 9209bff2eab..aa8f3229804 100644
> --- a/system/ioport.c
> +++ b/system/ioport.c
> @@ -145,6 +145,7 @@ void portio_list_destroy(PortioList *piolist)
>      for (i = 0; i < piolist->nr; ++i) {
>          mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr);
>          object_unparent(OBJECT(&mrpio->mr));
> +        object_unref(OBJECT(&mrpio->mr));
>          object_unref(mrpio);
>      }
>      g_free(piolist->regions);
> @@ -346,7 +347,8 @@ static void memory_region_portio_list_finalize(Object *obj)
>  {
>      MemoryRegionPortioList *mrpio = MEMORY_REGION_PORTIO_LIST(obj);
>  
> -    object_unref(&mrpio->mr);
> +    /* dropped by portio_list_destroy(), or never initialized */
> +    assert(!mrpio->mr.parent_obj.ref);

I'm not sure if it's worthwhile to just add such assert and violate the
layering here, by accessing mr's refcount. Shall we just drop this check?

OTOH, if we will move the unref to destroy, shall we move the ports below
over and just go ahead removing the whole finalize()?

>      g_free(mrpio->ports);
>  }
>  
> 
> -- 
> 2.53.0
> 

-- 
Peter Xu