"RuntimeError: Failed to disconnect" with PySide6==6.7.0
#552
Comments
bersbersbers
changed the title
PySide6==6.7.0
|
It is probably because the exception is being raised inside the Qt event loop. |
|
I'm seeing the same here: =================================== FAILURES ===================================
________________________________ test_destroyed ________________________________
CALL ERROR: Exceptions caught in Qt event loop:
________________________________________________________________________________
RuntimeError: Internal C++ object (Obj) already deleted.
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/builddir/build/BUILDROOT/python-pytest-qt-4.4.0-2.fc41.x86_64/usr/lib/python3.12/site-packages/pytestqt/wait_signal.py", line 219, in _quit_loop_by_signal
self._cleanup()
File "/builddir/build/BUILDROOT/python-pytest-qt-4.4.0-2.fc41.x86_64/usr/lib/python3.12/site-packages/pytestqt/wait_signal.py", line 226, in _cleanup
_silent_disconnect(signal, self._quit_loop_by_signal)
File "/builddir/build/BUILDROOT/python-pytest-qt-4.4.0-2.fc41.x86_64/usr/lib/python3.12/site-packages/pytestqt/wait_signal.py", line 741, in _silent_disconnect
signal.disconnect(slot)
SystemError: <class 'RuntimeError'> returned a result with an exception set
________________________________________________________________________________
----------------------------- Captured stderr call -----------------------------
Exceptions caught in Qt event loop:
________________________________________________________________________________
RuntimeError: Internal C++ object (Obj) already deleted.
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/builddir/build/BUILDROOT/python-pytest-qt-4.4.0-2.fc41.x86_64/usr/lib/python3.12/site-packages/pytestqt/wait_signal.py", line 219, in _quit_loop_by_signal
self._cleanup()
File "/builddir/build/BUILDROOT/python-pytest-qt-4.4.0-2.fc41.x86_64/usr/lib/python3.12/site-packages/pytestqt/wait_signal.py", line 226, in _cleanup
_silent_disconnect(signal, self._quit_loop_by_signal)
File "/builddir/build/BUILDROOT/python-pytest-qt-4.4.0-2.fc41.x86_64/usr/lib/python3.12/site-packages/pytestqt/wait_signal.py", line 741, in _silent_disconnect
signal.disconnect(slot)
SystemError: <class 'RuntimeError'> returned a result with an exception set
________________________________________________________________________________That test succeeded with |
|
A Perhaps it would be good create a MWE and post this to PySide's tracker. |
|
Well, one MWE (without from PySide6.QtCore import Signal
from PySide6.QtWidgets import QApplication, QMainWindow
class Window(QMainWindow):
signal = Signal()
def __init__(self):
super().__init__()
self.signal.connect(self.Slot)
self.signal.disconnect(self.Slot)
self.signal.disconnect(self.Slot)
def Slot(self): ...
app = QApplication()
window = Window()
print("Done.")But I somehow feel reporting this to PySide will give us the question "why are you disconnecting twice"? :) I think the comment in line 738 is relevant here - is that what you have in mind? pytest-qt/src/pytestqt/wait_signal.py Lines 736 to 743 in aac9be7 I don't suppose you remember which code example made you do 86e41e3 :) |
|
Okay, here's a real one: from PySide6.QtWidgets import QWidget
def test_disconnect(qtbot):
widget = QWidget()
qtbot.addWidget(widget)
_ = qtbot.waitSignal(widget.windowTitleChanged, timeout=10000)
widget.windowTitleChanged.emit("") |
|
And a workaround: from PySide6.QtWidgets import QWidget
def test_disconnect(qtbot):
widget = QWidget()
qtbot.addWidget(widget)
_ = qtbot.waitSignal(widget.windowTitleChanged, timeout=1)
del _
widget.windowTitleChanged.emit("")Edit: yes, that fixes the issue in my big app. Edit: another workaround is just to not assign to @nicoddemus, do you think this is reportable to PySide in this form, or will they want to see something without pytest-qt? Signal disconnect behaviors have been subject of bug fixing for the last 10 or so releases... |
|
@bersbersbers sorry for the delay. The problem is:
Here is a MWE which shows the problem, without using import warnings
from PySide6.QtCore import Signal
from PySide6.QtWidgets import QApplication, QMainWindow
warnings.filterwarnings('ignore')
class Window(QMainWindow):
signal = Signal()
def __init__(self):
super().__init__()
self.signal.connect(self.Slot)
self.signal.disconnect(self.Slot)
self.signal.disconnect(self.Slot)
def Slot(self): ...
warnings.warn(Warning("some warning"))
app = QApplication()
window = Window()
print("Done")While we should see no warnings at all due to As a sanity check, if we comment out |
|
@nicoddemus thanks - and no worries about the delay. I have found a workaround for my issue, and will work on getting this solved only as a bonus for others, so I have no strict timeline here. In summary, I see two issues:
I will report both issues to PySide and updates this thread here. |
|
Thanks! I will close for now, but looking forward to the follow ups. 👍 |
|
Follow https://bugreports.qt.io/projects/PYSIDE/issues/PYSIDE-2705 if interested. |
|
Thanks! Watching that issue. 👍 |
|
(Hopefully) fixed with PySide6 6.7.1 and 6.8.0. |
|
I tested this on an early 6.7.1 ( |
|
yeah, using |
|
I might - however, I came across this note:
This means it might impact the user's own warnings filters, which is not ideal. |
But this is temporary, it will not be a problem because the context manager will be short-lived: def _silent_disconnect(signal, slot):
"""Disconnects a signal from a slot, ignoring errors. Sometimes
Qt might disconnect a signal automatically for unknown reasons.
"""
with warnings.catch_warnings():
# PySide 6.7+ issues a UserWarning instead of an exception.
warnings.filterwarnings("ignore", category=UserWarning)
try:
signal.disconnect(slot)
except (TypeError, RuntimeError): # pragma: no cover
pass |
|
Well, imagine this situation: Thread 1: with warnings.catch_warnings():
warnings.filterwarnings("ignore", category=UserWarning)
thread_1_work()Thread 2: with warnings.catch_warnings():
warnings.filterwarnings("ignore", category=OtherWarning)
thread_2_work()What this is equivalent too, roughly: Thread 1: saved_warning_state_1 = global_warning_state
warnings.filterwarnings("ignore", category=UserWarning) # changes global_warning_state
thread_1_work()
global_warning_state = saved_warning_state_1Thread 2: saved_warning_state_2 = global_warning_state
warnings.filterwarnings("ignore", category=OtherWarning) # changes global_warning_state
thread_2_work()
global_warning_state = saved_warning_state_2And these two run concurrently - for example: saved_warning_state_1 = global_warning_state
warnings.filterwarnings("ignore", category=UserWarning) # changes global_warning_state
thread_1_work()
saved_warning_state_2 = global_warning_state
global_warning_state = saved_warning_state_1
warnings.filterwarnings("ignore", category=OtherWarning) # changes global_warning_state
thread_2_work()
global_warning_state = saved_warning_state_2
# Still ignoring UserWarning |
|
pytest is not multi-thread, so this is not a problem. Also, pytest itself uses |
That is true. But what if pytest is used to test multi-threaded code? |
The-Compiler
added a commit
that referenced
this issue
Mar 22, 2025
- Always create the timeout `QTimer`, even if it's not going to be used.
- Connect the signal once in `_AbstractSignalBlocker`/`CallbackBlocker.__init__`
and never disconnect it.
- When checking whether a timeout was set, simply check `self._timeout` instead of
`self._timer`.
Before this change, we conditionally created a `QTimer` and deleted it again
when cleaning up. In addition to coming with hard to follow complexity, this
also caused multiple issues around this timer:
Warnings about disconnecting signal
===================================
In `AbstractSignalBlocker._cleanup`, we attempted to disconnect
`self._timer.timeout()` from `self._quit_loop_by_timeout`, under the condition that
a timer was created in `__init__` (i.e. `self.timeout` was not `0` or `None`).
However, the signal only got connected in `AbstractSignalBlocker.wait()`.
Thus, if `AbstractSignalBlocker` is used as a context manager with a timeout and
the signal is emitted inside it:
- `self._timer` is present
- The signal calls `self._quit_loop_by_signal()`
- `self._cleanup()` gets called from there
- That tries to disconnect `self._timer.timeout()` from `self._quit_loop_by_timeout()`
- Since `self.wait()` was never called, the signal was never connected
- Which then results in either an exception (PyQt), an internal SystemError
(older PySide6) or a warning (newer PySide6).
In 560f565 and later
81b317c this was fixed by ignoring
`TypeError`/`RuntimeError`, but that turned out to not be sufficient for newer
PySide versions: #552, #558.
The proper fix for this is to not attempt to disconnect a signal that was never
connected, which makes all the PySide6 warnings go away (and thus we can run our
testsuite with `-Werror` now).
As a drive-by fix, this also removes the old `filterwarnings` mark definition,
which was introduced in 95fee8b (perhaps as a
stop-gap for older pytest versions?) but shouldn't be needed anymore (and is
unused since e338809).
AttributeError when a signal/callback is emitted from a different thread
========================================================================
If a signal is emitted from a thread, `_AbstractSignalBlocker._cleanup()` also gets
called from the non-main thread (Qt will complain: "QObject::killTimer: Timers
cannot be stopped from another thread").
If the signal emission just happened to happen between the None-check and
calling `.start()` in `wait()`, it failed with an `AttributeError`:
Main thread in `AbstractSignalBlocker.wait()`:
```python
if self._timer is not None: # <--- this was true...
self._timer.timeout.connect(self._quit_loop_by_timeout)
# <--- ...now here, but self._timer is None now!
self._timer.start()
````
Emitting thread in `AbstractSignalBlocker._cleanup()` via
`._quit_loop_by_signal()`:
```python
if self._timer is not None:
...
self._timer = None # <--- here
```
In SignalBlocker.connect, we used:
```python
actual_signal.connect(self._quit_loop_by_signal)
```
which by default is supposed to use a `QueuedConnection` if the signal gets
emitted in a different thread:
https://doc.qt.io/qt-6/qt.html#ConnectionType-enum
(Default) If the receiver lives in the thread that emits the signal,
`Qt::DirectConnection` is used. Otherwise, `Qt::QueuedConnection` is used. The
connection type is determined when the signal is emitted.
though then that page says:
https://doc.qt.io/qt-6/qobject.html#thread-affinity
Note: If a `QObject` has no thread affinity (that is, if `thread()` returns
zero), or if it lives in a thread that has no running event loop, then it
cannot receive queued signals or posted events.
Which means `AbstractSignalBlocker` needs to be a `QObject` for this to work.
However, that means we need to use `qt_api.QtCore.QObject` as subclass, i.e. at
import time of `wait_signal.py`. Yet, `qt_api` only gets initialized in
`pytest_configure` so that it's configurable via a pytest setting.
Unfortunately, doing that is tricky, as it means we can't import
`wait_signal.py` at all during import time, and people might do so for e.g. type
annotations.
With this refactoring, the `AttributeError` is out of the way, though there are
other subtle failures with multi-threaded signals now:
1) `_quit_loop_by_signal()` -> `_cleanup()` now simply calls `self._timer.stop()`
without setting `self._timer` to `None`. This still results in the same Qt
message quoted above (after all, the timer still doesn't belong to the calling
thread!), but it looks like the test terminates without any timeout anyways.
From what I can gather, while the "low level timer" continues to run (and
waste a minimal amount of resources), the QTimer still "detaches" from it and
stops running. The commit adds a test to catch this case (currently marked as
xfail).
2) The main thread in `wait()` can now still call `self._timer.start()` without
an `AttributeError`. However, in theory this could restart the timer after it
was already stopped by the signal emission, with a race between `_cleanup()` and
`wait()`.
See #586. This fixes the test-case posted by the reporter (added to the
testsuite in a simplified version), but not the additional considerations above.
The same fix is also applied to `CallbackBlocker`, though the test there is way
more unreliable in triggering the issue, and thus is skipped for taking too
long.
The-Compiler
added a commit
that referenced
this issue
Mar 22, 2025
- Always create the timeout `QTimer`, even if it's not going to be used.
- Connect the signal once in `_AbstractSignalBlocker`/`CallbackBlocker.__init__`
and never disconnect it.
- When checking whether a timeout was set, simply check `self._timeout` instead of
`self._timer`.
Before this change, we conditionally created a `QTimer` and deleted it again
when cleaning up. In addition to coming with hard to follow complexity, this
also caused multiple issues around this timer:
Warnings about disconnecting signal
===================================
In `AbstractSignalBlocker._cleanup`, we attempted to disconnect
`self._timer.timeout()` from `self._quit_loop_by_timeout`, under the condition that
a timer was created in `__init__` (i.e. `self.timeout` was not `0` or `None`).
However, the signal only got connected in `AbstractSignalBlocker.wait()`.
Thus, if `AbstractSignalBlocker` is used as a context manager with a timeout and
the signal is emitted inside it:
- `self._timer` is present
- The signal calls `self._quit_loop_by_signal()`
- `self._cleanup()` gets called from there
- That tries to disconnect `self._timer.timeout()` from `self._quit_loop_by_timeout()`
- Since `self.wait()` was never called, the signal was never connected
- Which then results in either an exception (PyQt), an internal SystemError
(older PySide6) or a warning (newer PySide6).
In 560f565 and later
81b317c this was fixed by ignoring
`TypeError`/`RuntimeError`, but that turned out to not be sufficient for newer
PySide versions: #552, #558.
The proper fix for this is to not attempt to disconnect a signal that was never
connected, which makes all the PySide6 warnings go away (and thus we can run our
testsuite with `-Werror` now).
As a drive-by fix, this also removes the old `filterwarnings` mark definition,
which was introduced in 95fee8b (perhaps as a
stop-gap for older pytest versions?) but shouldn't be needed anymore (and is
unused since e338809).
AttributeError when a signal/callback is emitted from a different thread
========================================================================
If a signal is emitted from a thread, `_AbstractSignalBlocker._cleanup()` also gets
called from the non-main thread (Qt will complain: "QObject::killTimer: Timers
cannot be stopped from another thread").
If the signal emission just happened to happen between the None-check and
calling `.start()` in `wait()`, it failed with an `AttributeError`:
Main thread in `AbstractSignalBlocker.wait()`:
```python
if self._timer is not None: # <--- this was true...
self._timer.timeout.connect(self._quit_loop_by_timeout)
# <--- ...now here, but self._timer is None now!
self._timer.start()
````
Emitting thread in `AbstractSignalBlocker._cleanup()` via
`._quit_loop_by_signal()`:
```python
if self._timer is not None:
...
self._timer = None # <--- here
```
In SignalBlocker.connect, we used:
```python
actual_signal.connect(self._quit_loop_by_signal)
```
which by default is supposed to use a `QueuedConnection` if the signal gets
emitted in a different thread:
https://doc.qt.io/qt-6/qt.html#ConnectionType-enum
(Default) If the receiver lives in the thread that emits the signal,
`Qt::DirectConnection` is used. Otherwise, `Qt::QueuedConnection` is used. The
connection type is determined when the signal is emitted.
though then that page says:
https://doc.qt.io/qt-6/qobject.html#thread-affinity
Note: If a `QObject` has no thread affinity (that is, if `thread()` returns
zero), or if it lives in a thread that has no running event loop, then it
cannot receive queued signals or posted events.
Which means `AbstractSignalBlocker` needs to be a `QObject` for this to work.
However, that means we need to use `qt_api.QtCore.QObject` as subclass, i.e. at
import time of `wait_signal.py`. Yet, `qt_api` only gets initialized in
`pytest_configure` so that it's configurable via a pytest setting.
Unfortunately, doing that is tricky, as it means we can't import
`wait_signal.py` at all during import time, and people might do so for e.g. type
annotations.
With this refactoring, the `AttributeError` is out of the way, though there are
other subtle failures with multi-threaded signals now:
1) `_quit_loop_by_signal()` -> `_cleanup()` now simply calls `self._timer.stop()`
without setting `self._timer` to `None`. This still results in the same Qt
message quoted above (after all, the timer still doesn't belong to the calling
thread!), but it looks like the test terminates without any timeout anyways.
From what I can gather, while the "low level timer" continues to run (and
waste a minimal amount of resources), the QTimer still "detaches" from it and
stops running. The commit adds a test to catch this case (currently marked as
xfail).
2) The main thread in `wait()` can now still call `self._timer.start()` without
an `AttributeError`. However, in theory this could restart the timer after it
was already stopped by the signal emission, with a race between `_cleanup()` and
`wait()`.
See #586. This fixes the test-case posted by the reporter (added to the
testsuite in a simplified version), but not the additional considerations above.
The same fix is also applied to `CallbackBlocker`, though the test there is way
more unreliable in triggering the issue, and thus is skipped for taking too
long.
|
This should be fixed properly with #596. |
The-Compiler
added a commit
that referenced
this issue
Mar 22, 2025
- Always create the timeout `QTimer`, even if it's not going to be used.
- Connect the signal once in `_AbstractSignalBlocker`/`CallbackBlocker.__init__`
and never disconnect it.
- When checking whether a timeout was set, simply check `self._timeout` instead of
`self._timer`.
Before this change, we conditionally created a `QTimer` and deleted it again
when cleaning up. In addition to coming with hard to follow complexity, this
also caused multiple issues around this timer:
Warnings about disconnecting signal
===================================
In `AbstractSignalBlocker._cleanup`, we attempted to disconnect
`self._timer.timeout()` from `self._quit_loop_by_timeout`, under the condition that
a timer was created in `__init__` (i.e. `self.timeout` was not `0` or `None`).
However, the signal only got connected in `AbstractSignalBlocker.wait()`.
Thus, if `AbstractSignalBlocker` is used as a context manager with a timeout and
the signal is emitted inside it:
- `self._timer` is present
- The signal calls `self._quit_loop_by_signal()`
- `self._cleanup()` gets called from there
- That tries to disconnect `self._timer.timeout()` from `self._quit_loop_by_timeout()`
- Since `self.wait()` was never called, the signal was never connected
- Which then results in either an exception (PyQt), an internal SystemError
(older PySide6) or a warning (newer PySide6).
In 560f565 and later
81b317c this was fixed by ignoring
`TypeError`/`RuntimeError`, but that turned out to not be sufficient for newer
PySide versions: #552, #558.
The proper fix for this is to not attempt to disconnect a signal that was never
connected, which makes all the PySide6 warnings go away (and thus we can run our
testsuite with `-Werror` now).
As a drive-by fix, this also removes the old `filterwarnings` mark definition,
which was introduced in 95fee8b (perhaps as a
stop-gap for older pytest versions?) but shouldn't be needed anymore (and is
unused since e338809).
AttributeError when a signal/callback is emitted from a different thread
========================================================================
If a signal is emitted from a thread, `_AbstractSignalBlocker._cleanup()` also gets
called from the non-main thread (Qt will complain: "QObject::killTimer: Timers
cannot be stopped from another thread").
If the signal emission just happened to happen between the None-check and
calling `.start()` in `wait()`, it failed with an `AttributeError`:
Main thread in `AbstractSignalBlocker.wait()`:
```python
if self._timer is not None: # <--- this was true...
self._timer.timeout.connect(self._quit_loop_by_timeout)
# <--- ...now here, but self._timer is None now!
self._timer.start()
````
Emitting thread in `AbstractSignalBlocker._cleanup()` via
`._quit_loop_by_signal()`:
```python
if self._timer is not None:
...
self._timer = None # <--- here
```
In SignalBlocker.connect, we used:
```python
actual_signal.connect(self._quit_loop_by_signal)
```
which by default is supposed to use a `QueuedConnection` if the signal gets
emitted in a different thread:
https://doc.qt.io/qt-6/qt.html#ConnectionType-enum
(Default) If the receiver lives in the thread that emits the signal,
`Qt::DirectConnection` is used. Otherwise, `Qt::QueuedConnection` is used. The
connection type is determined when the signal is emitted.
though then that page says:
https://doc.qt.io/qt-6/qobject.html#thread-affinity
Note: If a `QObject` has no thread affinity (that is, if `thread()` returns
zero), or if it lives in a thread that has no running event loop, then it
cannot receive queued signals or posted events.
Which means `AbstractSignalBlocker` needs to be a `QObject` for this to work.
However, that means we need to use `qt_api.QtCore.QObject` as subclass, i.e. at
import time of `wait_signal.py`. Yet, `qt_api` only gets initialized in
`pytest_configure` so that it's configurable via a pytest setting.
Unfortunately, doing that is tricky, as it means we can't import
`wait_signal.py` at all during import time, and people might do so for e.g. type
annotations.
With this refactoring, the `AttributeError` is out of the way, though there are
other subtle failures with multi-threaded signals now:
1) `_quit_loop_by_signal()` -> `_cleanup()` now simply calls `self._timer.stop()`
without setting `self._timer` to `None`. This still results in the same Qt
message quoted above (after all, the timer still doesn't belong to the calling
thread!), but it looks like the test terminates without any timeout anyways.
From what I can gather, while the "low level timer" continues to run (and
waste a minimal amount of resources), the QTimer still "detaches" from it and
stops running. The commit adds a test to catch this case (currently marked as
xfail).
2) The main thread in `wait()` can now still call `self._timer.start()` without
an `AttributeError`. However, in theory this could restart the timer after it
was already stopped by the signal emission, with a race between `_cleanup()` and
`wait()`.
See #586. This fixes the test-case posted by the reporter (added to the
testsuite in a simplified version), but not the additional considerations above.
The same fix is also applied to `CallbackBlocker`, though the test there is way
more unreliable in triggering the issue, and thus is skipped for taking too
long.
The-Compiler
added a commit
that referenced
this issue
Mar 22, 2025
- Always create the timeout `QTimer`, even if it's not going to be used.
- Connect the signal once in `_AbstractSignalBlocker`/`CallbackBlocker.__init__`
and never disconnect it.
- When checking whether a timeout was set, simply check `self._timeout` instead of
`self._timer`.
Before this change, we conditionally created a `QTimer` and deleted it again
when cleaning up. In addition to coming with hard to follow complexity, this
also caused multiple issues around this timer:
Warnings about disconnecting signal
===================================
In `AbstractSignalBlocker._cleanup`, we attempted to disconnect
`self._timer.timeout()` from `self._quit_loop_by_timeout`, under the condition that
a timer was created in `__init__` (i.e. `self.timeout` was not `0` or `None`).
However, the signal only got connected in `AbstractSignalBlocker.wait()`.
Thus, if `AbstractSignalBlocker` is used as a context manager with a timeout and
the signal is emitted inside it:
- `self._timer` is present
- The signal calls `self._quit_loop_by_signal()`
- `self._cleanup()` gets called from there
- That tries to disconnect `self._timer.timeout()` from `self._quit_loop_by_timeout()`
- Since `self.wait()` was never called, the signal was never connected
- Which then results in either an exception (PyQt), an internal SystemError
(older PySide6) or a warning (newer PySide6).
In 560f565 and later
81b317c this was fixed by ignoring
`TypeError`/`RuntimeError`, but that turned out to not be sufficient for newer
PySide versions: #552, #558.
The proper fix for this is to not attempt to disconnect a signal that was never
connected, which makes all the PySide6 warnings go away (and thus we can run our
testsuite with `-Werror` now).
As a drive-by fix, this also removes the old `filterwarnings` mark definition,
which was introduced in 95fee8b (perhaps as a
stop-gap for older pytest versions?) but shouldn't be needed anymore (and is
unused since e338809).
AttributeError when a signal/callback is emitted from a different thread
========================================================================
If a signal is emitted from a thread, `_AbstractSignalBlocker._cleanup()` also gets
called from the non-main thread (Qt will complain: "QObject::killTimer: Timers
cannot be stopped from another thread").
If the signal emission just happened to happen between the None-check and
calling `.start()` in `wait()`, it failed with an `AttributeError`:
Main thread in `AbstractSignalBlocker.wait()`:
```python
if self._timer is not None: # <--- this was true...
self._timer.timeout.connect(self._quit_loop_by_timeout)
# <--- ...now here, but self._timer is None now!
self._timer.start()
````
Emitting thread in `AbstractSignalBlocker._cleanup()` via
`._quit_loop_by_signal()`:
```python
if self._timer is not None:
...
self._timer = None # <--- here
```
In SignalBlocker.connect, we used:
```python
actual_signal.connect(self._quit_loop_by_signal)
```
which by default is supposed to use a `QueuedConnection` if the signal gets
emitted in a different thread:
https://doc.qt.io/qt-6/qt.html#ConnectionType-enum
(Default) If the receiver lives in the thread that emits the signal,
`Qt::DirectConnection` is used. Otherwise, `Qt::QueuedConnection` is used. The
connection type is determined when the signal is emitted.
though then that page says:
https://doc.qt.io/qt-6/qobject.html#thread-affinity
Note: If a `QObject` has no thread affinity (that is, if `thread()` returns
zero), or if it lives in a thread that has no running event loop, then it
cannot receive queued signals or posted events.
Which means `AbstractSignalBlocker` needs to be a `QObject` for this to work.
However, that means we need to use `qt_api.QtCore.QObject` as subclass, i.e. at
import time of `wait_signal.py`. Yet, `qt_api` only gets initialized in
`pytest_configure` so that it's configurable via a pytest setting.
Unfortunately, doing that is tricky, as it means we can't import
`wait_signal.py` at all during import time, and people might do so for e.g. type
annotations.
With this refactoring, the `AttributeError` is out of the way, though there are
other subtle failures with multi-threaded signals now:
1) `_quit_loop_by_signal()` -> `_cleanup()` now simply calls `self._timer.stop()`
without setting `self._timer` to `None`. This still results in the same Qt
message quoted above (after all, the timer still doesn't belong to the calling
thread!), but it looks like the test terminates without any timeout anyways.
From what I can gather, while the "low level timer" continues to run (and
waste a minimal amount of resources), the QTimer still "detaches" from it and
stops running. The commit adds a test to catch this case (currently marked as
xfail).
2) The main thread in `wait()` can now still call `self._timer.start()` without
an `AttributeError`. However, in theory this could restart the timer after it
was already stopped by the signal emission, with a race between `_cleanup()` and
`wait()`.
See #586. This fixes the test-case posted by the reporter (added to the
testsuite in a simplified version), but not the additional considerations above.
The same fix is also applied to `CallbackBlocker`, though the test there is way
more unreliable in triggering the issue, and thus is skipped for taking too
long.
The-Compiler
added a commit
that referenced
this issue
Mar 22, 2025
- Always create the timeout `QTimer`, even if it's not going to be used.
- Connect the signal once in `_AbstractSignalBlocker`/`CallbackBlocker.__init__`
and never disconnect it.
- When checking whether a timeout was set, simply check `self._timeout` instead of
`self._timer`.
Before this change, we conditionally created a `QTimer` and deleted it again
when cleaning up. In addition to coming with hard to follow complexity, this
also caused multiple issues around this timer:
Warnings about disconnecting signal
===================================
In `AbstractSignalBlocker._cleanup`, we attempted to disconnect
`self._timer.timeout()` from `self._quit_loop_by_timeout`, under the condition that
a timer was created in `__init__` (i.e. `self.timeout` was not `0` or `None`).
However, the signal only got connected in `AbstractSignalBlocker.wait()`.
Thus, if `AbstractSignalBlocker` is used as a context manager with a timeout and
the signal is emitted inside it:
- `self._timer` is present
- The signal calls `self._quit_loop_by_signal()`
- `self._cleanup()` gets called from there
- That tries to disconnect `self._timer.timeout()` from `self._quit_loop_by_timeout()`
- Since `self.wait()` was never called, the signal was never connected
- Which then results in either an exception (PyQt), an internal SystemError
(older PySide6) or a warning (newer PySide6).
In 560f565 and later
81b317c this was fixed by ignoring
`TypeError`/`RuntimeError`, but that turned out to not be sufficient for newer
PySide versions: #552, #558.
The proper fix for this is to not attempt to disconnect a signal that was never
connected, which makes all the PySide6 warnings go away (and thus we can run our
testsuite with `-Werror` now).
As a drive-by fix, this also removes the old `filterwarnings` mark definition,
which was introduced in 95fee8b (perhaps as a
stop-gap for older pytest versions?) but shouldn't be needed anymore (and is
unused since e338809).
AttributeError when a signal/callback is emitted from a different thread
========================================================================
If a signal is emitted from a thread, `_AbstractSignalBlocker._cleanup()` also gets
called from the non-main thread (Qt will complain: "QObject::killTimer: Timers
cannot be stopped from another thread").
If the signal emission just happened to happen between the None-check and
calling `.start()` in `wait()`, it failed with an `AttributeError`:
Main thread in `AbstractSignalBlocker.wait()`:
```python
if self._timer is not None: # <--- this was true...
self._timer.timeout.connect(self._quit_loop_by_timeout)
# <--- ...now here, but self._timer is None now!
self._timer.start()
````
Emitting thread in `AbstractSignalBlocker._cleanup()` via
`._quit_loop_by_signal()`:
```python
if self._timer is not None:
...
self._timer = None # <--- here
```
In SignalBlocker.connect, we used:
```python
actual_signal.connect(self._quit_loop_by_signal)
```
which by default is supposed to use a `QueuedConnection` if the signal gets
emitted in a different thread:
https://doc.qt.io/qt-6/qt.html#ConnectionType-enum
(Default) If the receiver lives in the thread that emits the signal,
`Qt::DirectConnection` is used. Otherwise, `Qt::QueuedConnection` is used. The
connection type is determined when the signal is emitted.
though then that page says:
https://doc.qt.io/qt-6/qobject.html#thread-affinity
Note: If a `QObject` has no thread affinity (that is, if `thread()` returns
zero), or if it lives in a thread that has no running event loop, then it
cannot receive queued signals or posted events.
Which means `AbstractSignalBlocker` needs to be a `QObject` for this to work.
However, that means we need to use `qt_api.QtCore.QObject` as subclass, i.e. at
import time of `wait_signal.py`. Yet, `qt_api` only gets initialized in
`pytest_configure` so that it's configurable via a pytest setting.
Unfortunately, doing that is tricky, as it means we can't import
`wait_signal.py` at all during import time, and people might do so for e.g. type
annotations.
With this refactoring, the `AttributeError` is out of the way, though there are
other subtle failures with multi-threaded signals now:
1) `_quit_loop_by_signal()` -> `_cleanup()` now simply calls `self._timer.stop()`
without setting `self._timer` to `None`. This still results in the same Qt
message quoted above (after all, the timer still doesn't belong to the calling
thread!), but it looks like the test terminates without any timeout anyways.
From what I can gather, while the "low level timer" continues to run (and
waste a minimal amount of resources), the QTimer still "detaches" from it and
stops running. The commit adds a test to catch this case (currently marked as
xfail).
2) The main thread in `wait()` can now still call `self._timer.start()` without
an `AttributeError`. However, in theory this could restart the timer after it
was already stopped by the signal emission, with a race between `_cleanup()` and
`wait()`.
See #586. This fixes the test-case posted by the reporter (added to the
testsuite in a simplified version), but not the additional considerations above.
The same fix is also applied to `CallbackBlocker`, though the test there is way
more unreliable in triggering the issue, and thus is skipped for taking too
long.
The-Compiler
added a commit
that referenced
this issue
Mar 22, 2025
- Always create the timeout `QTimer`, even if it's not going to be used.
- Connect the signal once in `_AbstractSignalBlocker`/`CallbackBlocker.__init__`
and never disconnect it.
- When checking whether a timeout was set, simply check `self._timeout` instead of
`self._timer`.
Before this change, we conditionally created a `QTimer` and deleted it again
when cleaning up. In addition to coming with hard to follow complexity, this
also caused multiple issues around this timer:
Warnings about disconnecting signal
===================================
In `AbstractSignalBlocker._cleanup`, we attempted to disconnect
`self._timer.timeout()` from `self._quit_loop_by_timeout`, under the condition that
a timer was created in `__init__` (i.e. `self.timeout` was not `0` or `None`).
However, the signal only got connected in `AbstractSignalBlocker.wait()`.
Thus, if `AbstractSignalBlocker` is used as a context manager with a timeout and
the signal is emitted inside it:
- `self._timer` is present
- The signal calls `self._quit_loop_by_signal()`
- `self._cleanup()` gets called from there
- That tries to disconnect `self._timer.timeout()` from `self._quit_loop_by_timeout()`
- Since `self.wait()` was never called, the signal was never connected
- Which then results in either an exception (PyQt), an internal SystemError
(older PySide6) or a warning (newer PySide6).
In 560f565 and later
81b317c this was fixed by ignoring
`TypeError`/`RuntimeError`, but that turned out to not be sufficient for newer
PySide versions: #552, #558.
The proper fix for this is to not attempt to disconnect a signal that was never
connected, which makes all the PySide6 warnings go away (and thus we can run our
testsuite with `-Werror` now).
As a drive-by fix, this also removes the old `filterwarnings` mark definition,
which was introduced in 95fee8b (perhaps as a
stop-gap for older pytest versions?) but shouldn't be needed anymore (and is
unused since e338809).
AttributeError when a signal/callback is emitted from a different thread
========================================================================
If a signal is emitted from a thread, `_AbstractSignalBlocker._cleanup()` also gets
called from the non-main thread (Qt will complain: "QObject::killTimer: Timers
cannot be stopped from another thread").
If the signal emission just happened to happen between the None-check and
calling `.start()` in `wait()`, it failed with an `AttributeError`:
Main thread in `AbstractSignalBlocker.wait()`:
```python
if self._timer is not None: # <--- this was true...
self._timer.timeout.connect(self._quit_loop_by_timeout)
# <--- ...now here, but self._timer is None now!
self._timer.start()
````
Emitting thread in `AbstractSignalBlocker._cleanup()` via
`._quit_loop_by_signal()`:
```python
if self._timer is not None:
...
self._timer = None # <--- here
```
In SignalBlocker.connect, we used:
```python
actual_signal.connect(self._quit_loop_by_signal)
```
which by default is supposed to use a `QueuedConnection` if the signal gets
emitted in a different thread:
https://doc.qt.io/qt-6/qt.html#ConnectionType-enum
(Default) If the receiver lives in the thread that emits the signal,
`Qt::DirectConnection` is used. Otherwise, `Qt::QueuedConnection` is used. The
connection type is determined when the signal is emitted.
though then that page says:
https://doc.qt.io/qt-6/qobject.html#thread-affinity
Note: If a `QObject` has no thread affinity (that is, if `thread()` returns
zero), or if it lives in a thread that has no running event loop, then it
cannot receive queued signals or posted events.
Which means `AbstractSignalBlocker` needs to be a `QObject` for this to work.
However, that means we need to use `qt_api.QtCore.QObject` as subclass, i.e. at
import time of `wait_signal.py`. Yet, `qt_api` only gets initialized in
`pytest_configure` so that it's configurable via a pytest setting.
Unfortunately, doing that is tricky, as it means we can't import
`wait_signal.py` at all during import time, and people might do so for e.g. type
annotations.
With this refactoring, the `AttributeError` is out of the way, though there are
other subtle failures with multi-threaded signals now:
1) `_quit_loop_by_signal()` -> `_cleanup()` now simply calls `self._timer.stop()`
without setting `self._timer` to `None`. This still results in the same Qt
message quoted above (after all, the timer still doesn't belong to the calling
thread!), but it looks like the test terminates without any timeout anyways.
From what I can gather, while the "low level timer" continues to run (and
waste a minimal amount of resources), the QTimer still "detaches" from it and
stops running. The commit adds a test to catch this case (currently marked as
xfail).
2) The main thread in `wait()` can now still call `self._timer.start()` without
an `AttributeError`. However, in theory this could restart the timer after it
was already stopped by the signal emission, with a race between `_cleanup()` and
`wait()`.
See #586. This fixes the test-case posted by the reporter (added to the
testsuite in a simplified version), but not the additional considerations above.
The same fix is also applied to `CallbackBlocker`, though the test there is way
more unreliable in triggering the issue, and thus is skipped for taking too
long.
The-Compiler
added a commit
that referenced
this issue
Mar 25, 2025
- Always create the timeout `QTimer`, even if it's not going to be used.
- Connect the signal once in `_AbstractSignalBlocker`/`CallbackBlocker.__init__`
and never disconnect it.
- When checking whether a timeout was set, simply check `self._timeout` instead of
`self._timer`.
Before this change, we conditionally created a `QTimer` and deleted it again
when cleaning up. In addition to coming with hard to follow complexity, this
also caused multiple issues around this timer:
Warnings about disconnecting signal
===================================
In `AbstractSignalBlocker._cleanup`, we attempted to disconnect
`self._timer.timeout()` from `self._quit_loop_by_timeout`, under the condition that
a timer was created in `__init__` (i.e. `self.timeout` was not `0` or `None`).
However, the signal only got connected in `AbstractSignalBlocker.wait()`.
Thus, if `AbstractSignalBlocker` is used as a context manager with a timeout and
the signal is emitted inside it:
- `self._timer` is present
- The signal calls `self._quit_loop_by_signal()`
- `self._cleanup()` gets called from there
- That tries to disconnect `self._timer.timeout()` from `self._quit_loop_by_timeout()`
- Since `self.wait()` was never called, the signal was never connected
- Which then results in either an exception (PyQt), an internal SystemError
(older PySide6) or a warning (newer PySide6).
In 560f565 and later
81b317c this was fixed by ignoring
`TypeError`/`RuntimeError`, but that turned out to not be sufficient for newer
PySide versions: #552, #558.
The proper fix for this is to not attempt to disconnect a signal that was never
connected, which makes all the PySide6 warnings go away (and thus we can run our
testsuite with `-Werror` now).
As a drive-by fix, this also removes the old `filterwarnings` mark definition,
which was introduced in 95fee8b (perhaps as a
stop-gap for older pytest versions?) but shouldn't be needed anymore (and is
unused since e338809).
AttributeError when a signal/callback is emitted from a different thread
========================================================================
If a signal is emitted from a thread, `_AbstractSignalBlocker._cleanup()` also gets
called from the non-main thread (Qt will complain: "QObject::killTimer: Timers
cannot be stopped from another thread").
If the signal emission just happened to happen between the None-check and
calling `.start()` in `wait()`, it failed with an `AttributeError`:
Main thread in `AbstractSignalBlocker.wait()`:
```python
if self._timer is not None: # <--- this was true...
self._timer.timeout.connect(self._quit_loop_by_timeout)
# <--- ...now here, but self._timer is None now!
self._timer.start()
````
Emitting thread in `AbstractSignalBlocker._cleanup()` via
`._quit_loop_by_signal()`:
```python
if self._timer is not None:
...
self._timer = None # <--- here
```
In SignalBlocker.connect, we used:
```python
actual_signal.connect(self._quit_loop_by_signal)
```
which by default is supposed to use a `QueuedConnection` if the signal gets
emitted in a different thread:
https://doc.qt.io/qt-6/qt.html#ConnectionType-enum
(Default) If the receiver lives in the thread that emits the signal,
`Qt::DirectConnection` is used. Otherwise, `Qt::QueuedConnection` is used. The
connection type is determined when the signal is emitted.
though then that page says:
https://doc.qt.io/qt-6/qobject.html#thread-affinity
Note: If a `QObject` has no thread affinity (that is, if `thread()` returns
zero), or if it lives in a thread that has no running event loop, then it
cannot receive queued signals or posted events.
Which means `AbstractSignalBlocker` needs to be a `QObject` for this to work.
However, that means we need to use `qt_api.QtCore.QObject` as subclass, i.e. at
import time of `wait_signal.py`. Yet, `qt_api` only gets initialized in
`pytest_configure` so that it's configurable via a pytest setting.
Unfortunately, doing that is tricky, as it means we can't import
`wait_signal.py` at all during import time, and people might do so for e.g. type
annotations.
With this refactoring, the `AttributeError` is out of the way, though there are
other subtle failures with multi-threaded signals now:
1) `_quit_loop_by_signal()` -> `_cleanup()` now simply calls `self._timer.stop()`
without setting `self._timer` to `None`. This still results in the same Qt
message quoted above (after all, the timer still doesn't belong to the calling
thread!), but it looks like the test terminates without any timeout anyways.
From what I can gather, while the "low level timer" continues to run (and
waste a minimal amount of resources), the QTimer still "detaches" from it and
stops running. The commit adds a test to catch this case (currently marked as
xfail).
2) The main thread in `wait()` can now still call `self._timer.start()` without
an `AttributeError`. However, in theory this could restart the timer after it
was already stopped by the signal emission, with a race between `_cleanup()` and
`wait()`.
See #586. This fixes the test-case posted by the reporter (added to the
testsuite in a simplified version), but not the additional considerations above.
The same fix is also applied to `CallbackBlocker`, though the test there is way
more unreliable in triggering the issue, and thus is skipped for taking too
long.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I recently tried upgrading to an early
PySide6==6.7.0(via https://download.qt.io/snapshots/ci/pyside/6.7/latest/pyside6), and I find thatpytestthen issues a lot of warnings that I cannot seem to suppress:filterwarnings = ["ignore"]does not seem to have an effect, possibly becauseRuntimeErroris not a proper warning.The text was updated successfully, but these errors were encountered: