[PATCH 09/14] functional: ensure sockets and files are closed

Daniel P. Berrangé posted 14 patches 4 months ago
Maintainers: John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Elena Ufimtseva <elena.ufimtseva@oracle.com>, Jagannathan Raman <jag.raman@oracle.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
[PATCH 09/14] functional: ensure sockets and files are closed
Posted by Daniel P. Berrangé 4 months ago
The multiprocess and virtio_gpu tests open sockets but then forget
to close them, which triggers resource leak warnings

The virtio_gpu test also fails to close a log file it opens.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/test_multiprocess.py | 3 +++
 tests/functional/test_virtio_gpu.py   | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/tests/functional/test_multiprocess.py b/tests/functional/test_multiprocess.py
index 751cf10e63..92d5207b0e 100755
--- a/tests/functional/test_multiprocess.py
+++ b/tests/functional/test_multiprocess.py
@@ -83,6 +83,9 @@ def do_test(self, kernel_asset, initrd_asset,
                                           'cat /sys/bus/pci/devices/*/uevent',
                                           'PCI_ID=1000:0012')
 
+        proxy_sock.close()
+        remote_sock.close()
+
     def test_multiprocess(self):
         kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
         if self.arch == 'x86_64':
diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
index 81c9156d63..be96de24da 100755
--- a/tests/functional/test_virtio_gpu.py
+++ b/tests/functional/test_virtio_gpu.py
@@ -108,6 +108,7 @@ def test_vhost_user_vga_virgl(self):
             shell=False,
             close_fds=False,
         )
+        self._vug_log_file.close()
 
         self.vm.set_console()
         self.vm.add_args("-cpu", "host")
@@ -135,6 +136,7 @@ def test_vhost_user_vga_virgl(self):
                                           "features: +virgl +edid")
         self.vm.shutdown()
         qemu_sock.close()
+        vug_sock.close()
         vugp.terminate()
         vugp.wait()
 
-- 
2.49.0


Re: [PATCH 09/14] functional: ensure sockets and files are closed
Posted by Thomas Huth 4 months ago
On 15/07/2025 16.30, Daniel P. Berrangé wrote:
> The multiprocess and virtio_gpu tests open sockets but then forget
> to close them, which triggers resource leak warnings
> 
> The virtio_gpu test also fails to close a log file it opens.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/test_multiprocess.py | 3 +++
>   tests/functional/test_virtio_gpu.py   | 2 ++
>   2 files changed, 5 insertions(+)
> 
> diff --git a/tests/functional/test_multiprocess.py b/tests/functional/test_multiprocess.py
> index 751cf10e63..92d5207b0e 100755
> --- a/tests/functional/test_multiprocess.py
> +++ b/tests/functional/test_multiprocess.py
> @@ -83,6 +83,9 @@ def do_test(self, kernel_asset, initrd_asset,
>                                             'cat /sys/bus/pci/devices/*/uevent',
>                                             'PCI_ID=1000:0012')
>   
> +        proxy_sock.close()
> +        remote_sock.close()
> +
>       def test_multiprocess(self):
>           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
>           if self.arch == 'x86_64':
> diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
> index 81c9156d63..be96de24da 100755
> --- a/tests/functional/test_virtio_gpu.py
> +++ b/tests/functional/test_virtio_gpu.py
> @@ -108,6 +108,7 @@ def test_vhost_user_vga_virgl(self):
>               shell=False,
>               close_fds=False,
>           )
> +        self._vug_log_file.close()

Maybe cleaner to close it at the end of the function?

  Thomas


>           self.vm.set_console()
>           self.vm.add_args("-cpu", "host")
> @@ -135,6 +136,7 @@ def test_vhost_user_vga_virgl(self):
>                                             "features: +virgl +edid")
>           self.vm.shutdown()
>           qemu_sock.close()
> +        vug_sock.close()
>           vugp.terminate()
>           vugp.wait()
>   


