pytest-dev / pytest-qt

pytest plugin for Qt (PyQt5/PyQt6 and PySide2/PySide6) application testing
https://pytest-qt.readthedocs.io
MIT License
410 stars 70 forks source link

QDBusConnection.systemBus() leaks thread after qapp teardown #546

Closed kalvdans closed 8 months ago

kalvdans commented 8 months ago

We have an unit test that connects to dbus with QDBusConnection.systemBus(), and it seems to create a thread that exist even after the unit test has been torn down. This disturbs other completely unrelated tests later on.

Small reproducer:

import pytest
from PyQt5.QtDBus import QDBusConnection

def n_threads_by_os():
    with open("/proc/self/stat") as f:
        line = f.read()
    return int(line.split()[19])

@pytest.fixture
def check_threads():
    threads_before = n_threads_by_os()
    yield
    assert n_threads_by_os() == threads_before

def test_dbus(check_threads, qapp):
    QDBusConnection.systemBus()

Running gives output:

================================ test session starts =================================
platform linux -- Python 3.12.2, pytest-8.0.1, pluggy-1.4.0
PyQt5 5.15.10 -- Qt runtime 5.15.2 -- Qt compiled 5.15.2
rootdir: /home/chn/reproducer
plugins: env-1.1.3, html-4.1.1, qt-4.4.0, metadata-3.1.1, cov-4.1.0, typeguard-4.1.5, leaks-0.3.1
collected 1 item                                                                     

qapp-threadleak.py .E                                                          [100%]

======================================= ERRORS =======================================
___________________________ ERROR at teardown of test_dbus ___________________________

    @pytest.fixture
    def check_threads():
        threads_before = n_threads_by_os()
        yield
>       assert n_threads_by_os() == threads_before
E       assert 3 == 2
E        +  where 3 = n_threads_by_os()

qapp-threadleak.py:14: AssertionError
============================== short test summary info ===============================
ERROR qapp-threadleak.py::test_dbus - assert 3 == 2
============================= 1 passed, 1 error in 0.09s =============================

Please tell me if you think this should be reported to QT instead.

nicoddemus commented 8 months ago

Hi,

According to the docs:

The object reference returned by this function is valid until the QCoreApplication's destructor is run, when the connection will be closed and the object, deleted.

So it seems the behavior is as expected: the connection/thread will stay open until P QCoreApplication` is destroyed, which will only happen at the end of the session. I don't think this is a bug in QT either, just the expected behavior.

Closing this for now, feel free to follow up with other questions.

kalvdans commented 8 months ago

Thanks @nicoddemus ! I thought the qapp fixture did a proper teardown but I see now in the source code that it keeps the QApplication object alive forever in a global variable.

Do you have any advice how to make our tests more isolated? Will you accept a pull request making QApplication live shorter or is that playing with fire?

nicoddemus commented 8 months ago

I thought the qapp fixture did a proper teardown but I see now in the source code that it keeps the QApplication object alive forever in a global variable.

Yes, this is due to a limitation within Qt itself, you cannot destroy and recreate QApplication within the same process more than once.

Do you have any advice how to make our tests more isolated?

In general it is good for tests to cleanup after themselves, (for example using a fixture which stops a thread started during the test), but I'm not sure this can be done with systemBus(), as it explicitly states the thread will only be closed when QApplication is destroyed...

Will you accept a pull request making QApplication live shorter or is that playing with fire?

As I mentioned this is not really possible, I'm afraid.