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

[Thomas Huth <thuth@redhat.com>]

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>