<div dir="ltr"><div>Sounds good - thank you...</div><div><br></div><div>Patch is</div><div><br></div><div>Signed-off-by: Mark Mielke <<a href="mailto:mark.mielke@gmail.com">mark.mielke@gmail.com</a>> <br></div><div><br></div><div dir="ltr"><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 13, 2022 at 5:08 AM Ján Tomko <<a href="mailto:jtomko@redhat.com">jtomko@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On a Sunday in 2022, Mark Mielke wrote:<br>
>This is a partial revert of 87a43a907f0ad4897a28ad7c216bc70f37270b93.<br>
<br>
I'll drop the trailing period before pushing.<br>
<br>
><br>
>The change to use g_clear_pointer() in more places was accidentally<br>
>applied to cases involving vir_g_source_unref().<br>
><br>
>In some cases, the ordering of g_source_destroy() and<br>
>vir_g_source_unref() was reversed, which resulted in the source being<br>
>marked as destroyed, after it is already unreferenced.<br>
<br>
Oops, sorry I missed that during review.<br>
<br>
>This<br>
>use-after-free case might work in many cases, but with versions of<br>
>glibc older than 2.64.0 it may defer unref to run within the main<br>
<br>
s/glibc/glib/<br>
<br>
>thread to avoid a race condition, which creates a large distance<br>
>between the g_source_unref() and g_source_destroy().<br>
><br>
>In some cases, the call to vir_g_source_unref() was replaced with a<br>
>second call to g_source_destroy(), leading to a memory leak or worse.<br>
><br>
>In our experience, the symptoms were that use of libvirt-python became<br>
>slower over time, with OpenStack nova-compute initially taking around<br>
>one second to periodically query the host PCI devices, and within an<br>
>hour it was taking over a minute to complete the same operation, until<br>
>it is was eventually running this query back-to-back, resulting in the<br>
>nova-compute process consuming 100% of one CPU thread, losing its<br>
>RabbitMQ connection frequently, and showing up as down to the control<br>
>plane.<br>
<br>
Your patch is missing a sign-off<br>
<a href="https://libvirt.org/hacking.html#developer-certificate-of-origin" rel="noreferrer" target="_blank">https://libvirt.org/hacking.html#developer-certificate-of-origin</a><br>
<br>
Just replying to this e-mail with the Signed-off-by line is enough - no<br>
need to resend the patch. I'll push it with the sign-off included after<br>
the pipeline succeds: <a href="https://gitlab.com/janotomko/libvirt/-/pipelines/562139546" rel="noreferrer" target="_blank">https://gitlab.com/janotomko/libvirt/-/pipelines/562139546</a><br>
<br>
>---<br>
> src/qemu/qemu_agent.c   |  3 ++-<br>
> src/qemu/qemu_monitor.c |  3 ++-<br>
> src/util/vireventglib.c | 12 ++++++++----<br>
> 3 files changed, 12 insertions(+), 6 deletions(-)<br>
><br>
<br>
Reviewed-by: Ján Tomko <<a href="mailto:jtomko@redhat.com" target="_blank">jtomko@redhat.com</a>><br>
<br>
Jano<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature">Mark Mielke <<a href="mailto:mark.mielke@gmail.com" target="_blank">mark.mielke@gmail.com</a>><br><br></div></div>