Re: [PATCH 09/14] functional: ensure sockets and files are closed
Posted by Daniel P. Berrangé 4 months ago
On Tue, Jul 15, 2025 at 05:03:07PM +0200, Thomas Huth wrote:
> On 15/07/2025 16.30, Daniel P. Berrangé wrote:
> > The multiprocess and virtio_gpu tests open sockets but then forget
> > to close them, which triggers resource leak warnings
> > 
> > The virtio_gpu test also fails to close a log file it opens.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >   tests/functional/test_multiprocess.py | 3 +++
> >   tests/functional/test_virtio_gpu.py   | 2 ++
> >   2 files changed, 5 insertions(+)
> > 
> > diff --git a/tests/functional/test_multiprocess.py b/tests/functional/test_multiprocess.py
> > index 751cf10e63..92d5207b0e 100755
> > --- a/tests/functional/test_multiprocess.py
> > +++ b/tests/functional/test_multiprocess.py
> > @@ -83,6 +83,9 @@ def do_test(self, kernel_asset, initrd_asset,
> >                                             'cat /sys/bus/pci/devices/*/uevent',
> >                                             'PCI_ID=1000:0012')
> > +        proxy_sock.close()
> > +        remote_sock.close()
> > +
> >       def test_multiprocess(self):
> >           kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
> >           if self.arch == 'x86_64':
> > diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
> > index 81c9156d63..be96de24da 100755
> > --- a/tests/functional/test_virtio_gpu.py
> > +++ b/tests/functional/test_virtio_gpu.py
> > @@ -108,6 +108,7 @@ def test_vhost_user_vga_virgl(self):
> >               shell=False,
> >               close_fds=False,
> >           )
> > +        self._vug_log_file.close()
> 
> Maybe cleaner to close it at the end of the function?

This was the last place it was needed - just out of sight in the context
of the diff the file is passed to Popen() which connects it to stderr.
So nothing in this test may access the file after this point to avoid
concurent overlapping writes to the same file.

> 
>  Thomas
> 
> 
> >           self.vm.set_console()
> >           self.vm.add_args("-cpu", "host")
> > @@ -135,6 +136,7 @@ def test_vhost_user_vga_virgl(self):
> >                                             "features: +virgl +edid")
> >           self.vm.shutdown()
> >           qemu_sock.close()
> > +        vug_sock.close()
> >           vugp.terminate()
> >           vugp.wait()
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH 09/14] functional: ensure sockets and files are closed
Posted by Thomas Huth 4 months ago
On 15/07/2025 17.12, Daniel P. Berrangé wrote:
> On Tue, Jul 15, 2025 at 05:03:07PM +0200, Thomas Huth wrote:
>> On 15/07/2025 16.30, Daniel P. Berrangé wrote:
>>> The multiprocess and virtio_gpu tests open sockets but then forget
>>> to close them, which triggers resource leak warnings
>>>
>>> The virtio_gpu test also fails to close a log file it opens.
>>>
>>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>>> ---
>>>    tests/functional/test_multiprocess.py | 3 +++
>>>    tests/functional/test_virtio_gpu.py   | 2 ++
>>>    2 files changed, 5 insertions(+)
>>>
>>> diff --git a/tests/functional/test_multiprocess.py b/tests/functional/test_multiprocess.py
>>> index 751cf10e63..92d5207b0e 100755
>>> --- a/tests/functional/test_multiprocess.py
>>> +++ b/tests/functional/test_multiprocess.py
>>> @@ -83,6 +83,9 @@ def do_test(self, kernel_asset, initrd_asset,
>>>                                              'cat /sys/bus/pci/devices/*/uevent',
>>>                                              'PCI_ID=1000:0012')
>>> +        proxy_sock.close()
>>> +        remote_sock.close()
>>> +
>>>        def test_multiprocess(self):
>>>            kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
>>>            if self.arch == 'x86_64':
>>> diff --git a/tests/functional/test_virtio_gpu.py b/tests/functional/test_virtio_gpu.py
>>> index 81c9156d63..be96de24da 100755
>>> --- a/tests/functional/test_virtio_gpu.py
>>> +++ b/tests/functional/test_virtio_gpu.py
>>> @@ -108,6 +108,7 @@ def test_vhost_user_vga_virgl(self):
>>>                shell=False,
>>>                close_fds=False,
>>>            )
>>> +        self._vug_log_file.close()
>>
>> Maybe cleaner to close it at the end of the function?
> 
> This was the last place it was needed - just out of sight in the context
> of the diff the file is passed to Popen() which connects it to stderr.
> So nothing in this test may access the file after this point to avoid
> concurent overlapping writes to the same file.

Ok, makes sense now.

Reviewed-by: Thomas Huth <thuth@redhat.com>