flatpak / libportal

libportal - Flatpak portal library
https://libportal.org
GNU Lesser General Public License v3.0
80 stars 39 forks source link

test_close_session_signal intermittently failing #166

Closed smcv closed 2 weeks ago

smcv commented 2 months ago

I'm seeing intermittent test failures with 0.8.0 (example full log) which still seem to be present in 0.8.1:

=================================== FAILURES ===================================
_________________ TestRemoteDesktop.test_close_session_signal __________________

self = <pyportaltest.test_remotedesktop.TestRemoteDesktop testMethod=test_close_session_signal>

    def test_close_session_signal(self):
        """
        Ensure that we get the GObject signal when our session is closed
        externally.
        """
        params = {"close-after-start": 500}
        setup = self.create_session(params=params)
        session = setup.session

        session_closed_signal_received = False

        def session_closed(session):
            nonlocal session_closed_signal_received
            session_closed_signal_received = True
            self.mainloop.quit()

        session.connect("closed", session_closed)

        self.mainloop.run()

>       assert session_closed_signal_received is True
E       assert False is True

pyportaltest/test_remotedesktop.py:599: AssertionError

I'm not sure I understand how this can happen: if session_closed didn't run, then how can we leave self.mainloop.run without having set session_closed_signal_received?

Is it possible that self.mainloop might sometimes be terminated by a leftover signal handler from a previous test, or something like that?

GeorgesStavracas commented 1 month ago

Hm, I was reasonably sure that commit a8f7b0e49e2f0aaa237467d7f970df22388468a9 would have fixed that - at least it made it unreproducible (so far) here.

@whot any idea what this could be?

smcv commented 1 month ago

This mainloop automatically quits after a fixed timeout [which is currently 2 seconds], but only on the first run

I wonder whether this is just too short, on an autobuilder system that might be heavily-loaded? Ideally each test that waits for an event should be explicitly stopping the main loop when that event has been received, so that the timeout in mainloop() is never reached unless something has gone wrong, which would mean it could be made longer (perhaps more like 20s than 2s).

I also wonder whether attributes of the PortalTest, like the _mainloop, its timeout handler, and any signal connections, could be bleeding from one test into the next (I'm not particularly familiar with dbusmock, so I don't know to what extent it recycles test objects between tests). Ideally we'd explicitly disconnect every timeout and signal handler after it has either fired or otherwise become irrelevant.

smcv commented 1 month ago

This doesn't seem to be a memory error, unlike #169. I was able to reproduce it with

LD_PRELOAD=libasan.so.8 ASAN_OPTIONS=detect_leaks=0 meson test -v -C ~/tmp/build/libportal/asan pytest --test-args '-x -s -k test_close_session_signal' --repeat=100 -j1

(it failed on the 22nd iteration) and there was no report of a use-after-free having happened.