Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix tmp_path_retention_policy crash when skipping from fixture #10517

Merged
merged 6 commits into from
Nov 23, 2022
Merged

Fix tmp_path_retention_policy crash when skipping from fixture #10517

merged 6 commits into from
Nov 23, 2022

Conversation

ykadowak
Copy link
Contributor

closes #10502

@ykadowak ykadowak mentioned this pull request Nov 19, 2022
Merged
The-Compiler
The-Compiler requested changes Nov 22, 2022
View reviewed changes
src/_pytest/tmpdir.py Outdated Show resolved Hide resolved
testing/test_tmpdir.py Outdated Show resolved Hide resolved
changelog/10502.bugfix.rst Outdated Show resolved Hide resolved
The-Compiler
The-Compiler approved these changes Nov 23, 2022
View reviewed changes
Copy link
Member

@The-Compiler The-Compiler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me, thanks! Let's see if @nicoddemus has anything to add, and if not, I think this can be merged.

nicoddemus
nicoddemus approved these changes Nov 23, 2022
View reviewed changes
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @yusuke-kadowaki!

Also thanks @The-Compiler for the review and catching using the stash. 👍

@nicoddemus nicoddemus merged commit 99dfc19 into pytest-dev:main Nov 23, 2022
@ykadowak ykadowak deleted the fix-10502 branch November 23, 2022 13:56
@ykadowak ykadowak mentioned this pull request Nov 26, 2022
Merged
The-Compiler added a commit to qutebrowser/qutebrowser that referenced this pull request Dec 6, 2022
@The-Compiler
Use a proper @pyqtSlot for FilenamePrompt
7b90fd3 
In bleeding tests, we started to get a segfault on the second test in
tests/unit/mainwindow/test_prompt.py, with a stacktrace like:

    Thread 1 "python" received signal SIGSEGV, Segmentation fault.
    0x00007ffff7b55912 in _PyFunction_Vectorcall (func=<function at remote 0x7fffc4368ca0>, stack=0x7fffd74656a8, nargsf=<optimized out>, kwnames=0x0) at Objects/call.c:341
    341	   if (((PyCodeObject *)f->fc_code)->co_flags & CO_OPTIMIZED) {
    (gdb) bt
    #0  0x00007ffff7b55912 in _PyFunction_Vectorcall (func=<function at remote 0x7fffc4368ca0>, stack=0x7fffd74656a8, nargsf=<optimized out>, kwnames=0x0) at Objects/call.c:341
    #1  0x00007ffff4e0b3f1 in PyQtSlot::call(_object*, _object*) const (this=0x555556c759c0, args=('/tmp/pytest-of-florian/pytest-70/test_simple_completion_1_next_0/test',), callable=<function at remote 0x7fffc4368ca0>)
        at ../../qpy/QtCore/qpycore_pyqtslot.cpp:247
    #2  PyQtSlot::invoke(void**, _object*, void*, bool) const (this=0x555556c759c0, qargs=<optimized out>, qargs@entry=0x7fffffff86c0, self=<optimized out>, self@entry=0x0, result=result@entry=0x0, no_receiver_check=<optimized out>)
        at ../../qpy/QtCore/qpycore_pyqtslot.cpp:159
    #3  0x00007ffff4e12213 in PyQtSlot::invoke(void**, bool) const (no_receiver_check=<optimized out>, qargs=0x7fffffff86c0, this=<optimized out>) at ../../qpy/QtCore/qpycore_pyqtslot.cpp:78
    #4  PyQtSlotProxy::unislot(void**) (qargs=0x7fffffff86c0, this=0x555557193e20) at ../../qpy/QtCore/qpycore_pyqtslotproxy.cpp:205
    #5  PyQtSlotProxy::unislot(void**) (qargs=0x7fffffff86c0, this=0x555557193e20) at ../../qpy/QtCore/qpycore_pyqtslotproxy.cpp:186
    #6  PyQtSlotProxy::qt_metacall(QMetaObject::Call, int, void**) (this=0x555557193e20, _c=<optimized out>, _id=0, _a=0x7fffffff86c0) at ../../qpy/QtCore/qpycore_pyqtslotproxy.cpp:170
    #7  0x00007ffff48bd91d in doActivate<false>(QObject*, int, void**) (sender=0x555556b3d680, signal_index=28, argv=0x7fffffff86c0) at kernel/qobject.cpp:3945
    #8  0x00007fffeff96aca in QFileSystemModel::directoryLoaded(QString const&) (this=<optimized out>, _t1=<optimized out>) at .moc/moc_qfilesystemmodel.cpp:272
    #9  0x00007ffff48b0be0 in QObject::event(QEvent*) (this=this@entry=0x555556b3d680, e=e@entry=0x7fff0c004af0) at kernel/qobject.cpp:1347
    #10 0x00007fffeff962ab in QFileSystemModel::event(QEvent*) (this=this@entry=0x555556b3d680, event=event@entry=0x7fff0c004af0) at dialogs/qfilesystemmodel.cpp:1748
    #11 0x00007ffff070995c in sipQFileSystemModel::event(QEvent*) (this=0x555556b3d680, a0=0x7fff0c004af0) at /usr/src/debug/pyqt5/PyQt5-5.15.7/build/QtWidgets/sipQtWidgetsQFileSystemModel.cpp:376
    [...]
    #23 0x00007ffff4da94ce in meth_QCoreApplication_processEvents(PyObject*, PyObject*, PyObject*) (sipArgs=<optimized out>, sipKwds=0x0) at /usr/src/debug/pyqt5/PyQt5-5.15.7/build/QtCore/sipQtCoreQCoreApplication.cpp:590
    [...]

from pytest-qt in Python:

    Current thread 0x00007fb366207740 (most recent call first):
    File ".../pytestqt/plugin.py", line 209 in _process_events
    File ".../pytestqt/plugin.py", line 171 in pytest_runtest_setup
    [...]
    File ".../pytest/src/_pytest/runner.py", line 115 in pytest_runtest_protocol
    [...]
    File ".../pytest/src/_pytest/main.py", line 317 in pytest_cmdline_main
    [...]

Introduced by this change in pytest, which now deletes the temporary
directory after the test by default:

pytest-dev/pytest#10517

It sounds like something odd like the directory removal causing (Py)Qt
to call the lambda, but the underlying Python object already being gone
or something along those lines. With a proper Qt slot (@pyqtSlot),
PyQt seems to do the right thing, so let's just use that instead...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicoddemus nicoddemus nicoddemus approved these changes

@The-Compiler The-Compiler The-Compiler approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tmp_path_retention_policy crash when skipping from fixture
3 participants
@ykadowak @The-Compiler @nicoddemus